Skip to content

Commit

Permalink
Merge pull request #294 from dtolnay/isize
Browse files Browse the repository at this point in the history
Check for isize overflow before Layout construction
  • Loading branch information
dtolnay authored Oct 4, 2022
2 parents 1623283 + 6b0af67 commit 626d7c7
Showing 1 changed file with 18 additions and 2 deletions.
20 changes: 18 additions & 2 deletions src/identifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,13 @@
// allows size_of::<Version>() == size_of::<Option<Version>>().

use crate::alloc::alloc::{alloc, dealloc, handle_alloc_error, Layout};
use core::isize;
use core::mem;
use core::num::{NonZeroU64, NonZeroUsize};
use core::ptr::{self, NonNull};
use core::slice;
use core::str;
use core::usize;

const PTR_BYTES: usize = mem::size_of::<NonNull<u8>>();

Expand Down Expand Up @@ -103,6 +105,7 @@ impl Identifier {
// SAFETY: string must be ASCII and not contain \0 bytes.
pub(crate) unsafe fn new_unchecked(string: &str) -> Self {
let len = string.len();
debug_assert!(len <= isize::MAX as usize);
match len as u64 {
0 => Self::empty(),
1..=8 => {
Expand All @@ -118,8 +121,21 @@ impl Identifier {
// SAFETY: len is in a range that does not contain 0.
let size = bytes_for_varint(unsafe { NonZeroUsize::new_unchecked(len) }) + len;
let align = 2;
// On 32-bit and 16-bit architecture, check for size overflowing
// isize::MAX. Making an allocation request bigger than this to
// the allocator is considered UB. All allocations (including
// static ones) are limited to isize::MAX so we're guaranteed
// len <= isize::MAX, and we know bytes_for_varint(len) <= 5
// because 128**5 > isize::MAX, which means the only problem
// that can arise is when isize::MAX - 5 <= len <= isize::MAX.
// This is pretty much guaranteed to be malicious input so we
// don't need to care about returning a good error message.
if mem::size_of::<usize>() < 8 {
let max_alloc = usize::MAX / 2 - align;
assert!(size <= max_alloc);
}
// SAFETY: align is not zero, align is a power of two, and
// rounding size up to align does not overflow usize::MAX.
// rounding size up to align does not overflow isize::MAX.
let layout = unsafe { Layout::from_size_align_unchecked(size, align) };
// SAFETY: layout's size is nonzero.
let ptr = unsafe { alloc(layout) };
Expand Down Expand Up @@ -200,7 +216,7 @@ impl Clone for Identifier {
let size = bytes_for_varint(len) + len.get();
let align = 2;
// SAFETY: align is not zero, align is a power of two, and rounding
// size up to align does not overflow usize::MAX. This is just
// size up to align does not overflow isize::MAX. This is just
// duplicating a previous allocation where all of these guarantees
// were already made.
let layout = unsafe { Layout::from_size_align_unchecked(size, align) };
Expand Down

0 comments on commit 626d7c7

Please sign in to comment.