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

From<String> for BoxedString unsound in smartstring crate (also, unmaintained) #1854

Closed
MolotovCherry opened this issue Jan 17, 2024 · 3 comments

Comments

@MolotovCherry
Copy link

MolotovCherry commented Jan 17, 2024

The smartstring crate has an unsound implementation for converting String to BoxedString.

At the moment, I don't know exactly what the conditions for the unsound behavior are, but it is documented in their issue 49, and I have an example there which triggers miri. Additionally, I have verified that PR 34 fixes the UB issue I encountered (however I am unsure whether it is still unsound after, or if other unsoundness exists)

The crate seems to be unmaintained as well. The author has not commented on any of the open PR's or issues since 3/24/2022 (afaict), including this one fixing the UB. After checking their profile, they do have 1 commit to a separate repo on 12/16/23, however their activity is quite sparse.

Checking the reverse dependencies, some big crates are using this, like swc.

I have a reproducible example in the issue linked below.

Crate: https://github.com/bodil/smartstring
Issue: bodil/smartstring#49
PR fixing it: bodil/smartstring#34

@Shnatsel
Copy link
Member

Thank you for the report!

This violates the still-experimental Stacked Borrows rules. Chances are it is not a problem in practice on the current version of the Rust compiler.

Could you re-test it with -Zmiri-tree-borrows flag to miri? If the issue is not reproducible there, then it's likely not a problem in practice.

@MolotovCherry
Copy link
Author

Hi, I re-tested with -Zmiri-tree-borrows as you asked, and miri didn't trigger

@Shnatsel
Copy link
Member

I see. The situation is unfortunate but shouldn't be posing a security issue right now.

LLVM does not perform optimizations based on the strict Stacked Borrows model, and likely never will, with the eventually accepted model likely being closer to Treed Borrows. So this should not result in miscompilations.

Since this issue is not security-relevant I'm going to go ahead and close this. But thank you for bringing this to our attention regardless, and please do report things that make miri complain with Treed Borrows enabled!

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

No branches or pull requests

2 participants