Skip to content

Commit

Permalink
Fix integer overflow in BlobVec::reserve_exact (#11234)
Browse files Browse the repository at this point in the history
# Objective

When `BlobVec::reserve` is called with an argument causing capacity
overflow, in release build capacity overflow is ignored, and capacity is
decreased.

I'm not sure it is possible to exploit this issue using public API of
`bevy_ecs`, but better fix it anyway.

## Solution

Check for capacity overflow.
  • Loading branch information
stepancheg authored Jan 6, 2024
1 parent 425570a commit a35a151
Showing 1 changed file with 23 additions and 5 deletions.
28 changes: 23 additions & 5 deletions crates/bevy_ecs/src/storage/blob_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,10 @@ impl BlobVec {
/// For ZST it panics unconditionally because ZST `BlobVec` capacity
/// is initialized to `usize::MAX` and always stays that way.
fn grow_exact(&mut self, increment: NonZeroUsize) {
// If we got here with ZST, we requested total capacity > usize::MAX.
assert!(self.item_layout.size() != 0, "ZST capacity overflow");

let new_capacity = self.capacity + increment.get();
let new_capacity = self
.capacity
.checked_add(increment.get())
.expect("capacity overflow");
let new_layout =
array_layout(&self.item_layout, new_capacity).expect("array layout should be valid");
let new_data = if self.capacity == 0 {
Expand Down Expand Up @@ -621,7 +621,7 @@ mod tests {
}

#[test]
#[should_panic(expected = "ZST capacity overflow")]
#[should_panic(expected = "capacity overflow")]
fn blob_vec_zst_size_overflow() {
// SAFETY: no drop is correct drop for `()`.
let mut blob_vec = unsafe { BlobVec::new(Layout::new::<()>(), None, 0) };
Expand All @@ -644,6 +644,24 @@ mod tests {
}
}

#[test]
#[should_panic(expected = "capacity overflow")]
fn blob_vec_capacity_overflow() {
// SAFETY: no drop is correct drop for `u32`.
let mut blob_vec = unsafe { BlobVec::new(Layout::new::<u32>(), None, 0) };

assert_eq!(0, blob_vec.capacity(), "Self-check");

OwningPtr::make(17u32, |ptr| {
// SAFETY: we push the value of correct type.
unsafe {
blob_vec.push(ptr);
}
});

blob_vec.reserve_exact(usize::MAX);
}

#[test]
fn aligned_zst() {
// NOTE: This test is explicitly for uncovering potential UB with miri.
Expand Down

0 comments on commit a35a151

Please sign in to comment.