4 ms·

A good opportunity to check how https://github.com/PistonDevelopers/image-png https://github.com/PistonDevelopers/image-png is doing (a PNG decoder written in R

A good opportunity to check how https://github.com/PistonDevelopers/image-png https://github.com/PistonDevelopers/image-png is doing (a PNG decoder written in Rust). Looks like it includes bindings to use miniz.c for DEFLATE decoding as well as "inflate" (which seems to be DEFLATE in Rust). Also, it seems to have a fuzzing driver (png-afl). Good times!

mike_hearn 11y ago
There are PNG decoders written in a bunch of safe languages. For instance the JVM uses a Java PNG decoder. So it isn't vulnerable.
- kibwen 11y ago
  What killercup might be implying is that Rust is both a "safe language" and that Rust code can expose a C-compatible ABI, which means that a library written in Rust could theoretically replace one written in C regardless of which other language is ultimately making use of it. For example, this is what Mozilla is working on doing in Firefox by replacing security-conscious components with Rust implementations ( https://bugzilla.mozilla.org/show_bug.cgi?id=1151899 https://bugzilla.mozilla.org/show_bug.cgi?id=1151899 , https://bugzilla.mozilla.org/show_bug.cgi?id=1161350 https://bugzilla.mozilla.org/show_bug.cgi?id=1161350 ). However, I don't know if piston-image specifically provides such a C interface.
  - tedunangst 11y ago
    However, if the bug is that the library writes to a too small allocation by the application (based on a lie told it by the library) then it doesn't much matter what language the library is in.
    - deleted 11y ago
      [deleted]
    - kibwen 11y ago
      I see there are some downvotes here, which I don't think are deserved. It's true that at some level of your stack you're going to have some code that's just poking bytes into buffers, and that generally takes some manual effort to verify. For what it's worth, I'd expect this sort of thing to be walled up behind an `unsafe` block in Rust, which if nothing else would lend increased scrutiny in an audit.
      - agwa 11y ago
        I believe the point tedunangst is trying to make is that if the API works something like this: size_t sz = lib_get_buffer_size(); char *buf = malloc(sz); lib_fill_buffer(buf); and there's a bug in lib_get_buffer_size that causes it to return too small a number, but lib_fill_buffer assumes it's big enough, you've got a vulnerability regardless of what language the library is written in. This isn't a bug in the application, because it's doing what it was told to do. It will be hard to spot in an audit of the library because the bug is in lib_get_buffer_size, which probably contains no unsafe blocks, while lib_fill_buffer probably looks fine. It's a deficiency in the API, which should require the buffer size to be passed to lib_fill_buffer so lib_fill_buffer doesn't have to make any assumptions about the size. But if you're trying to preserve compatibility with existing C APIs, you might be stuck with APIs like this.
        kibwen 11y ago
        To clarify further, the idiomatic usage of `unsafe` in Rust stipulates that if you can't guarantee that your function is memory-safe for all possible inputs, then you must mark the function itself as `unsafe` to force callers to be aware of the risk. Obviously if you're both calling this theoretical function from a language without an `unsafe` construct and if you're also striving to maintain exact API compatibility with the C function then you can't really make this aware to the caller. If you do have control of the API then the way that this would generally be presented on the Rust side would be to have two functions: a safe one named "foo" that also takes the length as an argument so that you can check at runtime and an unsafe function named "unsafe_foo" that has the same behavior as the C function.
Asbostos 11y ago
Why on earth isn't this common practice for any software that can be remotely controlled over the internet? Ie anything rendering parts of web pages, opening untrusted files, etc? Everyone has known what a massive security risk unsafe languages are for a long time. Nearly every vulnerability is a buffer overflow. What's the value in persisting in writing such dangerous code? Just because it's 10% faster than a safer language? Hopefully the likes of Rust bring and end to this. Perhaps there's a culture of C++ being the only proper language for libraries?
- technion 11y ago
  Libpng isn't exactly new. libpng.org seems to be offline at the moment so I'm not having a lot of success finding details - but it's safe bet that it predates Rust, afl-fuzz, and probably even clang's static analyser. I'm all for saying "now is the time to rewrite ", but it's hard to get upset at that project when anyone else could have gone and done a modern take on the project - I hadn't heard of the earlier quoted Rust PNG library until this happened.
  - cesarb 11y ago
    > but it's safe bet that it predates Rust, afl-fuzz, and probably even clang's static analyser. Not only it predates clang's static analyser, it predates clang itself! The site is now online, and I can see on it news about libpng from last century (1995 to be more exact).

mike_hearn 11y ago

There are PNG decoders written in a bunch of safe languages. For instance the JVM uses a Java PNG decoder. So it isn't vulnerable.

kibwen 11y ago
What killercup might be implying is that Rust is both a "safe language" and that Rust code can expose a C-compatible ABI, which means that a library written in Rust could theoretically replace one written in C regardless of which other language is ultimately making use of it. For example, this is what Mozilla is working on doing in Firefox by replacing security-conscious components with Rust implementations ( https://bugzilla.mozilla.org/show_bug.cgi?id=1151899 https://bugzilla.mozilla.org/show_bug.cgi?id=1151899 , https://bugzilla.mozilla.org/show_bug.cgi?id=1161350 https://bugzilla.mozilla.org/show_bug.cgi?id=1161350 ). However, I don't know if piston-image specifically provides such a C interface.
- tedunangst 11y ago
  However, if the bug is that the library writes to a too small allocation by the application (based on a lie told it by the library) then it doesn't much matter what language the library is in.
  - deleted 11y ago
    [deleted]
  - kibwen 11y ago
    I see there are some downvotes here, which I don't think are deserved. It's true that at some level of your stack you're going to have some code that's just poking bytes into buffers, and that generally takes some manual effort to verify. For what it's worth, I'd expect this sort of thing to be walled up behind an `unsafe` block in Rust, which if nothing else would lend increased scrutiny in an audit.
    - agwa 11y ago
      I believe the point tedunangst is trying to make is that if the API works something like this: size_t sz = lib_get_buffer_size(); char *buf = malloc(sz); lib_fill_buffer(buf); and there's a bug in lib_get_buffer_size that causes it to return too small a number, but lib_fill_buffer assumes it's big enough, you've got a vulnerability regardless of what language the library is written in. This isn't a bug in the application, because it's doing what it was told to do. It will be hard to spot in an audit of the library because the bug is in lib_get_buffer_size, which probably contains no unsafe blocks, while lib_fill_buffer probably looks fine. It's a deficiency in the API, which should require the buffer size to be passed to lib_fill_buffer so lib_fill_buffer doesn't have to make any assumptions about the size. But if you're trying to preserve compatibility with existing C APIs, you might be stuck with APIs like this.
      - kibwen 11y ago
        To clarify further, the idiomatic usage of `unsafe` in Rust stipulates that if you can't guarantee that your function is memory-safe for all possible inputs, then you must mark the function itself as `unsafe` to force callers to be aware of the risk. Obviously if you're both calling this theoretical function from a language without an `unsafe` construct and if you're also striving to maintain exact API compatibility with the C function then you can't really make this aware to the caller. If you do have control of the API then the way that this would generally be presented on the Rust side would be to have two functions: a safe one named "foo" that also takes the length as an argument so that you can check at runtime and an unsafe function named "unsafe_foo" that has the same behavior as the C function.

Asbostos 11y ago

Why on earth isn't this common practice for any software that can be remotely controlled over the internet? Ie anything rendering parts of web pages, opening untrusted files, etc? Everyone has known what a massive security risk unsafe languages are for a long time. Nearly every vulnerability is a buffer overflow. What's the value in persisting in writing such dangerous code? Just because it's 10% faster than a safer language? Hopefully the likes of Rust bring and end to this. Perhaps there's a culture of C++ being the only proper language for libraries?

technion 11y ago
Libpng isn't exactly new. libpng.org seems to be offline at the moment so I'm not having a lot of success finding details - but it's safe bet that it predates Rust, afl-fuzz, and probably even clang's static analyser. I'm all for saying "now is the time to rewrite ", but it's hard to get upset at that project when anyone else could have gone and done a modern take on the project - I hadn't heard of the earlier quoted Rust PNG library until this happened.
- cesarb 11y ago
  > but it's safe bet that it predates Rust, afl-fuzz, and probably even clang's static analyser. Not only it predates clang's static analyser, it predates clang itself! The site is now online, and I can see on it news about libpng from last century (1995 to be more exact).