4 ms·

I wrote a longer response to a sibling comment, but I highly dispute this: > Rust's array type is an actual array, and so a lot of the problems you're worried

by OskarS 3y ago

I wrote a longer response to a sibling comment, but I highly dispute this: > Rust's array type is an actual array, and so a lot of the problems you're worried about (which would be big pitfalls out of the box in C) only happen in Rust if you went to some effort to dig a hole for yourself so you could fall into it. It is an extremely easy hole to fall into. If you hold on to an index X into an array, and some different part of the program deleted an entry in the array with an index smaller than X, your X is now no longer valid, it's pointing to the wrong object. This is exactly equivalent to an "invalidated pointer", which is an issue the borrow checker was designed to solve. But there is no way for it to know this, because by using indexes instead of pointers, you've disabled its ability to analyze this situation. I agree that pointers in C and indexes in Rust are not the same thing, and there are other issues with C pointers that Rust's borrow checker solves. But imagining that you're magically safe from any lifetime issues "because Rust" is foolish.

davidatbu 3y ago
Here's the implementation of "remove a value" [1] from the popular Rust crate that implements this pattern. It completely sidesteps the issue you are describing because "deleting from array" does not mean "adjust every greater index by one". However, I'll grant you that using this crate means you have to extend _some_ trust (even if very small) to the crate maintainer to maintain safety invariants, in addition to Rust's compiler maintainers. But the fact that I could dive into its code (that I've never seen before) in 2 minutes and verify that it is not susceptible to the issue you describe gives me more confidence that auditing+trusting this crate will not be an undue burden. [1] https://docs.rs/slotmap/latest/src/slotmap/basic.rs.html#395 https://docs.rs/slotmap/latest/src/slotmap/basic.rs.html#395
tialaramex 3y ago
> some different part of the program deleted an entry in the array That's not a thing. This is the linchpin of your argument, and it just doesn't exist. Try it, make an array of, I dunno, Strings, and then "delete" one. You can't, there is no "delete" operator, that would be meaningless. People with some brief exposure to Rust might think, "OK smart arse, it's called drop, I can just drop the String". Nope. That's an array of Strings, you can't just drop one of them, it's part of the type [String; N] it's not OK if one of them isn't a String, so you're going to need to consciously swap it for a different String, you could core::mem::take that element of the array, leaving its default (an empty String) behind and then drop the one you took. But what we've got here isn't an "invalidated pointer" we've just got an ordinary logic error in our program where we mistakenly empty strings we still wanted. You have to go build your own pit, and fill it with spikes, and jump into it, all so that you can moan, "Oh, Rust basically has the same terrible pit traps as C or C++" except, you built the trap, so don't do that.
- OskarS 3y ago
  > Try it, make an array of, I dunno, Strings, and then "delete" one. You can't, there is no "delete" operator, that would be meaningless. Your argument is that there's no such function as `std::Vec::remove`? I can assure you, it exists: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.remove https://doc.rust-lang.org/std/vec/struct.Vec.html#method.rem... If you use that function, and you've held on to a an index that is no longer valid, it is pointing to the wrong string. Or worse: it's pointing outside of the array, and your program will panic. The index is no longer valid, just like a pointer in C is no longer valid after calling free(). > You have to go build your own pit, and fill it with spikes, and jump into it, all so that you can moan, "Oh, Rust basically has the same terrible pit traps as C or C++" except, you built the trap, so don't do that. I am not saying "Rust basically has the same terrible pit traps as C or C++". I have never said that, and I never would. What I'm saying is that if you're using indexes like this, you no longer have the benefit of lifetime safety guaranteed by the borrow checker. You have, in essence, turned the borrow checker off for this part. BTW, making the argument "as long as you're not a bad programmer, this isn't a problem" is exactly the argument that some Rust haters make: "well, just don't call free() twice, and it's not a problem!". Either way, it's bull: the whole point of the borrow checker is that it statically guarantees that these kinds of lifetime issues can't happen. But if you turn it off using indexes, it guarantee is out the window. The whole discussion started with the parent commentator pointing out a project that does exactly this, and me pointing out that this is a problem from a static safety perspective.
  - amluto 3y ago
    I think the disagreement here is about what happens when you mess up. In C, if you have a UAF, the attacker likely owns your entire process. They can call any function with any arguments. They can modify entirely unrelated objects. Your backups get encrypted and your secrets show up on dubious websites. You lose, badly. If you roll your own arena in Rust and you “UAF” a struct, you either panic or you continue executing, with a correct control flow graph, with no language-defined UB. The attacker can do only what your code can be convinced to do. Sure, if you implemented an entire VM based on your arena, maybe the attacker can convince your VM to encrypt your backups and leak your private data. But this only happens if your VM can do that in the first place. If your code just draws creatures in 3D, then all the attacker can do is mess with the creatures.

by OskarS 3y ago

davidatbu 3y ago

Here's the implementation of "remove a value" [1] from the popular Rust crate that implements this pattern. It completely sidesteps the issue you are describing because "deleting from array" does not mean "adjust every greater index by one". However, I'll grant you that using this crate means you have to extend _some_ trust (even if very small) to the crate maintainer to maintain safety invariants, in addition to Rust's compiler maintainers. But the fact that I could dive into its code (that I've never seen before) in 2 minutes and verify that it is not susceptible to the issue you describe gives me more confidence that auditing+trusting this crate will not be an undue burden. [1] https://docs.rs/slotmap/latest/src/slotmap/basic.rs.html#395 https://docs.rs/slotmap/latest/src/slotmap/basic.rs.html#395

tialaramex 3y ago

> some different part of the program deleted an entry in the array That's not a thing. This is the linchpin of your argument, and it just doesn't exist. Try it, make an array of, I dunno, Strings, and then "delete" one. You can't, there is no "delete" operator, that would be meaningless. People with some brief exposure to Rust might think, "OK smart arse, it's called drop, I can just drop the String". Nope. That's an array of Strings, you can't just drop one of them, it's part of the type [String; N] it's not OK if one of them isn't a String, so you're going to need to consciously swap it for a different String, you could core::mem::take that element of the array, leaving its default (an empty String) behind and then drop the one you took. But what we've got here isn't an "invalidated pointer" we've just got an ordinary logic error in our program where we mistakenly empty strings we still wanted. You have to go build your own pit, and fill it with spikes, and jump into it, all so that you can moan, "Oh, Rust basically has the same terrible pit traps as C or C++" except, you built the trap, so don't do that.

OskarS 3y ago
> Try it, make an array of, I dunno, Strings, and then "delete" one. You can't, there is no "delete" operator, that would be meaningless. Your argument is that there's no such function as `std::Vec::remove`? I can assure you, it exists: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.remove https://doc.rust-lang.org/std/vec/struct.Vec.html#method.rem... If you use that function, and you've held on to a an index that is no longer valid, it is pointing to the wrong string. Or worse: it's pointing outside of the array, and your program will panic. The index is no longer valid, just like a pointer in C is no longer valid after calling free(). > You have to go build your own pit, and fill it with spikes, and jump into it, all so that you can moan, "Oh, Rust basically has the same terrible pit traps as C or C++" except, you built the trap, so don't do that. I am not saying "Rust basically has the same terrible pit traps as C or C++". I have never said that, and I never would. What I'm saying is that if you're using indexes like this, you no longer have the benefit of lifetime safety guaranteed by the borrow checker. You have, in essence, turned the borrow checker off for this part. BTW, making the argument "as long as you're not a bad programmer, this isn't a problem" is exactly the argument that some Rust haters make: "well, just don't call free() twice, and it's not a problem!". Either way, it's bull: the whole point of the borrow checker is that it statically guarantees that these kinds of lifetime issues can't happen. But if you turn it off using indexes, it guarantee is out the window. The whole discussion started with the parent commentator pointing out a project that does exactly this, and me pointing out that this is a problem from a static safety perspective.
- amluto 3y ago
  I think the disagreement here is about what happens when you mess up. In C, if you have a UAF, the attacker likely owns your entire process. They can call any function with any arguments. They can modify entirely unrelated objects. Your backups get encrypted and your secrets show up on dubious websites. You lose, badly. If you roll your own arena in Rust and you “UAF” a struct, you either panic or you continue executing, with a correct control flow graph, with no language-defined UB. The attacker can do only what your code can be convinced to do. Sure, if you implemented an entire VM based on your arena, maybe the attacker can convince your VM to encrypt your backups and leak your private data. But this only happens if your VM can do that in the first place. If your code just draws creatures in 3D, then all the attacker can do is mess with the creatures.