I don't know what experience you have, but senior engineers constantly make security mistakes. Linux kernel developers are some of the smartest programmers in existence yet they continually introduce and fail to patch security vulnerabilities.
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.
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.
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
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'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.
Accidents and SQL injections happen, usually because of non-obvious query-building but this is a different level.
[1] https://threatpost.com/whatsapp-bug-malicious-code-injection...
I'm not familiar with that vuln, but I don't see how it could be the "same mistake". My guess is there simply wasn't support for parameterization or there was a non-obvious concatenation problem when building the query string—but please correct me if I'm wrong.
This instance is novel because the parameterization protection was removed in favor of concatenation and that the vuln is so obvious a first-year CS student wouldn't struggle to identify it.
Companies are very porous, people take shortcuts, temporary workarounds get forgotten and left in place, etc.
You can tell who actually has real experience by who is saying this and getting downvoted for it, ironically. All of the downvoted comments in this thread are correct.
I can imagine myself doing this, having not slept in the last 20 hours, after 7 martinis and one especially long island. Maybe he did that too, but he should have the common sense as CTO of the company not to!
That’s like going 120mph and telling the cop you were just “distracted”.
You’re correct that we often make mistakes when we’re busy, but this isn’t that. This is OWASP #1 and the most plain example of it.
This is not a “mistake” that engineers make, this is unethical to allow this to go into production. He even has a stackoverflow answer where he explains this exact problem: https://stackoverflow.com/questions/11554015/syntax-sql-wher...
If you think I’m wrong, then what could an engineer be responsible for? Literally nothing ever?
If the CTO pushed this code, and the processes the CTO themself is responsible for designing did not catch this, it is safe to say the CTO is incompetent.
Dumb mistakes happen, true. This one should never happen to security experts, ever, unless their expertise is fake/bluff.
Stripping out safe parameterised code and replacing it with injected user input is only something that "happen(s) to everyone" at a very early stage in their career.
- For an intern or a junior dev, it's just a learning experience and no names.
- For a dev with a few years experience, it's a cause for concern but still no names.
- For a senior dev, I'd wonder if they lied on their CV but still no names.
- At CTO level this is not acceptable. Maybe no names, I'm on the fence, but they are a semi-public figure by nature of their role and their name was on the commit so they are the ones that made it public.
Either way, the CTO should have lost all credibility with their colleagues and be out the door. They represent the norms, processes, and safeguards the company employs and should not be in that role.
PS. I understand that there is no proof this commit caused/allowed the data exfiltration. It doesn't matter; the commit is heinous enough on it's own merits.