rust-guide icon indicating copy to clipboard operation
rust-guide copied to clipboard

Forbid unsafe code

Open JeanMertz opened this issue 6 years ago • 7 comments
trafficstars

I noticed the chapter on unsafe code.

There's a TODO mentioning:

check that no unsafe blocks appear in the imported dependencies

You can do that without tools, if you add #![forbid(unsafe_code) at the top of your lib.rs or main.rs.

Having said that, this not only forbids unsafe code in imported libraries, but also your own code. But it could be argued that well maintained open source unsafe code might be safer than the one your write in a more closed/less scrutinized environment.

I know Cargo-geiger was already mentioned before, and wasn't considered an option, so I thought I'd mention this potential solution.

JeanMertz avatar Feb 09 '19 20:02 JeanMertz

As far as I understand, #![forbid()] lints only work on the current crate, not on dependencies. However, they do work for sub-modules that cannot #![allow()] them again.

Ekleog avatar Feb 09 '19 23:02 Ekleog

As far as I understand, #![forbid()] lints only work on the current crate, not on dependencies.

Ah yes, you are correct. I believe there is a way to modify the rustc command that cargo build runs when building dependencies, which should allow you to force it to run clippy and apply the forbid(unsafe_code) lint.

But it requires some bootstrapping of the project, it's not available by default.

JeanMertz avatar Feb 10 '19 09:02 JeanMertz

Thanks to your comments, I realize that the mention:

unsafe blocks are discussed in the following chapter

in the TODO can be misleading. There are in fact two separate problems:

  • forbidding unsafe code in the current crate,
  • verifying that dependencies don't use unsafe code, except when obviously needed (FFI, etc.).

As outlined by @Ekleog, the #![forbid(unsafe_code)] rule handles the first case.

For the second one, cargo-geiger may be a good option. The reason it has not been mentioned in the guide yet is that it doesn't detect unsafe code inside macros, and may have some issues (see opened issues on cargo-geiger). That said, this tool may be cited, as proposed before, but with a warning about potential false negatives.

I believe there is a way to modify the rustc command that cargo build runs when building dependencies, which should allow you to force it to run clippy and apply the forbid(unsafe_code) lint.

This could be a path to checking unsafe code in dependencies (I think that clippy is not required here, the -D unsafe_code of the rustc compiler can be used to deny compiler lint). But this would require a rather heavy setup (custom toolchain, keeping it up to date, etc.).

ad-anssi avatar Feb 13 '19 13:02 ad-anssi

Regarding the rustc compiler flag, using -F instead of -D uses forbid() instead of deny() (thus preventing code to overrule the lint).

One way to pass flags to rustc from Cargo is through the RUSTFLAGS environment variable: RUSTFLAGS="$RUSTFLAGS -Funsafe-code" cargo check

Sadly it does not seem to work for non-local crates, so a more sophisticated method (such as a custom toolchain) would indeed be needed.

danielhenrymantilla avatar Feb 15 '19 14:02 danielhenrymantilla

It looks like you could be interested in the latest version of cargo-geiger, it will scan for #![forbid(unsafe_code)] in all dependencies. https://github.com/anderejd/cargo-geiger/pull/52 Please see the changelog for 0.6.0 for more details.

anderejd avatar Feb 21 '19 08:02 anderejd

This is great news and very nice improvement of the cargo-geiger tool! Also thank you for mentioning this new version here, I'll add corresponding recommendations in the guide very soon.

ad-anssi avatar Feb 21 '19 09:02 ad-anssi

The current text of the Rule LANG-UNSAFE has:

With the exception of these cases, #[forbid(unsafe_code)] must appear in main.rs to generate compilation errors if unsafe is used in the code base.

This talks about "code base", which is misleading. It doesn't point out that this will not scrutinize dependencies. I think "current crate" or a stronger warning are in order here. It also talks about "main.rs", whereas this can be used in libraries as much as in binaries.

When advocating tools like cargo geiger or cargo audit, it's important to mention that those tool should be run with --all-features lest they also show misleading underestimations.

Sadly it does not seem to work for non-local crates, so a more sophisticated method (such as a custom toolchain) would indeed be needed.

cargo vendor might be of interest here. If all else fails one can grep for unsafe.

najamelan avatar Mar 22 '20 11:03 najamelan