4 ms·

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:

by 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.
    - secondcoming 3y ago
      This response is becoming so tiresome
      - kouteiheika 3y ago
        > This response is becoming so tiresome I'd argue the constant stream of CVEs due to memory unsafety is even more tiresome.
        saghm 3y ago
        Agreed, I'll stop saying it when we stop having these issues. I don't think people want to confront the fact that there isn't any easier solution, but we're going to keep running into the same thing over and over until we do.
- pclmulqdq 3y ago
  Over a library used by billions of people a lot of times a day, those bounds checks add up to a lot of wasted time and energy if you don't strictly need them. If this is a common function in the webp library, this function is probably called something like a trillion times a day.
  - kevincox 3y ago
    They aren't wasted if they are preventing security vulnerabilities. I also wouldn't be surprised if patching, rebuilding and distributing this fixed version of libwebp to all of these devices would be comparable to the extra cost of decoding.
    - pclmulqdq 3y ago
      "Preventing security vulnerabilities" is not an unadulturated good (the most secure computer does nothing but checking to make sure it hasn't been breached), and any bounds check that does not actively prevent an intrusion is wasted from a value perspective. Your intuition about the cost of a patch vs the extra instructions in the hot path to check bounds everywhere is also very much wrong: the cost of a patch (which is also amortized among many fixes) is not even close. The hot path adds up when you call it a lot.
      - saagarjha 3y ago
        The most secure computer does nothing, because checking if it has been breached is attack surface.
  - ctz 3y ago
    I think your intuition on the cost of not-taken bounds check branches is off, especially in the context of devices that spend most of their resources browsing the web, running radios, powering screen backlights. I bet any holistic measurement could not tell the difference.
    - kps 3y ago

by jsnell 3y ago

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.
  - secondcoming 3y ago
    This response is becoming so tiresome
    - kouteiheika 3y ago
      > This response is becoming so tiresome I'd argue the constant stream of CVEs due to memory unsafety is even more tiresome.
      - saghm 3y ago
        Agreed, I'll stop saying it when we stop having these issues. I don't think people want to confront the fact that there isn't any easier solution, but we're going to keep running into the same thing over and over until we do.
pclmulqdq 3y ago
Over a library used by billions of people a lot of times a day, those bounds checks add up to a lot of wasted time and energy if you don't strictly need them. If this is a common function in the webp library, this function is probably called something like a trillion times a day.
- kevincox 3y ago
  They aren't wasted if they are preventing security vulnerabilities. I also wouldn't be surprised if patching, rebuilding and distributing this fixed version of libwebp to all of these devices would be comparable to the extra cost of decoding.
  - pclmulqdq 3y ago
    "Preventing security vulnerabilities" is not an unadulturated good (the most secure computer does nothing but checking to make sure it hasn't been breached), and any bounds check that does not actively prevent an intrusion is wasted from a value perspective. Your intuition about the cost of a patch vs the extra instructions in the hot path to check bounds everywhere is also very much wrong: the cost of a patch (which is also amortized among many fixes) is not even close. The hot path adds up when you call it a lot.
    - saagarjha 3y ago
      The most secure computer does nothing, because checking if it has been breached is attack surface.
- ctz 3y ago
  I think your intuition on the cost of not-taken bounds check branches is off, especially in the context of devices that spend most of their resources browsing the web, running radios, powering screen backlights. I bet any holistic measurement could not tell the difference.
  - kps 3y ago