3 ms·

> Looks like every single one of the 38 vulnerabilities were either SQL injection, XSS, path traversal or "Insecure Direct Object Reference" aka failing to chec

> Looks like every single one of the 38 vulnerabilities were either SQL injection, XSS, path traversal or "Insecure Direct Object Reference" aka failing to check the caller was allowed to access the record. Seems like code review against a checklist of the most common vulnerabilities would have prevented these problems. So I guess there are two takeaways here: First, AI scanners are useful for catching security problems your team has overlooked. Second, maintaining a checklist of the most-common vulnerabilities and using it during code review is likely to not only prevent most of the problems that AI is likely to catch, but also show your development team many of their security blind spots at review time and teach them how to light those areas. That is, the team learns how to avoid creating those security mistakes in the first place.

dgb23 5mo ago
But by not having a checklist you avoid that your blind spots get exposed.
- tmoertel 5mo ago
  Why would you want to prevent your development team from learning about their blind spots?
  - dylan604 5mo ago
    So you can move faster to the next features obviously. Refactoring for secure code is time consuming, and clearly wasted cycles as the code is working. /s
  - dgb23 5mo ago
    I tried to make a joke about the tensions of security and accountability.
capiki 5mo ago
What about having the checklist and having an AI tool use it to catch things at review time (or even development time)?
- tmoertel 5mo ago
  Having AI tools do the review against the checklist would probably prevent the problems. However, it would probably be substantially inferior as a teaching tool for your team. The exercise of having reviewers hunt the checklisted vulnerabilities for themselves is what develops the mental muscles needed to understand the vulnerabilities in depth and avoid them when designing and writing future code. But, yes, I'd augment any manual review with a checklist and AI review as a final step. If the AI catches any problems then, your reviewers will be primed to think about why they overlooked them.
  - dylan604 5mo ago
    > The exercise of having reviewers hunt the checklisted vulnerabilities for themselves is what develops the mental muscles needed to understand the vulnerabilities in depth and avoid them when designing and writing future code. Could not agree any more strongly. These automagic tools are one thing in the hands of a dev that groks the basics like these examples. It would be one thing if new devs were actually reviewing the generated code to understand it, but so much is just vibe coded and deployed as soon as it "works". I get flack from not immediately deploying generated code because I want to take time to understand how it works. It's really grating and a lot of friction is coming from it.
  - capitalhilbilly 5mo ago
    For vulnerabilities of this nature is there really a point in training if an AI will catch them from now on? Seems like a variant of the allowing calculators problem and maybe the problem codeless platforms would have had. If these style of bugs don't change design in any meaningful way then the user can just write pseudo variables and the AI can normalize to safe code and their ability to work without the AI and IDE is probably less relevant than freeing their cognitive load for more complex constraint problems.
  - ndriscoll 5mo ago
    Suppose we still need humans to be writing code and caring about this stuff for the foreseeable future, so we need people to continue learning about the ways things can go wrong. For something like injection, you still ideally have a lint rule that says "don't concatenate things that look like SQL/HTML/etc. Use the correct macros for string interpolation". What does it actually teach for a reviewer to tell you that? You can ask the reviewer for more information, but you can ask your teammate anyway if you don't understand why the linter is mad. You can also ask the robot, who will patiently explain it to you even long after all of the knowledgeable humans have retired or died. The robot could even link to a prompt asking to explain it: https://chatgpt.com/share/69f10515-8808-83ea-abe3-a758d3144cc9 https://chatgpt.com/share/69f10515-8808-83ea-abe3-a758d3144c... If people aren't learning more with AI, that's a meta skill they need to develop. As for training the review muscles, why would you do that if you have a linter that rejects when you make the mistake? I don't expect reviewers to check whether you eschew nulls or uninitialized variables; I expect the compiler to do that, and I expect over time that more and more things will become tooling concerns (especially given that rigid tools with appropriate feedback are clearly a massive force multiplier for LLMs).
simonw 5mo ago
Yee, absolutely. A team with a strong code review culture that incorporates security review against common exploits ideally wouldn't end up with holes like this.
- ihaveajob 5mo ago
  I guess the value of the tool is that it gives you that same benefit for the cost of a few tokens.
  - tmoertel 5mo ago
    > I guess the value of the tool is that it gives you that same benefit for the cost of a few tokens. But it doesn't give you the same benefit. It gives you the partial benefit of catching these problems before they go to production, but it doesn't give you the remaining benefit of teaching your team about where their mental models are broken. A team that decides to delegate this responsibility entirely to AI is going to have a hard time learning about these serious defects in their mental models. Fixing those defects will pay dividends throughout the code base, not just in the places where AI would detect security failing.
    - ihaveajob 5mo ago
      Not if you treat it as a magical box that fixes things on its own. We're a tiny team and our process has improved a lot thanks to processing AI reviews and learning common patterns. It gets tiring to get the same feedback over and over so humans learn.
ulimn 5mo ago
Checking for OWASP top 10 items during code review is usually a mid level dev interview question IME. It's nothing new. Teams don't have to come up with these. These things exist.
Falell 5mo ago
I think it shows exactly the opposite of the second. Even with the availability of checklists, and instructions to use them, people won't and don't actually use them consistently. 'With enough eyes, all bugs are shallow' and AI is an automatable eye that looks at things we can tell nobody has seriously looked at before. It's not a panacea, there will be lots of false positives, but there's value there that we clearly aren't getting by 'just telling humans to use the tools available'. See also: modern practices and sanitizers and tools and test frameworks to avoid writing memory errors in C, and the reality that we keep writing memory errors in C.
- Avamander 5mo ago
  > See also: modern practices and sanitizers and tools and test frameworks to avoid writing memory errors in C, and the reality that we keep writing memory errors in C. I think there's a difference in how trivial some of these things are to detect and how difficult others are. IDOR and SQLi aren't nearly as complex as C unsafety is.
someguynamedq 5mo ago
"should" and yet didn't

dgb23 5mo ago

But by not having a checklist you avoid that your blind spots get exposed.

tmoertel 5mo ago
Why would you want to prevent your development team from learning about their blind spots?
- dylan604 5mo ago
  So you can move faster to the next features obviously. Refactoring for secure code is time consuming, and clearly wasted cycles as the code is working. /s
- dgb23 5mo ago
  I tried to make a joke about the tensions of security and accountability.

capiki 5mo ago

What about having the checklist and having an AI tool use it to catch things at review time (or even development time)?

tmoertel 5mo ago
Having AI tools do the review against the checklist would probably prevent the problems. However, it would probably be substantially inferior as a teaching tool for your team. The exercise of having reviewers hunt the checklisted vulnerabilities for themselves is what develops the mental muscles needed to understand the vulnerabilities in depth and avoid them when designing and writing future code. But, yes, I'd augment any manual review with a checklist and AI review as a final step. If the AI catches any problems then, your reviewers will be primed to think about why they overlooked them.
- dylan604 5mo ago
  > The exercise of having reviewers hunt the checklisted vulnerabilities for themselves is what develops the mental muscles needed to understand the vulnerabilities in depth and avoid them when designing and writing future code. Could not agree any more strongly. These automagic tools are one thing in the hands of a dev that groks the basics like these examples. It would be one thing if new devs were actually reviewing the generated code to understand it, but so much is just vibe coded and deployed as soon as it "works". I get flack from not immediately deploying generated code because I want to take time to understand how it works. It's really grating and a lot of friction is coming from it.
- capitalhilbilly 5mo ago
  For vulnerabilities of this nature is there really a point in training if an AI will catch them from now on? Seems like a variant of the allowing calculators problem and maybe the problem codeless platforms would have had. If these style of bugs don't change design in any meaningful way then the user can just write pseudo variables and the AI can normalize to safe code and their ability to work without the AI and IDE is probably less relevant than freeing their cognitive load for more complex constraint problems.
- ndriscoll 5mo ago
  Suppose we still need humans to be writing code and caring about this stuff for the foreseeable future, so we need people to continue learning about the ways things can go wrong. For something like injection, you still ideally have a lint rule that says "don't concatenate things that look like SQL/HTML/etc. Use the correct macros for string interpolation". What does it actually teach for a reviewer to tell you that? You can ask the reviewer for more information, but you can ask your teammate anyway if you don't understand why the linter is mad. You can also ask the robot, who will patiently explain it to you even long after all of the knowledgeable humans have retired or died. The robot could even link to a prompt asking to explain it: https://chatgpt.com/share/69f10515-8808-83ea-abe3-a758d3144cc9 https://chatgpt.com/share/69f10515-8808-83ea-abe3-a758d3144c... If people aren't learning more with AI, that's a meta skill they need to develop. As for training the review muscles, why would you do that if you have a linter that rejects when you make the mistake? I don't expect reviewers to check whether you eschew nulls or uninitialized variables; I expect the compiler to do that, and I expect over time that more and more things will become tooling concerns (especially given that rigid tools with appropriate feedback are clearly a massive force multiplier for LLMs).

simonw 5mo ago

Yee, absolutely. A team with a strong code review culture that incorporates security review against common exploits ideally wouldn't end up with holes like this.

ihaveajob 5mo ago
I guess the value of the tool is that it gives you that same benefit for the cost of a few tokens.
- tmoertel 5mo ago
  > I guess the value of the tool is that it gives you that same benefit for the cost of a few tokens. But it doesn't give you the same benefit. It gives you the partial benefit of catching these problems before they go to production, but it doesn't give you the remaining benefit of teaching your team about where their mental models are broken. A team that decides to delegate this responsibility entirely to AI is going to have a hard time learning about these serious defects in their mental models. Fixing those defects will pay dividends throughout the code base, not just in the places where AI would detect security failing.
  - ihaveajob 5mo ago
    Not if you treat it as a magical box that fixes things on its own. We're a tiny team and our process has improved a lot thanks to processing AI reviews and learning common patterns. It gets tiring to get the same feedback over and over so humans learn.

ulimn 5mo ago

Checking for OWASP top 10 items during code review is usually a mid level dev interview question IME. It's nothing new. Teams don't have to come up with these. These things exist.

Falell 5mo ago

I think it shows exactly the opposite of the second. Even with the availability of checklists, and instructions to use them, people won't and don't actually use them consistently. 'With enough eyes, all bugs are shallow' and AI is an automatable eye that looks at things we can tell nobody has seriously looked at before. It's not a panacea, there will be lots of false positives, but there's value there that we clearly aren't getting by 'just telling humans to use the tools available'. See also: modern practices and sanitizers and tools and test frameworks to avoid writing memory errors in C, and the reality that we keep writing memory errors in C.

Avamander 5mo ago
> See also: modern practices and sanitizers and tools and test frameworks to avoid writing memory errors in C, and the reality that we keep writing memory errors in C. I think there's a difference in how trivial some of these things are to detect and how difficult others are. IDOR and SQLi aren't nearly as complex as C unsafety is.

someguynamedq 5mo ago

"should" and yet didn't