4 ms·

I don't think this meets the definition of "safe" in "safe" Rust: "safe" doesn't just mean "won't crash due to spatial memory errors," it means that the code is

I don't think this meets the definition of "safe" in "safe" Rust: "safe" doesn't just mean "won't crash due to spatial memory errors," it means that the code is in fact spatially and temporally memory safe. In other words: this won't detect memory unsafety that doesn't result in an abnormal exit or other detectable fault. If I'm writing an exploit, my entire goal is to perform memory corruption without causing a fault; that's why Rust's safety property is much stronger than crash-freeness.

NoahKAndrews 2y ago
It's not just that it won't crash, it means that an exploit in the unsafe code won't allow corrupting memory used by the rest of the program
- woodruffw 2y ago
  This is pretty immaterial from an exploit development perspective: 1. The forked process has a copy of the program state. If I'm trying to steal in-process secrets, I can do it from the forked process. 2. The forked process is just as privileged as the original process. If I'm trying to obtain code execution, I don't care which process I'm in. This is why Chrome at al. have full-fledged sandboxes that communicate over restricted IPC; they don't fork the same process and call it a day.
mirashii 2y ago
Even better, this library, with its use of unsafe and fork underneath, introduces a whole new class of undefined behavior to a program by providing a safe interface over an unsafe API without actually enforcing the invariants necessary for safety. In order for the fork() it calls to be safe, it needs to guarantee a bunch of properties of the program that it simply cannot. If this gets used in a multithreaded program that calls malloc, you've got UB. There's a long list of caveats with fork mentioned in some other comments here. In my view, this is not serious code and should be regarded as a joke. There's no actual value in this type of isolation.
- woodruffw 2y ago
  Yep. I wanted to start from the high-level point of "safe doesn't mean doesn't crash," but you're right that the technique itself is unsound.
  - pclmulqdq 2y ago
    In rust terminology, "safe" actually implies more frequent crashes on untrusted inputs.
    - woodruffw 2y ago
      Why do you think this? The closest thing in “common” Rust would be unwraps/panics, but these are (1) not crashes per se, and (2) probably not more common than they would be in an equivalent C codebase.
      - pclmulqdq 2y ago
        "Panics are not crashes" is a new one. I'm referring to the fact that the rust code panics at the slightest sign of discomfort. And they are very much more common than in most C codebases. C codebases are generally often overly permissive in what they accept (hence to security bugs). Rust made a different trade.
        mubou 2y ago
        > I'm referring to the fact that the rust code panics at the slightest sign of discomfort. That's kind of up to you as the developer though. I generally avoid writing functions that can panic -- I'd even argue any non-test code that panics is simply poorly written, because you can't "catch" a panic like you can in a high-level language. Better to return an error result and let the calling code decide how to handle it. Which often means showing an error to the user, but that's better than an unexpected crash.
        pclmulqdq 2y ago
braxxox 2y ago
I've proposed these changes to shy away from the claims of "Run unsafe code safely" in this crate. Let me know what you think, or if you have any additional suggestions.
- braxxox 2y ago
  Oops, forgot to link the PR, lol https://github.com/brannondorsey/mem-isolate/pull/44 https://github.com/brannondorsey/mem-isolate/pull/44

NoahKAndrews 2y ago

It's not just that it won't crash, it means that an exploit in the unsafe code won't allow corrupting memory used by the rest of the program

woodruffw 2y ago
This is pretty immaterial from an exploit development perspective: 1. The forked process has a copy of the program state. If I'm trying to steal in-process secrets, I can do it from the forked process. 2. The forked process is just as privileged as the original process. If I'm trying to obtain code execution, I don't care which process I'm in. This is why Chrome at al. have full-fledged sandboxes that communicate over restricted IPC; they don't fork the same process and call it a day.

mirashii 2y ago

Even better, this library, with its use of unsafe and fork underneath, introduces a whole new class of undefined behavior to a program by providing a safe interface over an unsafe API without actually enforcing the invariants necessary for safety. In order for the fork() it calls to be safe, it needs to guarantee a bunch of properties of the program that it simply cannot. If this gets used in a multithreaded program that calls malloc, you've got UB. There's a long list of caveats with fork mentioned in some other comments here. In my view, this is not serious code and should be regarded as a joke. There's no actual value in this type of isolation.

woodruffw 2y ago
Yep. I wanted to start from the high-level point of "safe doesn't mean doesn't crash," but you're right that the technique itself is unsound.
- pclmulqdq 2y ago
  In rust terminology, "safe" actually implies more frequent crashes on untrusted inputs.
  - woodruffw 2y ago
    Why do you think this? The closest thing in “common” Rust would be unwraps/panics, but these are (1) not crashes per se, and (2) probably not more common than they would be in an equivalent C codebase.
    - pclmulqdq 2y ago
      "Panics are not crashes" is a new one. I'm referring to the fact that the rust code panics at the slightest sign of discomfort. And they are very much more common than in most C codebases. C codebases are generally often overly permissive in what they accept (hence to security bugs). Rust made a different trade.
      - mubou 2y ago
        > I'm referring to the fact that the rust code panics at the slightest sign of discomfort. That's kind of up to you as the developer though. I generally avoid writing functions that can panic -- I'd even argue any non-test code that panics is simply poorly written, because you can't "catch" a panic like you can in a high-level language. Better to return an error result and let the calling code decide how to handle it. Which often means showing an error to the user, but that's better than an unexpected crash.
        pclmulqdq 2y ago

braxxox 2y ago

I've proposed these changes to shy away from the claims of "Run unsafe code safely" in this crate. Let me know what you think, or if you have any additional suggestions.

braxxox 2y ago
Oops, forgot to link the PR, lol https://github.com/brannondorsey/mem-isolate/pull/44 https://github.com/brannondorsey/mem-isolate/pull/44