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.

Sign up for GitHub

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

Jump to bottom

Unsoundness in crate anstream #156

Closed
burakemir opened this issue Jan 11, 2024 · 0 comments · Fixed by #158
Closed

Unsoundness in crate anstream #156

burakemir opened this issue Jan 11, 2024 · 0 comments · Fixed by #158

Comments

@burakemir
Copy link

There is an unsoundness issue with multibyte sequences.

When I give a valid UTF8 string "ö\x1b😀" as input to crates/anstream/src/adapter/strip.rs the code will be confused.
The UTF8 bytes are \xc3\xb6 then \x1b then \xf0\x9f\x98\x80.

When we loop over "non-printable bytes" \x1b\xf0 will be considered as some non-printable sequence...

I do not know whether it is a valid escape sequence or not, but it does not matter: we will produce a broken str from the incorrectly segmented bytes via str::from_utf8_unchecked, and that should never happen.

I have a tentative fix that makes the code sound, which I will reference after filing the issue (so I can reference the issue in the PR).

Full credit goes to @Ralith who reviewed this code and asked me to follow up.
@burakemir burakemir mentioned this issue Jan 11, 2024
Closed
epage added a commit to epage/anstyle that referenced this issue Jan 12, 2024
@epage
fix(stream): Don't break out of UTF8 character part-way
a54cb5b 
This fixes a soundness issue where we create invalid UTF-8 data and then
do a `str::from_unchecked` on release builds.

This ensures we ignore up-to the start of UTF-8 sequences and not
mid-way through.

Fixes rust-cli#156
@epage epage mentioned this issue Jan 12, 2024
Merged
alexanderkjall added a commit to alexanderkjall/advisory-db that referenced this issue Sep 8, 2024
@alexanderkjall
add information about unsoundness issue in anstream, found here: rust…
0ec0bd2 
…-cli/anstyle#156
@alexanderkjall alexanderkjall mentioned this issue Sep 8, 2024
Merged
Shnatsel pushed a commit to rustsec/advisory-db that referenced this issue Dec 4, 2024
@alexanderkjall
add information about unsoundness issue in anstream, found here: rust…
08da351 
…-cli/anstyle#156 (#2075)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(stream): Don't break out of UTF8 character part-way epage/anstyle
1 participant
@burakemir