Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forbid unsafe code #10

Open
JeanMertz opened this issue Feb 9, 2019 · 7 comments
Open

Forbid unsafe code #10

JeanMertz opened this issue Feb 9, 2019 · 7 comments
Labels
enhancement New feature or request new recommendation New recommendation proposal

Comments

@JeanMertz
Copy link

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.

@Ekleog
Copy link

Ekleog commented Feb 9, 2019

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.

@JeanMertz
Copy link
Author

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.

@ad-anssi
Copy link
Contributor

ad-anssi commented Feb 13, 2019

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.).

@danielhenrymantilla
Copy link
Contributor

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.

@anderejd
Copy link

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.

@ad-anssi
Copy link
Contributor

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 ad-anssi added enhancement New feature or request new recommendation New recommendation proposal labels Feb 21, 2019
@najamelan
Copy link

najamelan commented Mar 22, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request new recommendation New recommendation proposal
Projects
None yet
Development

No branches or pull requests

6 participants