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

Try to detect typo-squatted crates #421

Open
jbg opened this issue May 13, 2022 · 4 comments
Open

Try to detect typo-squatted crates #421

jbg opened this issue May 13, 2022 · 4 comments
Labels
enhancement New feature or request

Comments

@jbg
Copy link
Contributor

jbg commented May 13, 2022

Is your feature request related to a problem? Please describe.
A malicious crate rustdecimal was recently uploaded to crates.io which attempted to compromise GitLab CI systems when Decimal::new was called. (GitHub issue / Advisory)

Describe the solution you'd like
cargo-deny could detect and warn/deny crates based on some heuristics, to try to detect such a situation. For example, if the crate has a name within a certain (probably very short) edit distance of another crate, especially if that other crate is much more popular. Ideally this would be applied across the entire dep tree to detect dependencies which themselves depend on typosquatted crates.

Describe alternatives you've considered
rust-lang/crates.io#1074 describes a manual review process by Rust Security Response WG which would be complementary. If they add a flag indicating whether the crate has been reviewed to the API, it could be used to automatically silence the warning.

That issue also suggests a warning when cargo add is used, but that relies on the use of cargo add whereas most people I know directly edit Cargo.toml.

@jbg jbg added the enhancement New feature or request label May 13, 2022
@Jake-Shadle
Copy link
Member

I think such a feature might be nice, but I'm not completely sold. One issue would be that figuring out which crates are "popular" would require some kind of external calls (crates.io most likely) which runs into problems of either rate limiting/blocking (https://crates.io/policies#crawlers) or if some other aggregator/data source was used possibly requiring something like API tokens or similar. Currently cargo-deny only relies on 3 sources of external data, the crates.io index, the crate source, and the advisory database(s) if doing advisory checks, all of which can be retrieved ahead of time so that cargo-deny can be run in --offline mode so no network access is done.

The other issue would be that of duplicating work/policies, which ideally would be done by crates.io and/or rustsec security advisories, and which would only be taken advantage of by cargo-deny users. I don't know how this particular issue is going to evolve, but I'm hopeful that actions can be taken by crates.io/cargo to make this kind of situation less likely in the future which would benefit all cargo/rust users, not just cargo-deny users.

All that being said, we do already have #128 which is a bit related in that users might want to always do a manual review of a build.rs script or proc macro crate whenever it is added or changes as they can do, well, just about anything they want, and typo squatted crates, even if they are legitimate may fall under the same umbrella as "this might be suspect, the user wants to fail checks if a typo squatted crate is added or changes".

@jbg
Copy link
Contributor Author

jbg commented May 13, 2022

I'm not sure how quickly or completely crates.io / rustsec will be able to solve this issue. The manual review idea has some problems (in particular, it would require manual review of every version of a crate within the edit distance threshold, which could end up being too much workload).

The feature could still be useful without the popularity gate, although it might be too noisy depending on the chosen edit distance threshold. But with an easy way to add false positives to a list in deny.toml, that might be fine.

@Shinyzenith
Copy link

Shinyzenith commented May 14, 2022

Hi, just chiming in on the issue.

Given the recent circumstances I personally believe supply chain attacks will never be completely gone, look at npm ( a package manager which is just as old as rust itself ) hasn't been able to solve this issue completely.
Given that we are all programmers, I think it's generally safe to say that most if not all of us will look for shady signs in any crate ( personally I always check out the main repo, if it's being maintained or not, stars and the number of crates.io downloads ) so personally I feel that the most effective mitigation apart from the already mentioned typo_squatting checks is to keep your eyes peeled for suspicious behavior while copying the version string from crates.io.

Now the question arises as to what we should do for cargo add. Again I think it would be nice if some crate information is printed to stdout on running cargo add so we get immediately alerted to see a popular crate with low download counts.

Finally, I've never contributed to rust or cargo-deny itself and I still consider myself an amateur rust developer. I believe there's way more qualified people here so please do correct me if my understanding is flawed. Cheers.

@jbg
Copy link
Contributor Author

jbg commented May 15, 2022

Now the question arises as to what we should do for cargo add. Again I think it would be nice if some crate information is printed to stdout on running cargo add so we get immediately alerted to see a popular crate with low download counts.

Any proposed change to cargo add (which is not a feature of Cargo, but part of the third-party cargo-edit plugin) would belong as an issue on the cargo-edit repo. But I think it would be a minor win compared to solutions that work even if you edit Cargo.toml directly.

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

No branches or pull requests

3 participants