9 ms·

The thing that concerns me most is looking at the fix it is very difficult to see why this fix is correct. It also appears as there is lots of code without expl

The thing that concerns me most is looking at the fix it is very difficult to see why this fix is correct. It also appears as there is lots of code without explicit bounds checks. It makes me worried because while the logic may be safe this makes the logic very complex. I wonder what the cost would be to add an explicit, local bounds check at every array access. This would serve as a backup that is much easier to verify. I suspect the cost would be relatively small. Small enough that I personally would be happy to pay it. https://github.com/webmproject/libwebp/commit/902bc9190331343b2017211debcec8d2ab87e17a https://github.com/webmproject/libwebp/commit/902bc919033134... This is also a great reminder that fuzzing isn't a solution to memory unsafe languages and libraries. If anything the massive amount of bugs found via fuzzing should scare us as it is likely only scratching the surface of the vulnerabilities that still lie in the code, a couple too many branches away from being likely to be found by fuzzing.

londons_explore 3y ago
Looks pretty correct to me... We just keep track of the table size and make it bigger if necessary. We additionally have a mode where we just calculate the necessary size, without writing any data structures. So we have kinda added double safety against this particular bug.
jsnell 3y ago
There was a great explanation a few days back on why the vulnerability existed, and on why the fix was correct: https://blog.isosceles.com/the-webp-0day/ https://blog.isosceles.com/the-webp-0day/ (Discussed in https://news.ycombinator.com/item?id=37600852 https://news.ycombinator.com/item?id=37600852)
- kevincox 3y ago
  I understand that the patch is correct. The point that I am trying to make is that the correctness of this code relies on non-local reasoning (the allocation/reallocation of the table before it is used). I believe that non-local reasoning raises the chance of error (and is likely one of the reasons why this bug existed in the first place). By adding length counts and explicit bounds checks locally you provide local reasoning to back up this non-local reasoning which is much *easier* to make and maintain correct. I think that would result in code that is less likely to have similar bugs in the future.
  - izacus 3y ago
    [flagged]
    - KirillPanov 3y ago
      ye gads man, did you even read the comment you're replying to?
      - izacus 3y ago
        I did - and I'm not sure how adding those checks here would help. Are you proposing a full library reaudit?
        unethical_ban 3y ago
        $ ./pedant.sh If you read what they proposed and disagreed with it, then don't ask "What do you propose?" - Argue/debate why you think their proposal, which you read, is not useful.
    - saghm 3y ago
      The best solution to this sort of bug is not using languages that are susceptible to this sort of bug. At the very least, I think it's time to retire the fallacy that we're generally capable of producing sound programs in memory unsafe languages. Just like we don't write code manually checking raw SQL to protect against injections and we don't roll our own crypto when we need to encrypt something or do a key exchange, we shouldn't treat writing safe code in C as something anyone can do by just being vaguely diligent.
      - izacus 3y ago
        You do understand that's not a practical solution here, do you?
        saghm 3y ago
        Implementing webp and similar libraries in another language may not be quick or easy, but having these sorts of issues continue to crop up over and over doesn't feel particularly practical to me; it's just easier to act like every time a critical vulnerability comes up due to memory safety that it's an isolated issue than confront the reality that its a systemic issue and there aren't any easy solutions.
winter_blue 3y ago
> ...isn't a solution to memory unsafe languages and libraries. If anything the massive amount of bugs found via fuzzing should scare us as it is likely only scratching the surface of the vulnerabilities that still lie in the code Yup. For example, the Linux code for its relatively new[1] io_uring subsystem was so memory-exploit-ridden that Google disabled it for apps on Android, and entirely on ChromeOS, and their servers[2]. It is insane that this is how easy it has been for a person to break into Linux. [1] released with kernel version 5.1 which came out in May 2019 [2] in June 2023: https://en.m.wikipedia.org/wiki/Io_uring#Security https://en.m.wikipedia.org/wiki/Io_uring#Security
- raverbashing 3y ago
  Only goes to show that C is a broken language and unfit for its purpose in today's day and age
  - Aeolos 3y ago
    For people downvoting, could you please explain which part of this statement you agree or disagree with?
    - mxkopy 3y ago
      [flagged]
      - raverbashing 3y ago
        > C is syntactic sugar around assembly It isn't As an example, see how C 'int', 'short', 'long' or whatever becomes actual different types > just admit you're not interested in computers I have the popcorn ready already for when the next C "hotshot" shoots himself in the foot yet again because he thinks a chainsaw without an emergency brake is just more fun
        peteradio 3y ago
        > chainsaw without an emergency brake is just more fun Fun and fast!
lukeschlather 3y ago
This is the sort of thing where I am very curious to hear what happens if you fine-tune an uncensored version of GPT4 on some memory exploits like this along with the vulnerable code and ask it for more. It seems like it ought to be really good at this and I am suspicious that people in the know are afraid to talk about it publicly because it's too good at it and once people start weaponizing LLMs for this purpose we just won't be able to use memory unsafe code anymore.
- Zigurd 3y ago
  LLMs should indeed be very good at finding vulns. It is a safe bet state actors have already done this, and have added to their stockpile of zero-day exploits. I suspect you are right that collectively we are whistling past the graveyard and the current "solution" is to make widely available LLMs explicitly not provide that kind of capability.
  - deleted 3y ago
    [deleted]
  - nahsra 3y ago
    My experience is they're not good at this for most vulnerability classes, especially the those that are tough to discover by classical methods. Have you had any experience using them for this? Trivial vulnerabilities are easily discoverable yes -- but, they are also trivially discoverable by standard automation available today. I've found GPT-4 to be shockingly bad at vulnerability analysis for all except the most popular vulnerability classes. My speculation is that there just isn't enough literature on these vulnerability classes for it to have practical mastery of them. Complex vulnerabilities are the emergent phenomena of multiple events across a codebase and it's dependencies, involving control flow, data flow, while missing type information and other runtime data. Even Anthropic's 100K context windows won't nearly fit it all, and if you stuff all the code into embeddings, the ability to reason across all this space will be poor. You can train a model to ask very pointed questions about particular snippets, but wholesale LLM-based analysis to find vulnerabilities seems like it'll be extremely slow, expensive and inaccurate.
toomuchtodo 3y ago
This is why I'm bullish on LLMs for application security vulnerability searches (like Shodan for code). It'll see what humans don't.
- malfist 3y ago
  Why would you expect that? LLMs are explicitly trained on what humans do. Humans miss security vulnerabilities all the time.
  - toomuchtodo 3y ago
    Because the LLM based appsec products I've tested on a large code base have been very good at finding what devs and appsec code reviews didn't.
    - reitzensteinm 3y ago
      Which ones, out of curiosity?
  - dnedic 3y ago
    Humans can't be forced to evaluate every piece of code 100000 times in an hour. LLMs can.
    - philipwhiuk 3y ago
      With infinite hardware, sure. LLMs are pretty expensive to run vs fuzzing.
  - ZiiS 3y ago
    Humans find security vulnerabilities all the time too. The assumption is a lot of the misses are because highly trained security experts don't have time to carefully read and understand all code without fatigue.
  - xeckr 3y ago
    Because of generalization.
mlichvar 3y ago
Fuzzing needs to cover all important bits of the code to be useful. The problem I see is that incomplete coverage creates a false sense of security. Projects have some minimal fuzzing coverage (e.g. in oss-fuzz) and care less about quality of the code, thinking fuzzing will catch all security bugs. Rust code needs proper fuzzing too. It takes a lot of effort to ensure everything is covered and stays covered as the code is developed. Crashing libraries or applications can be a denial of service. Sure, it's lower impact than an RCE due to a buffer overflow, but it is still a security issue.
- UncleMeat 3y ago
  This isn't even the problem. There are shitloads of open bugs found by fuzzing, even in the linux kernel, that nobody is fixing. Serious security vulns in the kernel regress because of lack of tests. It is clear that even if we stood up the best bug finding systems the world has ever seen that critical software will still be a disaster.

londons_explore 3y ago

Looks pretty correct to me... We just keep track of the table size and make it bigger if necessary. We additionally have a mode where we just calculate the necessary size, without writing any data structures. So we have kinda added double safety against this particular bug.

jsnell 3y ago

There was a great explanation a few days back on why the vulnerability existed, and on why the fix was correct: https://blog.isosceles.com/the-webp-0day/ https://blog.isosceles.com/the-webp-0day/ (Discussed in https://news.ycombinator.com/item?id=37600852 https://news.ycombinator.com/item?id=37600852)

kevincox 3y ago
I understand that the patch is correct. The point that I am trying to make is that the correctness of this code relies on non-local reasoning (the allocation/reallocation of the table before it is used). I believe that non-local reasoning raises the chance of error (and is likely one of the reasons why this bug existed in the first place). By adding length counts and explicit bounds checks locally you provide local reasoning to back up this non-local reasoning which is much *easier* to make and maintain correct. I think that would result in code that is less likely to have similar bugs in the future.
- izacus 3y ago
  [flagged]
  - KirillPanov 3y ago
    ye gads man, did you even read the comment you're replying to?
    - izacus 3y ago
      I did - and I'm not sure how adding those checks here would help. Are you proposing a full library reaudit?
      - unethical_ban 3y ago
        $ ./pedant.sh If you read what they proposed and disagreed with it, then don't ask "What do you propose?" - Argue/debate why you think their proposal, which you read, is not useful.
  - saghm 3y ago
    The best solution to this sort of bug is not using languages that are susceptible to this sort of bug. At the very least, I think it's time to retire the fallacy that we're generally capable of producing sound programs in memory unsafe languages. Just like we don't write code manually checking raw SQL to protect against injections and we don't roll our own crypto when we need to encrypt something or do a key exchange, we shouldn't treat writing safe code in C as something anyone can do by just being vaguely diligent.
    - izacus 3y ago
      You do understand that's not a practical solution here, do you?
      - saghm 3y ago
        Implementing webp and similar libraries in another language may not be quick or easy, but having these sorts of issues continue to crop up over and over doesn't feel particularly practical to me; it's just easier to act like every time a critical vulnerability comes up due to memory safety that it's an isolated issue than confront the reality that its a systemic issue and there aren't any easy solutions.

winter_blue 3y ago

> ...isn't a solution to memory unsafe languages and libraries. If anything the massive amount of bugs found via fuzzing should scare us as it is likely only scratching the surface of the vulnerabilities that still lie in the code Yup. For example, the Linux code for its relatively new[1] io_uring subsystem was so memory-exploit-ridden that Google disabled it for apps on Android, and entirely on ChromeOS, and their servers[2]. It is insane that this is how easy it has been for a person to break into Linux. [1] released with kernel version 5.1 which came out in May 2019 [2] in June 2023: https://en.m.wikipedia.org/wiki/Io_uring#Security https://en.m.wikipedia.org/wiki/Io_uring#Security

raverbashing 3y ago
Only goes to show that C is a broken language and unfit for its purpose in today's day and age
- Aeolos 3y ago
  For people downvoting, could you please explain which part of this statement you agree or disagree with?
  - mxkopy 3y ago
    [flagged]
    - raverbashing 3y ago
      > C is syntactic sugar around assembly It isn't As an example, see how C 'int', 'short', 'long' or whatever becomes actual different types > just admit you're not interested in computers I have the popcorn ready already for when the next C "hotshot" shoots himself in the foot yet again because he thinks a chainsaw without an emergency brake is just more fun
      - peteradio 3y ago
        > chainsaw without an emergency brake is just more fun Fun and fast!

lukeschlather 3y ago

This is the sort of thing where I am very curious to hear what happens if you fine-tune an uncensored version of GPT4 on some memory exploits like this along with the vulnerable code and ask it for more. It seems like it ought to be really good at this and I am suspicious that people in the know are afraid to talk about it publicly because it's too good at it and once people start weaponizing LLMs for this purpose we just won't be able to use memory unsafe code anymore.

Zigurd 3y ago
LLMs should indeed be very good at finding vulns. It is a safe bet state actors have already done this, and have added to their stockpile of zero-day exploits. I suspect you are right that collectively we are whistling past the graveyard and the current "solution" is to make widely available LLMs explicitly not provide that kind of capability.
- deleted 3y ago
  [deleted]
- nahsra 3y ago
  My experience is they're not good at this for most vulnerability classes, especially the those that are tough to discover by classical methods. Have you had any experience using them for this? Trivial vulnerabilities are easily discoverable yes -- but, they are also trivially discoverable by standard automation available today. I've found GPT-4 to be shockingly bad at vulnerability analysis for all except the most popular vulnerability classes. My speculation is that there just isn't enough literature on these vulnerability classes for it to have practical mastery of them. Complex vulnerabilities are the emergent phenomena of multiple events across a codebase and it's dependencies, involving control flow, data flow, while missing type information and other runtime data. Even Anthropic's 100K context windows won't nearly fit it all, and if you stuff all the code into embeddings, the ability to reason across all this space will be poor. You can train a model to ask very pointed questions about particular snippets, but wholesale LLM-based analysis to find vulnerabilities seems like it'll be extremely slow, expensive and inaccurate.

toomuchtodo 3y ago

This is why I'm bullish on LLMs for application security vulnerability searches (like Shodan for code). It'll see what humans don't.

malfist 3y ago
Why would you expect that? LLMs are explicitly trained on what humans do. Humans miss security vulnerabilities all the time.
- toomuchtodo 3y ago
  Because the LLM based appsec products I've tested on a large code base have been very good at finding what devs and appsec code reviews didn't.
  - reitzensteinm 3y ago
    Which ones, out of curiosity?
- dnedic 3y ago
  Humans can't be forced to evaluate every piece of code 100000 times in an hour. LLMs can.
  - philipwhiuk 3y ago
    With infinite hardware, sure. LLMs are pretty expensive to run vs fuzzing.
- ZiiS 3y ago
  Humans find security vulnerabilities all the time too. The assumption is a lot of the misses are because highly trained security experts don't have time to carefully read and understand all code without fatigue.
- xeckr 3y ago
  Because of generalization.

mlichvar 3y ago

Fuzzing needs to cover all important bits of the code to be useful. The problem I see is that incomplete coverage creates a false sense of security. Projects have some minimal fuzzing coverage (e.g. in oss-fuzz) and care less about quality of the code, thinking fuzzing will catch all security bugs. Rust code needs proper fuzzing too. It takes a lot of effort to ensure everything is covered and stays covered as the code is developed. Crashing libraries or applications can be a denial of service. Sure, it's lower impact than an RCE due to a buffer overflow, but it is still a security issue.

UncleMeat 3y ago
This isn't even the problem. There are shitloads of open bugs found by fuzzing, even in the linux kernel, that nobody is fixing. Serious security vulns in the kernel regress because of lack of tests. It is clear that even if we stood up the best bug finding systems the world has ever seen that critical software will still be a disaster.