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

SmallVec::insert is UB on large arguments #343

Closed
workingjubilee opened this issue Mar 20, 2024 · 3 comments
Closed

SmallVec::insert is UB on large arguments #343

workingjubilee opened this issue Mar 20, 2024 · 3 comments

Comments

@workingjubilee
Copy link
Contributor

This example fails in miri, you can try it on the Playground:

fn main() {
    use smallvec::{smallvec, SmallVec};

    // This SmallVec can hold up to 4 items on the stack:
    let mut v: SmallVec<[i32; 4]> = smallvec![1, 2, 3, 4];

    // It will automatically move its contents to the heap if
    // contains more than four items:
    v.push(5);

    // SmallVec points to a slice, so you can use normal slice
    // indexing and other methods to access its contents:
    v[0] = v[1] + v[2];
    v.sort();
    v.insert(10, 6);
}
@workingjubilee
Copy link
Contributor Author

The contract of pointer::add, for reference:

If any of the following conditions are violated, the result is Undefined Behavior:

  • Both the starting and resulting pointer must be either in bounds or one byte past the end of the same allocated object.
  • The computed offset, in bytes, cannot overflow an isize.
  • The offset being in bounds cannot rely on “wrapping around” the address space. That is, the infinite-precision sum must fit in a usize.

@workingjubilee workingjubilee changed the title SmallVec::insert is UB SmallVec::insert is UB on large arguments Mar 20, 2024
@mbrubeck
Copy link
Collaborator

This is a regression from #282. It affects versions >= 1.8.1, <= 1.13.1.

mbrubeck added a commit to mbrubeck/rust-smallvec that referenced this issue Mar 20, 2024
mbrubeck added a commit to mbrubeck/rust-smallvec that referenced this issue Mar 20, 2024
mbrubeck added a commit that referenced this issue Mar 20, 2024
@gendx
Copy link

gendx commented Jun 27, 2024

FYI, given the UB, I've opened an issue to file a security advisory for this: rustsec/advisory-db#1960.

There are more than 30k crates that transitively depend on smallvec's 1.x branch, and given that a fix is available I think folks who regularly use cargo audit will appreciate that the tooling warns them if they had locked an earlier version.

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

3 participants