Skip to content

Commit

Permalink
Merge pull request #1120 from spacejam/tyler_fix_ivec_asmut
Browse files Browse the repository at this point in the history
Fix use-after-free bug in IVec::as_mut caused by missing ref keyword on Copy type
  • Loading branch information
spacejam authored Jul 12, 2020
2 parents b22dddc + 2d77f97 commit 674c6f7
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 6 deletions.
1 change: 1 addition & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ jobs:
name: Cargo Test on ${{ matrix.os }}
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]
steps:
Expand Down
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@
by the same data.
* #1108 conversions from `Box<[u8]>` to `IVec` are fixed.

## Bug Fixes

* Fixed a use-after-free caused by missing `ref` keyword
on a `Copy` type in a pattern match in `IVec::as_mut`.

# 0.32

## New Features
Expand Down
50 changes: 46 additions & 4 deletions src/ivec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ impl AsRef<[u8]> for IVec {
buf.get_unchecked(..*sz as usize)
},
IVecInner::Remote(buf) => buf,
IVecInner::Subslice { base, offset, len } => {
IVecInner::Subslice { ref base, offset, len } => {
&base[*offset..*offset + *len]
}
}
Expand All @@ -287,13 +287,13 @@ impl AsMut<[u8]> for IVec {
fn as_mut(&mut self) -> &mut [u8] {
self.make_mut();

match self.0 {
IVecInner::Inline(ref sz, mut buf) => unsafe {
match &mut self.0 {
IVecInner::Inline(ref sz, ref mut buf) => unsafe {
std::slice::from_raw_parts_mut(buf.as_mut_ptr(), *sz as usize)
},
IVecInner::Remote(ref mut buf) => Arc::get_mut(buf).unwrap(),
IVecInner::Subslice { ref mut base, offset, len } => {
&mut Arc::get_mut(base).unwrap()[offset..offset + len]
&mut Arc::get_mut(base).unwrap()[*offset..*offset + *len]
}
}
}
Expand Down Expand Up @@ -362,3 +362,45 @@ fn subslice_usage_01() {
let iv1 = IVec::from(vec![1, 2, 3]);
let _subslice = iv1.subslice(3, 1);
}

#[test]
fn ivec_as_mut_identity() {
let initial = &[1];
let mut iv = IVec::from(initial);
assert_eq!(&*initial, &*iv);
assert_eq!(&*initial, &mut *iv);
assert_eq!(&*initial, iv.as_mut());
}

#[cfg(test)]
mod qc {
use super::IVec;

fn prop_identity(ivec: IVec) -> bool {
let mut iv2 = ivec.clone();

if iv2 != ivec {
println!("expected clone to equal original");
return false;
}

if &*ivec != &mut *iv2 {
println!("expected AsMut to equal original");
return false;
}

if &*ivec != iv2.as_mut() {
println!("expected AsMut to equal original");
return false;
}

true
}

quickcheck::quickcheck! {
#[cfg_attr(miri, ignore)]
fn bool(item: IVec) -> bool {
prop_identity(item)
}
}
}
3 changes: 1 addition & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@
//clippy::integer_arithmetic,
clippy::invalid_upcast_comparisons,
clippy::items_after_statements,
clippy::map_entry,
clippy::map_flatten,
clippy::match_same_arms,
clippy::maybe_infinite_iter,
Expand All @@ -129,12 +130,10 @@
clippy::needless_continue,
clippy::needless_pass_by_value,
clippy::non_ascii_literal,
clippy::map_entry,
clippy::path_buf_push_overwrite,
clippy::print_stdout,
clippy::pub_enum_variant_names,
clippy::redundant_closure_for_method_calls,
clippy::map_unwrap_or,
clippy::shadow_reuse,
clippy::shadow_same,
clippy::shadow_unrelated,
Expand Down

0 comments on commit 674c6f7

Please sign in to comment.