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

String::replace_range is unsound #81138

Closed
KamilaBorowska opened this issue Jan 17, 2021 · 1 comment · Fixed by #81169
Closed

String::replace_range is unsound #81138

KamilaBorowska opened this issue Jan 17, 2021 · 1 comment · Fixed by #81169
Labels
A-Unicode Area: Unicode C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-high High priority T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@KamilaBorowska
Copy link
Contributor

KamilaBorowska commented Jan 17, 2021

It's possible to use String::replace_range to create invalid strings containing broken Unicode. This happens because replace_range checks range bounds twice - first time to check parameter validity and second time for actually doing the splice.

use std::cell::Cell;
use std::ops::{Bound, RangeBounds};
use std::str;

struct EvilRange(Cell<bool>);

impl RangeBounds<usize> for EvilRange {
    fn start_bound(&self) -> Bound<&usize> {
        Bound::Included(if self.0.get() {
            &1
        } else {
            self.0.set(true);
            &0
        })
    }
    fn end_bound(&self) -> Bound<&usize> {
        Bound::Unbounded
    }
}

fn main() {
    let mut s = String::from("🦀");
    s.replace_range(EvilRange(Cell::new(false)), "");
    println!("{:?}", str::from_utf8(s.as_bytes()));
}

This will print Utf8Error showing that a string is not valid UTF-8.

@KamilaBorowska KamilaBorowska added the C-bug Category: This is a bug. label Jan 17, 2021
@Mark-Simulacrum Mark-Simulacrum added I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jan 17, 2021
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jan 17, 2021
@jyn514 jyn514 added the A-Unicode Area: Unicode label Jan 18, 2021
@rylev
Copy link
Member

rylev commented Jan 18, 2021

Assigning P-high as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

@rylev rylev added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jan 18, 2021
@bors bors closed this as completed in 7d7b22d Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Unicode Area: Unicode C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-high High priority T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants