search

rust-secure-code / wg

Coordination repository for the Secure Code Working Group
150 stars 10 forks source link

Item shadowing #38

Open burdges opened 4 years ago

burdges commented 4 years ago

Are we concerned about item shadowing? Do we want lints that forbid shadowing?

At a technical level, item shadowing might not create so many new threats per se, but they simplify innocent looking bug doors.

It remains unclear to me if https://github.com/rust-lang/rfcs/pull/2845 makes the item shadowing situation better or worse.

Shnatsel commented 4 years ago

Item shadowing is a double-edged sword. Intentional use for removing variables you don't want to use accidentally is great. On the other hand, accidental shadowing may cause issues.

I don't recall any RustSec advisories for issues caused by shadowing, for what it's worth. Neither do I recall any in the fuzzing trophy case, but I can't remember all of those for sure.

IIRC there is a clippy lint that makes any kind of shadowing a warning.

kpcyrd commented 4 years ago

I usually use shadowing when I get something like Result<Option<T>> and I actually want a T. There used to be an underhanded-rust contest going on, maybe it's time to do another round. :)

burdges commented 4 years ago

I'd think such attacks live only in NPM land still, but we've so much large crypto-currency software being written in rust that bug door attacks using shadowing sound plausible eventually.

IIRC there is a clippy lint that makes any kind of shadowing a warning.

Any? We do not care about local variable shadowing, only cross-crate method shadowing. We can close this issue if clippy can do methods only. We've some cargo tool to run clippy on dependencies?