Skip to content

Report unsoundness in cve-rs, totally-safe-transmute and totally-safe #2221

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

Merged
merged 4 commits into from
May 5, 2025

Conversation

Nugine
Copy link
Contributor

@Nugine Nugine commented Feb 10, 2025

We are using cargo audit -D warnings to avoid problematic dependencies.

To prevent anyone from using these crates in the dependency tree (accidentally?), I think it is meaningful to include them in advisories.

Reverse dependencies

resolves #826

@tarcieri
Copy link
Member

See previous discussion on #826

@Nugine
Copy link
Contributor Author

Nugine commented Feb 10, 2025

The three crates are intentional exploits of rustc soundness bugs, like fake-static, which has been flagged in #207 and #270 previously. I personally recommend including them as informational = "unsound".
If RustSec cannot make a decision yet, I'm Ok to leave this PR open and wait.

@kornelski
Copy link
Contributor

These crates may be a joke, but the APIs are unsound for real. I expect cargo audit is used in projects where ending up with any of them as a dependency isn't funny. I would prefer to have them flagged.

@riking
Copy link
Contributor

riking commented Mar 21, 2025

I think some new common vocabulary here might be helpful - perhaps "[crate] is intentionally unsound" with keyword=["intentionally-unsound", "compiler-bug"] ?

@djc
Copy link
Contributor

djc commented Mar 22, 2025

I think some new common vocabulary here might be helpful - perhaps "[crate] is intentionally unsound" with keyword=["intentionally-unsound", "compiler-bug"] ?

I like this suggestion.

@fintelia
Copy link

fintelia commented May 4, 2025

I wonder if something along the lines of "exploit-demo" would be a better tag. The crates are all demonstrations of unsoundness caused by other software (either rustc or the standard library) which work by exposing a clearly unsound API while using no unsafe code themselves.

@djc
Copy link
Contributor

djc commented May 5, 2025

I wonder if something along the lines of "exploit-demo" would be a better tag. The crates are all demonstrations of unsoundness caused by other software (either rustc or the standard library) which work by exposing a clearly unsound API while using no unsafe code themselves.

Maybe unsound-demo, then?

@tarcieri
Copy link
Member

tarcieri commented May 5, 2025

I think "soundness hole" is the typical term for these

@fintelia
Copy link

fintelia commented May 5, 2025

These crates do not contain a soundness hole. Rather, they're demonstrations that a soundness hole exists in the compiler/standard library.

@tarcieri
Copy link
Member

tarcieri commented May 5, 2025

Yes, I was suggesting they exploit a soundness hole

@djc djc merged commit bb435cc into rustsec:main May 5, 2025
1 check passed
@djc
Copy link
Contributor

djc commented May 5, 2025

Thanks!

@Nugine Nugine deleted the report-unsoundness branch May 5, 2025 18:29
@fintelia
Copy link

fintelia commented May 5, 2025

Having the category be "unsound" and the keyword "soundness-hole" in the advisory makes it sound to me like these are saying that the crates themselves contain soundness holes. Further, the advisory text lacks any discussion that these are demonstrations and not intended for use by other crates. In fact, restating the satirical content of those crate's documentation provides the exact opposite impression. It would be very easy to interpret these advisories as disparaging the crate authors

@djc
Copy link
Contributor

djc commented May 5, 2025

Having the category be "unsound" and the keyword "soundness-hole" in the advisory makes it sound to me like these are saying that the crates themselves contain soundness holes. Further, the advisory text lacks any discussion that these are demonstrations and not intended for use by other crates. In fact, restating the satirical content of those crate's documentation provides the exact opposite impression. It would be very easy to interpret these advisories as disparaging the crate authors

A PR revising the text in a direction you find more appropriate would be welcome.

@tarcieri
Copy link
Member

tarcieri commented May 5, 2025

@fintelia how can a crate introduce a soundness hole? Isn't that definitionally impossible as soundness holes are a property of the language?

@Nugine
Copy link
Contributor Author

Nugine commented May 5, 2025

Having the category be "unsound" and the keyword "soundness-hole" in the advisory makes it sound to me like these are saying that the crates themselves contain soundness holes. Further, the advisory text lacks any discussion that these are demonstrations and not intended for use by other crates. In fact, restating the satirical content of those crate's documentation provides the exact opposite impression. It would be very easy to interpret these advisories as disparaging the crate authors

If you can trigger UB without using any unsafe API from a library, then the library is unsound. These crates are unsound even though the holes are "re-exported" from rustc.

These demonstrations should not be published to crates.io if they are really not intended for use by other crates.

I don't think copying the introduction of these crates makes it satirical to the crate authors.

@fintelia
Copy link

fintelia commented May 6, 2025

@fintelia how can a crate introduce a soundness hole? Isn't that definitionally impossible as soundness holes are a property of the language?

I don't know the precise definition, but there's no language level issue involved in totally-safe-transmute. It merely demonstrates that the unsafe blocks used to implement std::fs::File aren't actually sound, because operating on /proc/self/mem can violate soundness. The reason I'm arguing is because there's probably thousands of crates that behave unsoundly if you pass in /proc/self/mem as a path argument. Rustsec does not and should not issue advisories for all of them.

As a side note, did anyone reach out to the crate authors to ask about phrasing or what they think about these advisories?

@djc
Copy link
Contributor

djc commented May 6, 2025

@Speykious, @ben0x539, @viktorlott any comments/suggestions?

@ben0x539
Copy link
Contributor

ben0x539 commented May 6, 2025

Hi I'm the totally-safe-transmute author. I appreciate the throughtful discussion in this thread over my silly joke crate. I'm currently not using rust in production so I happily defer to others' expectations that cargo-audit warns them if someone genuinely pulls in totally-safe-transmute.

I would appreciate it if the wording of the advisory included some indication that the crate is doing a bit and as such wasn't meant to be used in real code, and that's why it will not have patched versions. I acknowledge I didn't exactly bother explicitly giving any such indication either, but then again my target audience was rust programmers with extra time on their hands and not consumers of advisories from an automated system who did not sign up for divining intent from context clues.

If you were to put something like "This crate is a toy and should never be used" I think I should be perfectly satisfied, and I think it would also be good information for advisory consumers, since, like, the problem that someone is pulling toy code into your production dependency tree is distinct from the problem that that code is sketchy. I suppose it doesn't actually come up a lot or there would be a tag for it already.

These demonstrations should not be published to crates.io if they are really not intended for use by other crates.

Yeah I suppose I never had a good reason for posting to crates.io beyond roleplaying as a real crate. I apologize to anyone who's had extra headaches at work because of me having a fun time with my silly code.

@djc
Copy link
Contributor

djc commented May 6, 2025

If you were to put something like "This crate is a toy and should never be used" I think I should be perfectly satisfied, and I think it would also be good information for advisory consumers, since, like, the problem that someone is pulling toy code into your production dependency tree is distinct from the problem that that code is sketchy. I suppose it doesn't actually come up a lot or there would be a tag for it already.

Want to submit a PR along these lines?


# cve-rs introduces memory vulnerabilities in safe Rust

`cve-rs` allows you to introduce common memory vulnerabilities (such as buffer overflows and segfaults) into your Rust program in a memory safe manner.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw some discussion above about the utility of restating the project's satirical description. I don't have much of anything to suggest in regards to this PR, but if someone makes a PR in the future and wants to change the descriptions, I propose:

`cve-rs` provides demonstrations of common memory vulnerabilities (such as buffer overflows and segfaults) implemented completely within safe Rust.

I'm fine either way. That being said, I don't have much experience using Rustsec (I probably should).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For you too: a PR making this change would be great!

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

Successfully merging this pull request may close these issues.

totally-safe-transmute is totally unsafe
8 participants