Yes, and I'd argue this was beyond a simple "mistake", it's borderline negligence for anyone but a very junior engineer: https://archive.vn/oxbck

EDIT: that link isn't working, but here is a screenshot: https://imgur.com/4lyElZI

Especially in Rails it is. Since there have been so many iterations on how to sanitize input strings and typically you have explicitly choose not to sanitize it to allow for this to happen. Which is exactly what the author of that code did. So yes, I'm in general against mocking coding mistakes, but in this case I'd say its warranted given that the person at fault actively tried to cheat his way out of using the framework provided primitives.

I don’t know if we have comparable life experience.

I have seen more shitty code than high quality code in my life. It’s hard for me to empathize with anyone who thinks startup code is going to generally meet some decent quality bar.

People like Brian acton and Jan Korum exist, but they are the exception. If you always expect people to write good code, you will be fucking disappointed.

The only solution that scales is having adequate speed bumps and guard rails that prevent people from pushing code till after it’s been properly tested. And just because it’s in a git log doesn’t mean it gets pushed to production.

This article reminds me that journalists only make money when they squeeze your amygdala. Reading content from ars is the equivalent of an intellectual jump scare. Add 127.0.0.1 arstechnica.com to your resolv.conf file for mental health. Fuck off ars

Right, but the point is that either Gab had normal, reasonable safeguards in place (for the reasons you point out) and their CTO bypassed them creating a very serious security hole, or they didn’t have the safeguards in the first place meaning their internal processes are poorly set up and controlled.

What’s important here is that the CTO is ultimately responsible for the failure either way. That’s the point of the article and it’s why the article is valid (even if you don’t like it for some reason): the engineering buck stops at the CTO, especially so if they personally create a bug that exfiltrates all of the company user data, regardless of how they managed to screw it up.

Brian Acton and Jan Koum weren’t exceptions either. WhatsApp had some hilariously bad vulnerabilities. Like no authentication on their APIs and trusting the client.

https://www.mathyvanhoef.com/2012/05/whatsapp-considered-ins...

https://www.mathyvanhoef.com/2012/07/whatsapp-follow-up-unau...

To be fair, Rails has a lot more baked in base security than erlang did. Yes Rails had it's fair share of vulnerabilities. I remember there was a major eval issue in some deserializer code, but things such as sql injections or using POST over query strings is just basic knowledge almost every mediocre+ rails dev knows. They picked erlang for their scalability, but as a result had to write a lot of other things from scratch. WhatsApp is still an interesting engineering case study.

it's not a rails vulnerability though. This would happen irrespective of the language/framework if the developer decide to forego basic sanity checks. This would have been prevented with basic input validations (in any of the layers before it hit model, and model itself), or the organization did basic security testing.

You miss the point. If you use rails provided primitives it absolutely wouldn't happen. So it's fair to categorize this as a rookie coding mistake. The author took code that had these security primitives baked in the framework and REMOVED them, replacing it with insecure custom sql, which in turn led to the vulnerability.

Hah! Too great. Ok, so no good code in startups.

I'm not even front end and I spotted that SQL injection vulnerability in like 4 seconds. That is like 101 stuff.

This is totally untrue, considering the caliber of companies which have been hit with SQL injection vulns, including Google, Facebook (your employer), Reserve Banks, governments, etc.

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.

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.

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.

I've been on teams building web applications for 14 years and not once have I seen someone simply rip out a query builder like this in production code and replace it with a non-parameterized string—let alone someone with 20+ years of experience.

Accidents and SQL injections happen, usually because of non-obvious query-building but this is a different level.

When swift was still young the only mysql library available for it did the same mistake. The memory safety sure didnt help here... Sure enough i had to roll my own code.

Well not all injection vulns deserve this level of scrutiny.

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.

It happened to your employer. You might not have been on the team but this stuff is rife. WhatsApp in particular has made some absolutely rookie-tier security blunders [1].

[1] https://threatpost.com/whatsapp-bug-malicious-code-injection...

Yeah, I beat this dead horse every time the armchair security quarterbacks climb on their high horse in these comments. These “dumb” mistakes happen to everyone, at every company. Even security experts. Even one of tptacek’s companies had a well known incident, and people said all the same things about their “dumb” mistakes.

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 take the same standpoint as you most of the time. But you can't fucking interpolate user input into SQL queries without physically cringing and call yourself an experienced engineer. Can you really pull up a code editor and write that out without your brain sounding off a thousand alarm bells that you're massively messing up something?

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!

Who knows what was going on. In an ideal world we have the time needed to do things well. In reality, you've got twenty developers asking for something, users requesting features, various government agencies asking for documents, the lawyers inform you about some case and... oh your wife is calling on the other line because... etc etc etc.

I get so busy I rip out query builders to SQL concat strings all the time. /s

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?

This is the textbook example of what not to do with SQL queries. You can't even learn SQL or an SQL library API without being warned against this a dozen times.

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.

> These “dumb” mistakes happen to everyone, at every company. Even security experts.

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.

No they don't. If you do something as blindingly stupid as this, you don't deserve commit access.

Is it possible that it was intentional?

I could certainly understand someone working for Gab for a paycheck, and then feeling guilty and "making a mistake".

To be clear, I'm not saying that one should feel that way, just acknowledging the possibility.

While it doesn’t make sense how a mistake like this could happen, I also don’t think this could be intentional either. I don’t see why anyone would throw their reputation into the gutter like that.

Imagine trying to explain this to a future employer.

There’s really nothing stopping one developer from pushing a commit purporting to be from another developer. You can put any name or email address you like in each commit. However, that’s something that should be caught during review, so it’s not much of an excuse.

This is also why companies should enforce signing commits.

Even a small company has a huge amount of things going on at the same time. Any one thing in isolation is simple, but put together they amount to a huge amount of work where you can't put the time needed into become an expert on everything.

That applies to anyone any employee, including the CTO, especially so. They're just people.

Publishing an entire article to shame them is just disgusting. Like, why? The reporter if anyone deserves being shamed for apparently making a career of flinging shit at people.

Yeah most management books (See: Radical Candor) say that executive’s should own up to their mistakes publicly and receive public criticism so that it makes it easier for subordinates to give criticism and even receive it themselves privately.

On another note, how do we know that the new CTO of Gab didn’t just do this on purpose so that there would be a breach?

Considering that the new CTO is ex Facebook, doesn't that give more credence to this suggestion? That it was intentional?

It’s easy to be armchair sql injection expert and point fingers.

I can guarantee that any system that you’ve worked on has numerous OWASP security bugs. You’ve probably looked at the bugs countless times and never noticed it.

Every software engineer of all levels has overlooked obvious sql injection bugs in their code base. Most likely you’ve added to the bug list.

Software bugs are simple part of any development effort. All major companies, Microsoft, Google, Facebook, etc. has very simple bugs like this in their systems.

That’s why they pay out bug bounties, it’s cheaper for them to add the bug and have some random security researcher find the bugs for them.