Skip to content

Mutable slices as function parameters allow aliasing mutable references #40288

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
abonander opened this issue Mar 6, 2017 · 7 comments
Closed
Assignees
Labels
I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@abonander
Copy link
Contributor

abonander commented Mar 6, 2017

This currently affects all 3 release channels.

If you have a mutable slice of references as an input to a function, it allows borrows to escape and cause aliasing. The following example compiles and prints "3", but it shouldn't compile at all (Playground):

fn save_ref<'a>(refr: &'a i32, to: &mut [&'a i32]) {
    for val in &mut *to {
        *val = refr;
    }
}

fn main() {
    let ref init = 0i32;
    let ref mut refr = 1i32;

    let mut out = [init];
    
    save_ref(&*refr, &mut out);
    
    // This shouldn't be allowed as `refr` is borrowed
    *refr = 3;
    
    // Prints 3?!
    println!("{:?}", out[0]);
}

This appears to be tied to slices as the non-slice equivalent fails to compile as expected (Playground):

fn save_ref<'a>(refr: &'a i32, to: &mut &'a i32) {
    *to = refr;
}

fn main() {
    let ref mut refr = 1i32;
    let mut out = &0i32;
    
    save_ref(&*refr, &mut out);
    
    // ERROR: cannot assign to `*refr` because it is borrowed
    *refr = 3;

    println!("{:?}", out);
}

cc seanmonstar/httparse#34

@sfackler sfackler added I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 6, 2017
@alexcrichton
Copy link
Member

cc @rust-lang/compiler

@eddyb
Copy link
Member

eddyb commented Mar 6, 2017

This is genuinely scary. I would've expected [T; N]: Unsize<[T]> to link up the lifetimes, but if it doesn't... Borrows are based on lifetimes, the same lifetimes that prevent use-after-free. If this has to do with unsizing in general, then it could mean you can produce any lifetime you want?!

@eddyb
Copy link
Member

eddyb commented Mar 6, 2017

Single-function, use-after-free, and showing that the lifetime can become 'static:

fn prove_static(_: [&'static str; 1]) {}

fn main() {
    let mut out = ["foo"];
    {
        let mut x = String::from("bar");
        let slice: &mut [_] = &mut out;
        slice[0] = &x[..];
    }
    let _new = String::from("boo");
    println!("{}", out[0]);
    prove_static(out);
}

@eddyb
Copy link
Member

eddyb commented Mar 6, 2017

General-purpose lifetime_transmute (well, limited to references but still very much unsound):

fn prove_static<T: 'static + ?Sized>(_: &'static T) {}

fn lifetime_transmute<'a, T: ?Sized>(x: &'a T, y: &T) -> &'a T {
    let mut out = [x];
    (&mut out as &mut [_])[0] = y;
    out[0]
}

fn main() {
    prove_static(lifetime_transmute("", &String::from("foo")));
}

@eddyb
Copy link
Member

eddyb commented Mar 6, 2017

fn lifetime_transmute<'a, 'b, T: ?Sized>(x: &'a T, y: &'b T) -> &'a T {
    let mut out = [x];
    // Neither 'a nor 'b work here, they both error correctly.
    (&mut out as &mut [&'b T])[0] = y;
    out[0]
}

Maybe we're using subtyping in the wrong direction, or should be using equality instead?

EDIT: Yupp, subtyping alright (two more cases above and below).

EDIT2: Just confirmed it was introduced by #24619, you get an error in 1.0.

@abonander
Copy link
Contributor Author

Good stuff, glad I caught this.

@nikomatsakis
Copy link
Contributor

triage: P-high

@rust-highfive rust-highfive added the P-high High priority label Mar 9, 2017
alexcrichton added a commit to alexcrichton/rust that referenced this issue Mar 10, 2017
…r=nikomatsakis

Disallow subtyping between T and U in T: Unsize<U>.

Because `&mut T` can be coerced to `&mut U`, `T` and `U` must be unified invariantly. Fixes rust-lang#40288.
E.g. coercing `&mut [&'a X; N]` to `&mut [&'b X]` must require `'a` be equal to `'b`, otherwise you can convert between `&'a X` and `&'b X` (in either direction), potentially unsoundly lengthening lifetimes.

Subtyping here was introduced with `Unsize` in rust-lang#24619 (landed in 1.1, original PR is rust-lang#23785).
alexcrichton added a commit to alexcrichton/rust that referenced this issue Mar 10, 2017
…r=nikomatsakis

Disallow subtyping between T and U in T: Unsize<U>.

Because `&mut T` can be coerced to `&mut U`, `T` and `U` must be unified invariantly. Fixes rust-lang#40288.
E.g. coercing `&mut [&'a X; N]` to `&mut [&'b X]` must require `'a` be equal to `'b`, otherwise you can convert between `&'a X` and `&'b X` (in either direction), potentially unsoundly lengthening lifetimes.

Subtyping here was introduced with `Unsize` in rust-lang#24619 (landed in 1.1, original PR is rust-lang#23785).
arielb1 pushed a commit to arielb1/rust that referenced this issue Mar 10, 2017
…r=nikomatsakis

Disallow subtyping between T and U in T: Unsize<U>.

Because `&mut T` can be coerced to `&mut U`, `T` and `U` must be unified invariantly. Fixes rust-lang#40288.
E.g. coercing `&mut [&'a X; N]` to `&mut [&'b X]` must require `'a` be equal to `'b`, otherwise you can convert between `&'a X` and `&'b X` (in either direction), potentially unsoundly lengthening lifetimes.

Subtyping here was introduced with `Unsize` in rust-lang#24619 (landed in 1.1, original PR is rust-lang#23785).
arielb1 pushed a commit to arielb1/rust that referenced this issue Mar 10, 2017
…r=nikomatsakis

Disallow subtyping between T and U in T: Unsize<U>.

Because `&mut T` can be coerced to `&mut U`, `T` and `U` must be unified invariantly. Fixes rust-lang#40288.
E.g. coercing `&mut [&'a X; N]` to `&mut [&'b X]` must require `'a` be equal to `'b`, otherwise you can convert between `&'a X` and `&'b X` (in either direction), potentially unsoundly lengthening lifetimes.

Subtyping here was introduced with `Unsize` in rust-lang#24619 (landed in 1.1, original PR is rust-lang#23785).
arielb1 pushed a commit to arielb1/rust that referenced this issue Mar 10, 2017
…r=nikomatsakis

Disallow subtyping between T and U in T: Unsize<U>.

Because `&mut T` can be coerced to `&mut U`, `T` and `U` must be unified invariantly. Fixes rust-lang#40288.
E.g. coercing `&mut [&'a X; N]` to `&mut [&'b X]` must require `'a` be equal to `'b`, otherwise you can convert between `&'a X` and `&'b X` (in either direction), potentially unsoundly lengthening lifetimes.

Subtyping here was introduced with `Unsize` in rust-lang#24619 (landed in 1.1, original PR is rust-lang#23785).
arielb1 pushed a commit to arielb1/rust that referenced this issue Mar 11, 2017
…r=nikomatsakis

Disallow subtyping between T and U in T: Unsize<U>.

Because `&mut T` can be coerced to `&mut U`, `T` and `U` must be unified invariantly. Fixes rust-lang#40288.
E.g. coercing `&mut [&'a X; N]` to `&mut [&'b X]` must require `'a` be equal to `'b`, otherwise you can convert between `&'a X` and `&'b X` (in either direction), potentially unsoundly lengthening lifetimes.

Subtyping here was introduced with `Unsize` in rust-lang#24619 (landed in 1.1, original PR is rust-lang#23785).
arielb1 pushed a commit to arielb1/rust that referenced this issue Mar 11, 2017
…r=nikomatsakis

Disallow subtyping between T and U in T: Unsize<U>.

Because `&mut T` can be coerced to `&mut U`, `T` and `U` must be unified invariantly. Fixes rust-lang#40288.
E.g. coercing `&mut [&'a X; N]` to `&mut [&'b X]` must require `'a` be equal to `'b`, otherwise you can convert between `&'a X` and `&'b X` (in either direction), potentially unsoundly lengthening lifetimes.

Subtyping here was introduced with `Unsize` in rust-lang#24619 (landed in 1.1, original PR is rust-lang#23785).
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants