Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Secure Rust Guidelines

anssi-fr.github.io

Post reply on HN
Print view View original

41–50 of 61 posts

Re: Secure Rust Guidelines

#41

Post by masklinn »

baby wrote: post #39
masklinn wrote: post #12

Earlier quoted context omitted.

Yes that is one of the primary failures of Rust at the moment: to my knowledge it currently has no good way to safely manage allocation failures (it also has serious issues with stack overflows). This is an issue with all heap-allocating construct, not just collections but also Box or Rc. > I wish Rust had some easy way to recover from out-of-memory situations when using the standard library. I have been considering…

Honestly all C programs that fail to allocate panic as well. What else can you do?

> Honestly all C programs that fail to allocate panic as well.

I'd say that most C programs that fail to allocate panic (if only because e.g. they're running over-committing anyway so allocations never fail), but it's not true that all of them do. There are systems which do handle heap allocation failures "properly", usually ones which run on systems which are both resource-constrained enough that allocation failure is a possibility yet large enough that heap allocation is a possibility e.g. modern embedded-ish systems running with dozens (!) or even hundreds (!!!) of megabytes of RAM.

Or the software might be designed for running with a low ulimit for some reason (I think that also triggers allocation failures, I remember that being a case where Python throws MemoryError).

Even on larger machines, I'd also guess it is / was also more common on 32b systems where address space exhaustion could be a real concern.

> What else can you do?

You might just abort the current task because it's not that important when resource constrained, or you might have caches you can clear. Or possibly (if that's a possibly routine issue e.g. a background task which might run in cases of both plentiful and constained resources) you might switch to a less CPU efficient but more memory-efficient algorithm.

Re: Secure Rust Guidelines

#42

Post by masklinn »

baby wrote: post #38
masklinn wrote: post #7

Earlier quoted context omitted.

> First of all, they kind of dodge this later by saying "Array indexing must be properly tested" (else it can panic) -- everyone thinks they write code which is "properly tested". You're skipping half the recommendation though: > Array indexing must be properly tested, or the get method should be used to return an Option . emphasis mine > Also, in rust if we don't want to panic we need to never use array indexing and…

There’s a number of functions that panic in the std if misused, for example copy_from_slice

Yes?

Re: Secure Rust Guidelines

#43

Post by masklinn »

swiftcoder wrote: post #30
steveklabnik wrote: post #21

Earlier quoted context omitted.

Same as any other failure; functions that may try to allocate memory and fail will return a Result instead of just a T.

Which would be pretty painful for everyone not working in a memory-constrained environment (at a guess, that's most developers). Much like std:println! panicking instead of returning an error, ergonomics are important too a lot of the time.

It obviously wouldn't be the default (it couldn't be, in fact, since the existing APIs are set in stone short of memory unsafety).

The way it'd work is either you'd have two different collections entirely, one which'd signal and one which'd panic (the latter possibly being built atop the former) or every method which can panic on allocation failure would also have a panic-safe alternative.

Example: https://github.com/rust-lang/rust/issues/48043 similarly Vec could gain `try_push`, `try_append`, `try_insert`, `try_split_off`, …

Re: Secure Rust Guidelines

#44

Post by rectang »

mbrubeck wrote: post #35
cesarb wrote: post #33

Earlier quoted context omitted.

> If you don't want a panic to take down the whole system, you can isolate the code in a thread and use a supervision tree, or use `catch_unwind` to let the thread perform cleanup and then continue from a known state. Playing devil's advocate: with unwinding panics (which are necessary for these two approaches), it's harder to make sure all the data structures the thread was using are left in a coherent state. It's n…

Agreed. There are definitely good arguments for using abort-on-panic, and doing isolation and recovery at the process level rather than the thread level. This is what we do with all the Rust code in Firefox, for example. Unwind safety is a real issue. I have some personal experience with fixing panic-safety issues in unsafe Rust code: https://github.com/servo/rust-smallvec/pull/103 I wrote a bit more about it here: h…

Even isolation and recovery at the process level does not guarantee that a multi-process application will have no invalid state! By the time invalid state is detected in one process and a panic happens, the invalid state may have already propagated to another process via IPC messaging.

And even taking down an entire multi-process application doesn't fully protect against invalid state, if that invalid state has wound up in persistent storage.

All recoveries from panics are ultimately heuristics.

Re: Secure Rust Guidelines

#45

Post by Paul-ish »

CJefferson wrote: post #5

I personally strongly disagree with: Functions or instructions that can cause the code to panic at runtime must not be used. First of all, they kind of dodge this later by saying "Array indexing must be properly tested" (else it can panic) -- everyone thinks they write code which is "properly tested". Personally, I often write panicing code -- if the code gets in a state where I have no idea how to fix it, I panic. F…

I agree wholeheartedly. If we tell people "Don't write code that panics" then we collectively start writing code that assume panics don't happen. Suddenly panics become a footgun in Rust, and it isn't the safe language we want it to be.

If I'm feeling in a mood, I think we should panic all the time. We should have a "panic monkey" tool that places panics at random parts of the code, to see how panics are handled. Eg to see if they interact poorly with unsafe code.

Re: Secure Rust Guidelines

#46

Post by Paul-ish »

baby wrote: post #36
CJefferson wrote: post #5

I personally strongly disagree with: Functions or instructions that can cause the code to panic at runtime must not be used. First of all, they kind of dodge this later by saying "Array indexing must be properly tested" (else it can panic) -- everyone thinks they write code which is "properly tested". Personally, I often write panicing code -- if the code gets in a state where I have no idea how to fix it, I panic. F…

No，if you are writing secure code then a panic is a denial of service. Their point is that you should never panic, unless you do reach an unrecoverable error.

It seems like, in the absence of panics, there is a dichotomy between "You should know everything that can happen to your program and account for it." and "If you hit some unknown state, carry on." The first one is impossible, the second sounds dangerous. When you get into this dichotomy, panicking seems like a reasonable third option. We should strive for the first, but avoid the second. Nobody wants their code to panic, but we need to be able to deal with unexpected states.

Re: Secure Rust Guidelines

#47

Post by dmm »

rectang wrote: post #44
mbrubeck wrote: post #35

Earlier quoted context omitted.

Agreed. There are definitely good arguments for using abort-on-panic, and doing isolation and recovery at the process level rather than the thread level. This is what we do with all the Rust code in Firefox, for example. Unwind safety is a real issue. I have some personal experience with fixing panic-safety issues in unsafe Rust code: https://github.com/servo/rust-smallvec/pull/103 I wrote a bit more about it here: h…

Even isolation and recovery at the process level does not guarantee that a multi-process application will have no invalid state! By the time invalid state is detected in one process and a panic happens, the invalid state may have already propagated to another process via IPC messaging. And even taking down an entire multi-process application doesn't fully protect against invalid state, if that invalid state has wound…

> if that invalid state has wound up in persistent storage.

The same can be said for a program recovering from a power loss?

Re: Secure Rust Guidelines

#48

Post by eeZah7Ux »

mbrubeck wrote: post #25
CJefferson wrote: post #5

I personally strongly disagree with: Functions or instructions that can cause the code to panic at runtime must not be used. First of all, they kind of dodge this later by saying "Array indexing must be properly tested" (else it can panic) -- everyone thinks they write code which is "properly tested". Personally, I often write panicing code -- if the code gets in a state where I have no idea how to fix it, I panic. F…

Panicking is, in many ways, the best-case scenario for code that contains a bug. A bug that causes a panic is much easier to find through fuzzing, property testing, even static analysis or analyzing failures in production. It also prevents the bug from "infecting" other code by allowing the program to proceed in an invalid state. If you don't want a panic to take down the whole system, you can isolate the code in a t…

> Panicking is, in many ways, the best-case scenario for code that contains a bug.

The very opposite: it's among the worst behaviors, short of creating a vulnerability.

From any modern languages we should expect an exception to be raised.

Re: Secure Rust Guidelines

#49

Post by dathinab »

I found at least one problematic section when scanning:

> The environment variables RUSTC, RUSTC_WRAPPER and RUSTFLAGS must not be overriden when using Cargo to build project.

This is simply not true at all. Mainly build cache systems like sccache work by wrapping rustc, there is no reason why using a build cache should be forbidden.

(Through currently rust support of sccache is still not perfect and there are some limitations, doesn't change that the rules are to broad.)

I also wouldn't be surprised if there are some rustc flags not exposed in cargo profiles which allow trigger some security mechanisms in llvm which are not enabled by default but beneficial for your project.

Like always the important think is that you understand what you do and want implications it had.

Re: Secure Rust Guidelines

#50

Post by dathinab »

Paul-ish wrote: post #45
CJefferson wrote: post #5

I personally strongly disagree with: Functions or instructions that can cause the code to panic at runtime must not be used. First of all, they kind of dodge this later by saying "Array indexing must be properly tested" (else it can panic) -- everyone thinks they write code which is "properly tested". Personally, I often write panicing code -- if the code gets in a state where I have no idea how to fix it, I panic. F…

I agree wholeheartedly. If we tell people "Don't write code that panics" then we collectively start writing code that assume panics don't happen. Suddenly panics become a footgun in Rust, and it isn't the safe language we want it to be. If I'm feeling in a mood, I think we should panic all the time. We should have a "panic monkey" tool that places panics at random parts of the code, to see how panics are handled. Eg…

I can only agree diesel had (maybe still has) a serve bug around connection reuse and panics.

Panics in rust originally where a pretty nice way to have a error kennel pattern and as long as you always kill the whole error kennel in which the panic occurred and don't share mutable state between error kennels this fully save from any inconsistent state.

Sure panics across FFI boundaries can't be done nice at all, but this can still be handled by making sure there is no FFI boundary insider of an error kernel, just in between.

So I would push panic usage for error kennels, and just that. But also the unwind safe traits need an refresh: Enforce them (by linting against inconsistencies) and better defaults for what does and doesn't implement them.

For example a type which is sync but doesn't implement the referential unwind safe trait is broken/bad in 99% of cases. (Either it should implement unwind safe market or not be send). The remaining 1% are edge cases between the usage of catch unwind and thread local storage.

Post reply on HN