8 ms·

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 https://archiv

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 https://archive.vn/oxbck EDIT: that link isn't working, but here is a screenshot: https://imgur.com/4lyElZI https://imgur.com/4lyElZI

DaniloDias 6y ago
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
- deleted 6y ago
  [deleted]
- lawnchair_larry 6y ago
  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-insecure.html https://www.mathyvanhoef.com/2012/05/whatsapp-considered-ins... https://www.mathyvanhoef.com/2012/07/whatsapp-follow-up-unauthenticated.html https://www.mathyvanhoef.com/2012/07/whatsapp-follow-up-unau...
  - DaniloDias 6y ago
    Hah! Too great. Ok, so no good code in startups.
  - rjzzleep 6y ago
    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.
    - quetzthecoatl 6y ago
      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.
      - rjzzleep 6y ago
        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.
- mrzimmerman 6y ago
  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.
0xy 6y ago
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.
- jdxcode 6y ago
  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.
  - 0xy 6y ago
    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-rce/152578/ https://threatpost.com/whatsapp-bug-malicious-code-injection...
  - Cloudef 6y ago
    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.
    - jdxcode 6y ago
      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.
rjzzleep 6y ago
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.
kortex 6y ago
I'm not even front end and I spotted that SQL injection vulnerability in like 4 seconds. That is like 101 stuff.
Shared404 6y ago
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.
- jdxcode 6y ago
  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.
  - zenexer 6y ago
    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.
    - jtokoph 6y ago
      This is also why companies should enforce signing commits.

DaniloDias 6y ago

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

deleted 6y ago
[deleted]
lawnchair_larry 6y ago
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-insecure.html https://www.mathyvanhoef.com/2012/05/whatsapp-considered-ins... https://www.mathyvanhoef.com/2012/07/whatsapp-follow-up-unauthenticated.html https://www.mathyvanhoef.com/2012/07/whatsapp-follow-up-unau...
- DaniloDias 6y ago
  Hah! Too great. Ok, so no good code in startups.
- rjzzleep 6y ago
  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.
  - quetzthecoatl 6y ago
    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.
    - rjzzleep 6y ago
      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.
mrzimmerman 6y ago
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.

0xy 6y ago

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.

jdxcode 6y ago
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.
- 0xy 6y ago
  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-rce/152578/ https://threatpost.com/whatsapp-bug-malicious-code-injection...
- Cloudef 6y ago
  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.
  - jdxcode 6y ago
    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.

rjzzleep 6y ago

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.

kortex 6y ago

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

Shared404 6y ago

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.

jdxcode 6y ago
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.
- zenexer 6y ago
  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.
  - jtokoph 6y ago
    This is also why companies should enforce signing commits.