geiger-rs / cargo-geiger

Detects usage of unsafe Rust in a Rust crate and its dependencies.
https://crates.io/crates/cargo-geiger
1.41k stars 67 forks source link

Listing lines that can panic in your crate #50

Closed RazrFalcon closed 2 years ago

RazrFalcon commented 5 years ago

Is it possible? Something like this:

text[5..100]; // Line N can panic
RazrFalcon commented 5 years ago

Whoops, this crate is about unsafe, not panics.

anderejd commented 5 years ago

Interesting idea though!

And, it's extra important to be panic/exception safe inside unsafe blocks, right? Finding potential panic sites inside unsafe blocks would be valuable.

alexchandel commented 4 years ago

@anderejd Please reopen this as a request for counting panics (and perhaps listing their lines). Panics are very overused in libraries, and can unexpectedly crash unwary users and poison their mutexes.

RazrFalcon commented 4 years ago

@alexchandel Afaik, cargo-geiger simply parses the source code for unsafe. It's not enough to find panics. For example, a pretty common slicing will be a problem.

anderejd commented 4 years ago

@alexchandel RazorFalcon is correct, this tools is not capable if that kind of analysis, at least not without a major rewrite. Maybe you would be interested in this: https://crates.io/crates/no-panic ?

alexchandel commented 4 years ago

Yes, but sometimes "no-panic" isn't achievable. What's needed is an analysis of panicking routes, for minimization. It might not be feasible now, but given the nature of panicking, it fits the idea of a "geiger counter."

Slicing might show up a lot (absent a flag to ignore slicing panics), but often the compiler can prove a slice will never panic.

tarcieri commented 4 years ago

This sounds like it needs a call graph analyzer ala https://github.com/trailofbits/siderophile

alexchandel commented 4 years ago

Oh perhaps. I'll open it there.

Shnatsel commented 4 years ago

https://github.com/Technolution/rustig already addresses this

RazrFalcon commented 4 years ago

@Shnatsel Does it actually works? Looks like it is abandoned and last time I've checked it didn't do anything.

Shnatsel commented 4 years ago

I haven't tried it in a while, so can't really comment.

anderejd commented 4 years ago

It seems this discussion is coming to life again, let's re-open the issue to make it more visible.

pinkforest commented 2 years ago

Well I went down a rabbithole today related to panic caused by the usual library-unwrap() and Err() pattern that had affected us and some others e.g. tarpaulin.

https://github.com/rust-secure-code/cargo-geiger/issues/83#issuecomment-1006431479

I have a "want" to track down this pattern across ecosystem in weighted manner (I already have a synced & tracked copy of it) to trace that kind of pattern across libs to create analysis from weakest points in the ecosystem related to the pattern - binaries can panic all they want.

I'll close this issue so it doesn't confuse from repo issues - if we find a security related use we could add issue in rust-secure-code/wg

Perhaps we can moderate a moonshots tracker idea issue somewhere and refer from there to here so it's neither buried under a lot of other stuff e.g. repo issues.

8573 commented 2 years ago

Is it possible? Something like this:

text[5..100]; // Line N can panic

Without suggesting reopening this ticket, I note that some use-cases for this may be served by the many Clippy lints returned by searching the Clippy lint index for "panic", with Indexing in particular being covered by indexing_slicing.

pinkforest commented 2 years ago

Since clippy has introduced very geiger relevant #247 and we are thinking of introducing clippy runner as part of 0.12 this may be viable as a feature to plug into

I'll convert this into discussion re: feasibility discussion