From f39e015163f4a1ddfa615cbb0f9a27999be2c20b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 4 Jun 2017 18:47:31 -0700 Subject: [PATCH] check for overflow when doing pointer arithmetic --- src/error.rs | 3 ++ src/eval_context.rs | 31 ++++++++++++++------- src/lvalue.rs | 8 +++--- src/memory.rs | 24 ++++++++++++---- src/terminator/intrinsic.rs | 2 +- src/terminator/mod.rs | 10 +++---- src/traits.rs | 10 +++---- src/value.rs | 4 +-- tests/compile-fail/ptr_offset_overflow.rs | 8 ++++++ tests/run-pass/ptr_arith_offset_overflow.rs | 9 ++++++ 10 files changed, 76 insertions(+), 33 deletions(-) create mode 100644 tests/compile-fail/ptr_offset_overflow.rs create mode 100644 tests/run-pass/ptr_arith_offset_overflow.rs diff --git a/src/error.rs b/src/error.rs index 702c2c4940f..42df2398b46 100644 --- a/src/error.rs +++ b/src/error.rs @@ -23,6 +23,7 @@ pub enum EvalError<'tcx> { }, ReadPointerAsBytes, InvalidPointerMath, + OverflowingPointerMath, ReadUndefBytes, DeadLocal, InvalidBoolOp(mir::BinOp), @@ -82,6 +83,8 @@ impl<'tcx> Error for EvalError<'tcx> { "a raw memory access tried to access part of a pointer value as raw bytes", EvalError::InvalidPointerMath => "attempted to do math or a comparison on pointers into different allocations", + EvalError::OverflowingPointerMath => + "attempted to do overflowing math on a pointer", EvalError::ReadUndefBytes => "attempted to read undefined bytes", EvalError::DeadLocal => diff --git a/src/eval_context.rs b/src/eval_context.rs index 8eba793c9f6..4b617621766 100644 --- a/src/eval_context.rs +++ b/src/eval_context.rs @@ -391,7 +391,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { // FIXME(solson) let dest_ptr = self.force_allocation(dest)?.to_ptr(); - let discr_dest = dest_ptr.offset(discr_offset); + let discr_dest = dest_ptr.offset(discr_offset)?; self.memory.write_uint(discr_dest, discr_val, discr_size)?; let dest = Lvalue::Ptr { @@ -550,7 +550,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { // FIXME(solson) let dest = self.force_allocation(dest)?.to_ptr(); - let dest = dest.offset(offset.bytes()); + let dest = dest.offset(offset.bytes())?; let dest_size = self.type_size(ty)? .expect("bad StructWrappedNullablePointer discrfield"); self.memory.write_int(dest, 0, dest_size)?; @@ -610,7 +610,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { let dest = self.force_allocation(dest)?.to_ptr(); for i in 0..length { - let elem_dest = dest.offset(i * elem_size); + let elem_dest = dest.offset(i * elem_size)?; self.write_value_to_ptr(value, elem_dest, elem_ty)?; } } @@ -841,11 +841,22 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { } } + pub(super) fn wrapping_pointer_offset(&self, ptr: Pointer, pointee_ty: Ty<'tcx>, offset: i64) -> EvalResult<'tcx, Pointer> { + // FIXME: assuming here that type size is < i64::max_value() + let pointee_size = self.type_size(pointee_ty)?.expect("cannot offset a pointer to an unsized type") as i64; + let offset = offset.overflowing_mul(pointee_size).0; + Ok(ptr.wrapping_signed_offset(offset)) + } + pub(super) fn pointer_offset(&self, ptr: Pointer, pointee_ty: Ty<'tcx>, offset: i64) -> EvalResult<'tcx, Pointer> { // FIXME: assuming here that type size is < i64::max_value() let pointee_size = self.type_size(pointee_ty)?.expect("cannot offset a pointer to an unsized type") as i64; - // FIXME: Check overflow, out-of-bounds - Ok(ptr.signed_offset(offset * pointee_size)) + // FIXME: Check out-of-bounds + return if let Some(offset) = offset.checked_mul(pointee_size) { + ptr.signed_offset(offset) + } else { + Err(EvalError::OverflowingPointerMath) + } } pub(super) fn eval_operand_to_primval(&mut self, op: &mir::Operand<'tcx>) -> EvalResult<'tcx, PrimVal> { @@ -1099,8 +1110,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { let field_1_ty = self.get_field_ty(ty, 1)?; let field_0_size = self.type_size(field_0_ty)?.expect("pair element type must be sized"); let field_1_size = self.type_size(field_1_ty)?.expect("pair element type must be sized"); - self.memory.write_primval(ptr.offset(field_0), a, field_0_size)?; - self.memory.write_primval(ptr.offset(field_1), b, field_1_size)?; + self.memory.write_primval(ptr.offset(field_0)?, a, field_0_size)?; + self.memory.write_primval(ptr.offset(field_1)?, b, field_1_size)?; Ok(()) } @@ -1217,7 +1228,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { Ok(Value::ByVal(PrimVal::Ptr(p))) } else { trace!("reading fat pointer extra of type {}", pointee_ty); - let extra = ptr.offset(self.memory.pointer_size()); + let extra = ptr.offset(self.memory.pointer_size())?; let extra = match self.tcx.struct_tail(pointee_ty).sty { ty::TyDynamic(..) => PrimVal::Ptr(self.memory.read_ptr(extra)?), ty::TySlice(..) | @@ -1402,8 +1413,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { } let src_field_offset = self.get_field_offset(src_ty, i)?.bytes(); let dst_field_offset = self.get_field_offset(dest_ty, i)?.bytes(); - let src_f_ptr = src_ptr.offset(src_field_offset); - let dst_f_ptr = dest.offset(dst_field_offset); + let src_f_ptr = src_ptr.offset(src_field_offset)?; + let dst_f_ptr = dest.offset(dst_field_offset)?; if src_fty == dst_fty { self.copy(src_f_ptr, dst_f_ptr, src_fty)?; } else { diff --git a/src/lvalue.rs b/src/lvalue.rs index 1d796d254d7..e4deddbfb48 100644 --- a/src/lvalue.rs +++ b/src/lvalue.rs @@ -270,7 +270,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { _ => offset.bytes(), }; - let ptr = base_ptr.offset(offset); + let ptr = base_ptr.offset(offset)?; let field_ty = self.monomorphize(field_ty, self.substs()); @@ -363,7 +363,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { let usize = self.tcx.types.usize; let n = self.value_to_primval(n_ptr, usize)?.to_u64()?; assert!(n < len, "Tried to access element {} of array/slice with length {}", n, len); - let ptr = base_ptr.offset(n * elem_size); + let ptr = base_ptr.offset(n * elem_size)?; (ptr, LvalueExtra::None) } @@ -384,7 +384,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { u64::from(offset) }; - let ptr = base_ptr.offset(index * elem_size); + let ptr = base_ptr.offset(index * elem_size)?; (ptr, LvalueExtra::None) } @@ -398,7 +398,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { let (elem_ty, n) = base.elem_ty_and_len(base_ty); let elem_size = self.type_size(elem_ty)?.expect("slice element must be sized"); assert!(u64::from(from) <= n - u64::from(to)); - let ptr = base_ptr.offset(u64::from(from) * elem_size); + let ptr = base_ptr.offset(u64::from(from) * elem_size)?; let extra = LvalueExtra::Length(n - u64::from(to) - u64::from(from)); (ptr, extra) } diff --git a/src/memory.rs b/src/memory.rs index e21b9c9e4d3..46d5968abac 100644 --- a/src/memory.rs +++ b/src/memory.rs @@ -60,20 +60,32 @@ impl Pointer { Pointer { alloc_id, offset } } - pub fn signed_offset(self, i: i64) -> Self { + pub fn wrapping_signed_offset<'tcx>(self, i: i64) -> Self { + Pointer::new(self.alloc_id, self.offset.wrapping_add(i as u64)) + } + + pub fn signed_offset<'tcx>(self, i: i64) -> EvalResult<'tcx, Self> { // FIXME: is it possible to over/underflow here? if i < 0 { // trickery to ensure that i64::min_value() works fine // this formula only works for true negative values, it panics for zero! let n = u64::max_value() - (i as u64) + 1; - Pointer::new(self.alloc_id, self.offset - n) + if let Some(res) = self.offset.checked_sub(n) { + Ok(Pointer::new(self.alloc_id, res)) + } else { + Err(EvalError::OverflowingPointerMath) + } } else { self.offset(i as u64) } } - pub fn offset(self, i: u64) -> Self { - Pointer::new(self.alloc_id, self.offset + i) + pub fn offset<'tcx>(self, i: u64) -> EvalResult<'tcx, Self> { + if let Some(res) = self.offset.checked_add(i) { + Ok(Pointer::new(self.alloc_id, res)) + } else { + Err(EvalError::OverflowingPointerMath) + } } pub fn points_to_zst(&self) -> bool { @@ -271,7 +283,7 @@ impl<'a, 'tcx> Memory<'a, 'tcx> { alloc.undef_mask.grow(amount, false); } else if size > new_size { self.memory_usage -= size - new_size; - self.clear_relocations(ptr.offset(new_size), size - new_size)?; + self.clear_relocations(ptr.offset(new_size)?, size - new_size)?; let alloc = self.get_mut(ptr.alloc_id)?; // `as usize` is fine here, since it is smaller than `size`, which came from a usize alloc.bytes.truncate(new_size as usize); @@ -919,7 +931,7 @@ impl<'a, 'tcx> Memory<'a, 'tcx> { fn check_relocation_edges(&self, ptr: Pointer, size: u64) -> EvalResult<'tcx> { let overlapping_start = self.relocations(ptr, 0)?.count(); - let overlapping_end = self.relocations(ptr.offset(size), 0)?.count(); + let overlapping_end = self.relocations(ptr.offset(size)?, 0)?.count(); if overlapping_start + overlapping_end != 0 { return Err(EvalError::ReadPointerAsBytes); } diff --git a/src/terminator/intrinsic.rs b/src/terminator/intrinsic.rs index 770cde77709..31886e9cc6d 100644 --- a/src/terminator/intrinsic.rs +++ b/src/terminator/intrinsic.rs @@ -46,7 +46,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { // FIXME: Switch to non-checked, wrapped version of pointer_offset let offset = self.value_to_primval(arg_vals[1], isize)?.to_i128()? as i64; let ptr = arg_vals[0].read_ptr(&self.memory)?; - let result_ptr = self.pointer_offset(ptr, substs.type_at(0), offset)?; + let result_ptr = self.wrapping_pointer_offset(ptr, substs.type_at(0), offset)?; self.write_primval(dest, PrimVal::Ptr(result_ptr), dest_ty)?; } diff --git a/src/terminator/mod.rs b/src/terminator/mod.rs index 49570c7ba5b..ebf8723e827 100644 --- a/src/terminator/mod.rs +++ b/src/terminator/mod.rs @@ -305,7 +305,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { match arg_val { Value::ByRef(ptr) => { for ((offset, ty), arg_local) in offsets.zip(fields).zip(arg_locals) { - let arg = Value::ByRef(ptr.offset(offset)); + let arg = Value::ByRef(ptr.offset(offset)?); let dest = self.eval_lvalue(&mir::Lvalue::Local(arg_local))?; trace!("writing arg {:?} to {:?} (type: {})", arg, dest, ty); self.write_value(arg, dest, ty)?; @@ -387,7 +387,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { ty::InstanceDef::Virtual(_, idx) => { let ptr_size = self.memory.pointer_size(); let (_, vtable) = self.eval_operand(&arg_operands[0])?.expect_ptr_vtable_pair(&self.memory)?; - let fn_ptr = self.memory.read_ptr(vtable.offset(ptr_size * (idx as u64 + 3)))?; + let fn_ptr = self.memory.read_ptr(vtable.offset(ptr_size * (idx as u64 + 3))?)?; let instance = self.memory.get_fn(fn_ptr.alloc_id)?; let mut arg_operands = arg_operands.to_vec(); let ty = self.operand_ty(&arg_operands[0]); @@ -473,7 +473,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { StructWrappedNullablePointer { nndiscr, ref discrfield, .. } => { let (offset, ty) = self.nonnull_offset_and_ty(adt_ty, nndiscr, discrfield)?; - let nonnull = adt_ptr.offset(offset.bytes()); + let nonnull = adt_ptr.offset(offset.bytes())?; trace!("struct wrapped nullable pointer type: {}", ty); // only the pointer part of a fat pointer is used for this space optimization let discr_size = self.type_size(ty)?.expect("bad StructWrappedNullablePointer discrfield"); @@ -654,7 +654,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { let val = self.value_to_primval(args[1], usize)?.to_u64()? as u8; let num = self.value_to_primval(args[2], usize)?.to_u64()?; if let Some(idx) = self.memory.read_bytes(ptr, num)?.iter().rev().position(|&c| c == val) { - let new_ptr = ptr.offset(num - idx as u64 - 1); + let new_ptr = ptr.offset(num - idx as u64 - 1)?; self.write_primval(dest, PrimVal::Ptr(new_ptr), dest_ty)?; } else { self.write_primval(dest, PrimVal::Bytes(0), dest_ty)?; @@ -666,7 +666,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { let val = self.value_to_primval(args[1], usize)?.to_u64()? as u8; let num = self.value_to_primval(args[2], usize)?.to_u64()?; if let Some(idx) = self.memory.read_bytes(ptr, num)?.iter().position(|&c| c == val) { - let new_ptr = ptr.offset(idx as u64); + let new_ptr = ptr.offset(idx as u64)?; self.write_primval(dest, PrimVal::Ptr(new_ptr), dest_ty)?; } else { self.write_primval(dest, PrimVal::Bytes(0), dest_ty)?; diff --git a/src/traits.rs b/src/traits.rs index 622eddfde1b..bf9e47da991 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -56,14 +56,14 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { let drop = self.memory.create_fn_alloc(drop); self.memory.write_ptr(vtable, drop)?; - self.memory.write_usize(vtable.offset(ptr_size), size)?; - self.memory.write_usize(vtable.offset(ptr_size * 2), align)?; + self.memory.write_usize(vtable.offset(ptr_size)?, size)?; + self.memory.write_usize(vtable.offset(ptr_size * 2)?, align)?; for (i, method) in ::rustc::traits::get_vtable_methods(self.tcx, trait_ref).enumerate() { if let Some((def_id, substs)) = method { let instance = ::eval_context::resolve(self.tcx, def_id, substs); let fn_ptr = self.memory.create_fn_alloc(instance); - self.memory.write_ptr(vtable.offset(ptr_size * (3 + i as u64)), fn_ptr)?; + self.memory.write_ptr(vtable.offset(ptr_size * (3 + i as u64))?, fn_ptr)?; } } @@ -88,8 +88,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { pub fn read_size_and_align_from_vtable(&self, vtable: Pointer) -> EvalResult<'tcx, (u64, u64)> { let pointer_size = self.memory.pointer_size(); - let size = self.memory.read_usize(vtable.offset(pointer_size))?; - let align = self.memory.read_usize(vtable.offset(pointer_size * 2))?; + let size = self.memory.read_usize(vtable.offset(pointer_size)?)?; + let align = self.memory.read_usize(vtable.offset(pointer_size * 2)?)?; Ok((size, align)) } diff --git a/src/value.rs b/src/value.rs index e812d116286..efe5aac71f5 100644 --- a/src/value.rs +++ b/src/value.rs @@ -90,7 +90,7 @@ impl<'a, 'tcx: 'a> Value { match *self { ByRef(ref_ptr) => { let ptr = mem.read_ptr(ref_ptr)?; - let vtable = mem.read_ptr(ref_ptr.offset(mem.pointer_size()))?; + let vtable = mem.read_ptr(ref_ptr.offset(mem.pointer_size())?)?; Ok((ptr, vtable)) } @@ -105,7 +105,7 @@ impl<'a, 'tcx: 'a> Value { match *self { ByRef(ref_ptr) => { let ptr = mem.read_ptr(ref_ptr)?; - let len = mem.read_usize(ref_ptr.offset(mem.pointer_size()))?; + let len = mem.read_usize(ref_ptr.offset(mem.pointer_size())?)?; Ok((ptr, len)) }, ByValPair(ptr, val) => { diff --git a/tests/compile-fail/ptr_offset_overflow.rs b/tests/compile-fail/ptr_offset_overflow.rs new file mode 100644 index 00000000000..fa93d0daf76 --- /dev/null +++ b/tests/compile-fail/ptr_offset_overflow.rs @@ -0,0 +1,8 @@ +//error-pattern: overflowing math on a pointer +fn main() { + let v = [1i8, 2]; + let x = &v[1] as *const i8; + // One of them is guaranteed to overflow + let _ = unsafe { x.offset(isize::max_value()) }; + let _ = unsafe { x.offset(isize::min_value()) }; +} diff --git a/tests/run-pass/ptr_arith_offset_overflow.rs b/tests/run-pass/ptr_arith_offset_overflow.rs new file mode 100644 index 00000000000..3383c3b8014 --- /dev/null +++ b/tests/run-pass/ptr_arith_offset_overflow.rs @@ -0,0 +1,9 @@ +fn main() { + let v = [1i16, 2]; + let x = &v[1] as *const i16; + // Adding 2*isize::max and then 1 is like substracting 1 + let x = x.wrapping_offset(isize::max_value()); + let x = x.wrapping_offset(isize::max_value()); + let x = x.wrapping_offset(1); + assert_eq!(unsafe { *x }, 1); +}