Skip to content

Commit

Permalink
cryptovec: Fix a segmentation fault (#288)
Browse files Browse the repository at this point in the history
The `CryptoVec::resize` implementation was running into a segmentation
fault in some case. If the capacity of the vector was more than 0, and
that the new allocation failed, the call to
`std::ptr::copy_non_overlapping` would have a null pointer as
destination. This was very easy to trigger by a malicious peer, they
just had to send a packet with an announced size large enough for the
allocation to fail. The code now correctly panics, which would only end
the current thread and not crash the whole application without giving it
a chance to continue running.
  • Loading branch information
elegaanz authored May 25, 2024
1 parent f356408 commit e4584a4
Showing 1 changed file with 31 additions and 10 deletions.
41 changes: 31 additions & 10 deletions cryptovec/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,15 @@ impl CryptoVec {
let next_capacity = size.next_power_of_two();
let old_ptr = self.p;
let next_layout = std::alloc::Layout::from_size_align_unchecked(next_capacity, 1);
self.p = std::alloc::alloc_zeroed(next_layout);
let new_ptr = std::alloc::alloc_zeroed(next_layout);
if new_ptr.is_null() {
#[allow(clippy::panic)]
{
panic!("Realloc failed, pointer = {:?} {:?}", self, size)
}
}

self.p = new_ptr;
mlock(self.p, next_capacity);

if self.capacity > 0 {
Expand All @@ -261,15 +269,8 @@ impl CryptoVec {
std::alloc::dealloc(old_ptr, layout);
}

if self.p.is_null() {
#[allow(clippy::panic)]
{
panic!("Realloc failed, pointer = {:?} {:?}", self, size)
}
} else {
self.capacity = next_capacity;
self.size = size;
}
self.capacity = next_capacity;
self.size = size;
}
}
}
Expand Down Expand Up @@ -429,3 +430,23 @@ impl Drop for CryptoVec {
}
}
}

#[cfg(test)]
mod tests {
use super::*;

// If `resize` is called with a size that is too large to be allocated, it
// should panic, and not segfault or fail silently.
#[test]
fn large_resize_panics() {
let result = std::panic::catch_unwind(|| {
let mut vec = CryptoVec::new();
// Write something into the vector, so that there is something to
// copy when reallocating, to test all code paths.
vec.push(42);

vec.resize(1_000_000_000_000)
});
assert!(result.is_err());
}
}

0 comments on commit e4584a4

Please sign in to comment.