3 ms·

Unsafe is hardly a bigger red flag than mmap'ing an opaque blob with PROT_EXEC and then calling a function in it. But the entire pretense of such an attack is t

by duped 3y ago

Unsafe is hardly a bigger red flag than mmap'ing an opaque blob with PROT_EXEC and then calling a function in it. But the entire pretense of such an attack is that you're not looking at the malicious code. Unsafe is not really a red flag, except for people who don't understand what unsafe means. And like you point out, if you see it, you need to audit the code regardless.

dgacmu 3y ago
These attacks would be very noticeable in a crate that was not already using unsafe. That's a pretty reasonable start, at least - and it gives a little more benefit to reducing the number of unsafe-using crates you pull in. (For non-rust folks, using mmap requires using unsafe, so you can't map your own machine code in without being explicit about unsafe)
- duped 3y ago
  The issue isn't `mmap`, that's just a contrived example to show that scanning source code isn't enough to stop an attacker from adding malicious code to the supply chain. Forbidding unsafe is a big gun. You need unsafe code for any FFI, so attackers will just look for crates that link to system libraries and add their malicious code there. If you want to do any kind of fast iteration over slices/vectors you need to use unsafe to explicitly elide bounds checks. I think the real nugget of truth is that the vulnerability is `cargo update` and that tooling needs to look at changes that happen between versions, and alerting to new unsafe or extern bindings is a good way to do that. But I still see a few obvious ways around it - what you really want is a linker shim that can detect when symbols are being relocated into the final executable that shouldn't be there.
  - dgacmu 3y ago
    Absolutely. Crates that already use unsafe will be a more attractive target. Most of the ways to sneak arbitrary code execution into rust require unsafe. As I said, it helps. It's not perfect. But there are a lot of crates out there that don't use unsafe, and having a way to focus your attention is useful. Looking at some of my own projects, for example, I notice that I often use crates like "env_logger" with 3.9M downloads. What an absolutely magical target for a supply chain attack -- but it doesn't use unsafe, or net, or tokio, or a lot of other things, and pinning that restriction with Cackle would let me update it with much less worry. I kinda like that. I'm really not arguing that this is a silver bullet. It's just .. nice. Edited to add, as a counterpoint: The "log" crate, which is used by env_logger, _does_ use unsafe. Great target for a supply chain attack -- and perhaps reason to nudge the developers to find ways to get rid of it!
    - duped 3y ago
      > and perhaps reason to nudge the developers to find ways to get rid of it! Or perhaps nudge users to understand what `unsafe` means and why its necessary rather than evangelize at maintainers.
      - dgacmu 3y ago
        Nah. The log crate primarily uses unsafe to poke at pointers in a probably-ok-but-racy way on platforms that don't support atomics. It would almost certainly be better to use a platform-appropriate mutex for those limited contexts, or at least allow the presumably-faster, presumably-benign race to be hidden behind a feature flag. I'm quite familiar with what unsafe means. And in the context of supply chain attacks, there's a clear benefit from going from "a few uses of unsafe as a possible performance optimization" to "no unsafe". Obviously, that doesn't work for all crates and we shouldn't demand it. But I made the comment about log after looking at the code to understand its use of unsafe.
Xeamek 3y ago
Except now You only need to audition the Unsafe part, which should, and usually is being utilized to minimum, rather then the entire project.
- galangalalgol 3y ago
  And even if the crate with ffi isn't compromised, they are the most likely spots for a cve anyway. Openssl and libcurl bindings for instance. So we should be paying attention to them anyway. I always prefer a pure safe rust crate for that reason, and because it is easier to deploy as a from scratch container or stand alone binary built against musl. Openssl and libcurl have permissive licenses so they are statically linked anyway, and there are no other options of course.

by duped 3y ago

dgacmu 3y ago

These attacks would be very noticeable in a crate that was not already using unsafe. That's a pretty reasonable start, at least - and it gives a little more benefit to reducing the number of unsafe-using crates you pull in. (For non-rust folks, using mmap requires using unsafe, so you can't map your own machine code in without being explicit about unsafe)

duped 3y ago
The issue isn't `mmap`, that's just a contrived example to show that scanning source code isn't enough to stop an attacker from adding malicious code to the supply chain. Forbidding unsafe is a big gun. You need unsafe code for any FFI, so attackers will just look for crates that link to system libraries and add their malicious code there. If you want to do any kind of fast iteration over slices/vectors you need to use unsafe to explicitly elide bounds checks. I think the real nugget of truth is that the vulnerability is `cargo update` and that tooling needs to look at changes that happen between versions, and alerting to new unsafe or extern bindings is a good way to do that. But I still see a few obvious ways around it - what you really want is a linker shim that can detect when symbols are being relocated into the final executable that shouldn't be there.
- dgacmu 3y ago
  Absolutely. Crates that already use unsafe will be a more attractive target. Most of the ways to sneak arbitrary code execution into rust require unsafe. As I said, it helps. It's not perfect. But there are a lot of crates out there that don't use unsafe, and having a way to focus your attention is useful. Looking at some of my own projects, for example, I notice that I often use crates like "env_logger" with 3.9M downloads. What an absolutely magical target for a supply chain attack -- but it doesn't use unsafe, or net, or tokio, or a lot of other things, and pinning that restriction with Cackle would let me update it with much less worry. I kinda like that. I'm really not arguing that this is a silver bullet. It's just .. nice. Edited to add, as a counterpoint: The "log" crate, which is used by env_logger, _does_ use unsafe. Great target for a supply chain attack -- and perhaps reason to nudge the developers to find ways to get rid of it!
  - duped 3y ago
    > and perhaps reason to nudge the developers to find ways to get rid of it! Or perhaps nudge users to understand what `unsafe` means and why its necessary rather than evangelize at maintainers.
    - dgacmu 3y ago
      Nah. The log crate primarily uses unsafe to poke at pointers in a probably-ok-but-racy way on platforms that don't support atomics. It would almost certainly be better to use a platform-appropriate mutex for those limited contexts, or at least allow the presumably-faster, presumably-benign race to be hidden behind a feature flag. I'm quite familiar with what unsafe means. And in the context of supply chain attacks, there's a clear benefit from going from "a few uses of unsafe as a possible performance optimization" to "no unsafe". Obviously, that doesn't work for all crates and we shouldn't demand it. But I made the comment about log after looking at the code to understand its use of unsafe.

Xeamek 3y ago

Except now You only need to audition the Unsafe part, which should, and usually is being utilized to minimum, rather then the entire project.

galangalalgol 3y ago
And even if the crate with ffi isn't compromised, they are the most likely spots for a cve anyway. Openssl and libcurl bindings for instance. So we should be paying attention to them anyway. I always prefer a pure safe rust crate for that reason, and because it is easier to deploy as a from scratch container or stand alone binary built against musl. Openssl and libcurl have permissive licenses so they are statically linked anyway, and there are no other options of course.