Hacker News Reader

Comment by dj_mc_merlin | Hacker News Reader

No senior engineer should blatantly concatenate strings to make an SQL query. That is a junior mistake, no doubts about it. If you haven't learned that user input is not to be concatenated into code unless you think long and hard about it and you're really sure there's no better option, you are not in a position to lead.

Seriously, if this is a mistake a senior engineer could do at your company, why would you work there? I'm not one for gatekeeping but this is learned within a couple of code reviews in your first years.

View on HN

By your logic, nobody should work at Facebook, Google, Apple etc etc etc.

Unless you're claiming that those SQL injection exploits were introduced AND review approved by junior engineers, a practical impossibility according to policy.

Your claim that senior engineers don't make whopping security blunders is absolutely false according to all available evidence. The smartest programmers working on kernels make them constantly.

Former Facebooker, on the storage team—if a senior engineer* wrote the supplied code, well, I can't say they'd be fired, but it 100% would not reflect well on them come review time. Also, all their peers would know they made the most boneheaded of mistakes, and they would definitely be getting the hairy eyeball come code review.

Raw untrusted string substitution into a SQL query is just unacceptable. It's on the level of storing unhashed passwords, or 1000-line god functions: the kind of mistake you'd expect from a self-taught developer writing PHP in 2002. Literally every popular SQL client and ORM provides quoted substitution; please use it. And, dear reader, if this was news to you, and you write SQL, please go get educated, because it's not the last nor the sneakiest pitfall in this space!

* Senior enough to be writing raw SQL code at Facebook

Facebook has had multiple injection exploits, including SQL and JS RCEs. It's possible no senior were the git blame behind them, but it's absolutely impossible that no seniors reviewed and then maintained that code.

WhatsApp had a raw JavaScript RCE literally last year.

Also, blaming colleagues for making a mistake is extremely toxic behavior. I've fixed dozens of >9.0 CVSS vulnerabilities at multiple billion-dollar organizations. I'd say senior engineers were involved in their introduction at least half of the time.

I suspect that the mockery comes at least in part because it's Ars reporting on Gab. If it were Breitbart reporting on MoveOn.org, then I have a feeling folks would be a bit more receptive to your call for empathy here.

I'm with you in the sense that it always is the process, rarely the dev. It's complicated that this mistake was made by the CTO, who is responsible for being sure such processes are implemented. The lesson for me is that we all must be continually vigilant against both shitty code and evil, both without and within. Anyone can write terrible code, irrespective of political stance. Likewise anyone at all can do evil.

Bit of an aside, but it's hilarious that we've gone from SQL as a language targeted at non-programmer business analysts to something that apparently only senior devs should even consider writing out manually. The whole 4GL experiment turned out to be a disaster.

I think that's more of an OLAP vs OLTP difference here. Say you're an international shipping company: you would be perfectly happy to have your analysts write SELECTs versus your shipment database (or a replica), but you wouldn't want them writing the UPDATEs executed from field data.

You could not be more wrong. You ought to be more humble, however.

Reply on news.ycombinator.com