7 ms·

Hubris and people thinking they can safely code in memory unsafe languages (they can't)

by ojii 3y ago

delfinom 3y ago
Not to worry, there are serialization and other vulnerabilities in memory safe languages :3
- JulianWasTaken 3y ago
  Eliminating a whole class of vulnerabilities is the right thing to do regardless of whether other classes of vulnerabilities remain.
- unaindz 3y ago
  In both safe and unsafe languages
- johnnyjeans 3y ago
  The solution, if you can consider there to be one, was always languages with semantics to guarantee logical correctness. But we're never going to get that because it requires a pace of software development that's incompatible with the money people's fetish for churn and burn. So let's put all our eggs into the Rust basket and while we're at it trust the hardware to never lead us astray :)
- himinlomax 3y ago
  I never lock my door because burglars can break a window.
BeFlatXIII 3y ago
But we need the performance of hand-tuning memory to save the environment and provide accessibility to legacy devices! /s
ninkendo 3y ago
A memory-safe language could have easily resulted in the same vulnerability. In this context, “memory-safe” means it does bounds checking on an array when you try to access an element. But the webp code does bounds checks up-front so that array accesses can be non-checked, to help performance. (If they didn’t want this performance, they could have easily used a std::vector and used bounds-checked access.) The vulnerability happened because this up-front bounds checking logic was incorrect. If we were to hypothesize a counterfactual world where webp was written in rust, presumably the devs would have wanted a similar optimization where they did the bounds checking up-front still, and they would have put the actual array access in an unsafe block to have the same perf optimization. The same bug would have thus happened. The lesson here is that bounds checking on every element access is probably a worthwhile overhead, no matter what the language is. C++ has safe ways to do this (well, safe-ish… safe for the purposes of this bug at least) with std::vector, and maybe they should just switch to that and eat the performance overhead.
- howinteresting 3y ago
  While yes, that's theoretically possible, do you have data to establish this? For example, having small sections of the code be marked as unsafe would allow for greater scrutiny of those sections. Also, unsafe access is more annoying to perform in Rust than in C or C++, so maybe that would have acted as a deterrent (or at least the code would have been profiled to make sure that unsafe access was worth it). https://security.googleblog.com/2022/12/memory-safe-languages-in-android-13.html https://security.googleblog.com/2022/12/memory-safe-language... shows improvements at scale.
  - ninkendo 3y ago
    Elsewhere in the comments someone linked to this: https://dropbox.tech/infrastructure/lossless-compression-with-brotli https://dropbox.tech/infrastructure/lossless-compression-wit... It looks like dropbox experimented with disabling bounds checks in their huffman coding impl, and found that using the unsafe pattern increased throughput from 224 MB/s to 249 MB/s (11%-ish faster.) We don’t even need to hypothesize about whether webp would have elminated bounds checking, we can see that other companies arrived at the same conclusion: Disabling it can be worth it if you’re quite sure you’ve gotten the up-front checking right. We can imagine that if Dropbox went to prod with the unchecked huffman implementation (never mind that that article isn’t about webp in particular), we could imagine they could easily have the same bug. And I don’t think a naive code review saying “unsafe is bad” would have stopped them from doing it: they clearly did the work to show why it’s worth it.
    - howinteresting 3y ago
      Thank you.
    - deleted 3y ago
      [deleted]
    - hot_gril 3y ago
      The risk probably doesn't matter for their use case because they're doing all this datacenter-scale image conversion on processes separate from the main logic (and likely not even on the same machines). Unlike in a phone's web browser or something, where it's 11% speedup with ??% added risk. It's already common for PC apps to split potentially unsafe rendering into subprocesses, like in Chrome. If you don't want to pay the full IPC toll, there's shared memory. In theory should be about the same speed as inlined unsafe code, right? What if Rust's "unsafe" blocks could do this for you?

Hubris and people thinking they can safely code in memory unsafe languages (they can't)

by ojii 3y ago

Hubris and people thinking they can safely code in memory unsafe languages (they can't)

delfinom 3y ago

Not to worry, there are serialization and other vulnerabilities in memory safe languages :3

JulianWasTaken 3y ago
Eliminating a whole class of vulnerabilities is the right thing to do regardless of whether other classes of vulnerabilities remain.
unaindz 3y ago
In both safe and unsafe languages
johnnyjeans 3y ago
The solution, if you can consider there to be one, was always languages with semantics to guarantee logical correctness. But we're never going to get that because it requires a pace of software development that's incompatible with the money people's fetish for churn and burn. So let's put all our eggs into the Rust basket and while we're at it trust the hardware to never lead us astray :)
himinlomax 3y ago
I never lock my door because burglars can break a window.

BeFlatXIII 3y ago

But we need the performance of hand-tuning memory to save the environment and provide accessibility to legacy devices! /s

ninkendo 3y ago

A memory-safe language could have easily resulted in the same vulnerability. In this context, “memory-safe” means it does bounds checking on an array when you try to access an element. But the webp code does bounds checks up-front so that array accesses can be non-checked, to help performance. (If they didn’t want this performance, they could have easily used a std::vector and used bounds-checked access.) The vulnerability happened because this up-front bounds checking logic was incorrect. If we were to hypothesize a counterfactual world where webp was written in rust, presumably the devs would have wanted a similar optimization where they did the bounds checking up-front still, and they would have put the actual array access in an unsafe block to have the same perf optimization. The same bug would have thus happened. The lesson here is that bounds checking on every element access is probably a worthwhile overhead, no matter what the language is. C++ has safe ways to do this (well, safe-ish… safe for the purposes of this bug at least) with std::vector, and maybe they should just switch to that and eat the performance overhead.

howinteresting 3y ago
While yes, that's theoretically possible, do you have data to establish this? For example, having small sections of the code be marked as unsafe would allow for greater scrutiny of those sections. Also, unsafe access is more annoying to perform in Rust than in C or C++, so maybe that would have acted as a deterrent (or at least the code would have been profiled to make sure that unsafe access was worth it). https://security.googleblog.com/2022/12/memory-safe-languages-in-android-13.html https://security.googleblog.com/2022/12/memory-safe-language... shows improvements at scale.
- ninkendo 3y ago
  Elsewhere in the comments someone linked to this: https://dropbox.tech/infrastructure/lossless-compression-with-brotli https://dropbox.tech/infrastructure/lossless-compression-wit... It looks like dropbox experimented with disabling bounds checks in their huffman coding impl, and found that using the unsafe pattern increased throughput from 224 MB/s to 249 MB/s (11%-ish faster.) We don’t even need to hypothesize about whether webp would have elminated bounds checking, we can see that other companies arrived at the same conclusion: Disabling it can be worth it if you’re quite sure you’ve gotten the up-front checking right. We can imagine that if Dropbox went to prod with the unchecked huffman implementation (never mind that that article isn’t about webp in particular), we could imagine they could easily have the same bug. And I don’t think a naive code review saying “unsafe is bad” would have stopped them from doing it: they clearly did the work to show why it’s worth it.
  - howinteresting 3y ago
    Thank you.
  - deleted 3y ago
    [deleted]
  - hot_gril 3y ago
    The risk probably doesn't matter for their use case because they're doing all this datacenter-scale image conversion on processes separate from the main logic (and likely not even on the same machines). Unlike in a phone's web browser or something, where it's 11% speedup with ??% added risk. It's already common for PC apps to split potentially unsafe rendering into subprocesses, like in Chrome. If you don't want to pay the full IPC toll, there's shared memory. In theory should be about the same speed as inlined unsafe code, right? What if Rust's "unsafe" blocks could do this for you?