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

Add unsound const-cstr #1613

Merged
merged 4 commits into from
Mar 12, 2023
Merged

Add unsound const-cstr #1613

merged 4 commits into from
Mar 12, 2023

Conversation

oherrala
Copy link
Contributor

const-cstr is crate by @abonander who has archived bunch of his Rust crates in GitHub.

See communication attempts on other crates:

@Nugine
Copy link
Contributor

Nugine commented Mar 6, 2023

Alternatives:

@pinkforest
Copy link
Contributor

btw const_cstr doesn't check for any interior nul bytes

Both the constructor and the canonical macro allows this:

const_cstr! {
    FOO = "\u{0}💖\u{0}";
}

let foo = FOO.as_cstr();

This violates the safety contract of ffi::CStr::from_bytes_with_nul_unchecked that is used in as_cstr()

ConstCStr::as_cstr assert only checks the last byte is nul that is relied for safety:
https://github.com/abonander/const-cstr/blob/master/src/lib.rs#L122

Also because unicode(tm) vs bytes - as_str goes:

"\x00\xf0\x9f\x92\x96\x00" vs ConstCStr { val: "\0💖\0" }

@pinkforest
Copy link
Contributor

pinkforest commented Mar 12, 2023

Ok I've filled the advisory with some details - please check if anything missed. Thanks

@pinkforest pinkforest added Unsound Informational / Unsound Unmaintained Informational / Unmaintained Propose-Merge Propose-Merge labels Mar 12, 2023
@pinkforest pinkforest changed the title Add unmaintained advisory for const-cstr Add unsound const-cstr Mar 12, 2023
@oherrala
Copy link
Contributor Author

@pinkforest LGTM 👍

@pinkforest pinkforest merged commit 5c42175 into rustsec:main Mar 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Propose-Merge Propose-Merge Unmaintained Informational / Unmaintained Unsound Informational / Unsound
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants