Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix potential integer overflows in WasmPtr memory access methods #2786

Merged
merged 2 commits into from
Feb 11, 2022
Merged

Fix potential integer overflows in WasmPtr memory access methods #2786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fe025cd
Fix potential integer overflows in WasmPtr memory access methods
Amanieu Feb 10, 2022
037c630
Merge branch 'master' into fix_str_overflow
ptitSeb Feb 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 11 additions & 13 deletions lib/api/src/js/ptr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,12 @@ impl<T: Copy + ValueType> WasmPtr<T, Item> {
/// This invariant will be enforced in the future.
#[inline]
pub fn deref<'a>(self, memory: &'a Memory) -> Option<WasmCell<T>> {
let total_len = (self.offset as usize) + mem::size_of::<T>();
if total_len > memory.size().bytes().0 || mem::size_of::<T>() == 0 {
let end = (self.offset as usize).checked_add(mem::size_of::<T>())?;
if end > memory.size().bytes().0 || mem::size_of::<T>() == 0 {
return None;
}
let subarray = memory.uint8view().subarray(self.offset, total_len as u32);

let subarray = memory.uint8view().subarray(self.offset, end as u32);
Some(WasmCell::new(subarray))
}
}
Expand All @@ -122,13 +123,12 @@ impl<T: Copy + ValueType> WasmPtr<T, Array> {
// gets the size of the item in the array with padding added such that
// for any index, we will always result an aligned memory access
let item_size = mem::size_of::<T>() as u32;
let slice_full_len = index + length;
let slice_full_len = index.checked_add(length)?;
let memory_size = memory.size().bytes().0 as u32;

if self.offset + (item_size * slice_full_len) > memory_size
|| self.offset >= memory_size
|| item_size == 0
{
let end = self
.offset
.checked_add(item_size.checked_mul(slice_full_len)?)?;
if end > memory_size || item_size == 0 {
return None;
}

Expand Down Expand Up @@ -175,10 +175,8 @@ impl<T: Copy + ValueType> WasmPtr<T, Array> {
///
/// an aliasing `WasmPtr` is used to mutate memory.
pub fn get_utf8_string(self, memory: &Memory, str_len: u32) -> Option<String> {
let memory_size = memory.size().bytes().0;
if self.offset as usize + str_len as usize > memory.size().bytes().0
|| self.offset as usize >= memory_size
{
let end = self.offset.checked_add(str_len)?;
if end as usize > memory.size().bytes().0 {
return None;
}

Expand Down
30 changes: 12 additions & 18 deletions lib/api/src/sys/ptr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,11 @@ impl<T: Copy + ValueType> WasmPtr<T, Item> {
/// This invariant will be enforced in the future.
#[inline]
pub fn deref<'a>(self, memory: &'a Memory) -> Option<WasmCell<'a, T>> {
if (self.offset as usize) + mem::size_of::<T>() > memory.size().bytes().0
|| mem::size_of::<T>() == 0
{
let end = (self.offset as usize).checked_add(mem::size_of::<T>())?;
if end > memory.size().bytes().0 || mem::size_of::<T>() == 0 {
return None;
}

unsafe {
let cell_ptr = align_pointer(
memory.view::<u8>().as_ptr().add(self.offset as usize) as usize,
Expand Down Expand Up @@ -140,15 +140,13 @@ impl<T: Copy + ValueType> WasmPtr<T, Array> {
// gets the size of the item in the array with padding added such that
// for any index, we will always result an aligned memory access
let item_size = mem::size_of::<T>();
let slice_full_len = index as usize + length as usize;
let slice_full_len = (index as usize).checked_add(length as usize)?;
let memory_size = memory.size().bytes().0;

if (self.offset as usize) + (item_size * slice_full_len) > memory_size
|| (self.offset as usize) >= memory_size
|| item_size == 0
{
let end = (self.offset as usize).checked_add(item_size.checked_mul(slice_full_len)?)?;
if end > memory_size || item_size == 0 {
return None;
}

let cell_ptrs = unsafe {
let cell_ptr = align_pointer(
memory.view::<u8>().as_ptr().add(self.offset as usize) as usize,
Expand Down Expand Up @@ -182,13 +180,11 @@ impl<T: Copy + ValueType> WasmPtr<T, Array> {
/// Additionally, if `memory` is dynamic, the caller must also ensure that `memory`
/// is not grown while the reference is held.
pub unsafe fn get_utf8_str<'a>(self, memory: &'a Memory, str_len: u32) -> Option<&'a str> {
let memory_size = memory.size().bytes().0;

if self.offset as usize + str_len as usize > memory.size().bytes().0
|| self.offset as usize >= memory_size
{
let end = self.offset.checked_add(str_len)?;
if end as usize > memory.size().bytes().0 {
return None;
}

let ptr = memory.view::<u8>().as_ptr().add(self.offset as usize) as *const u8;
let slice: &[u8] = std::slice::from_raw_parts(ptr, str_len as usize);
std::str::from_utf8(slice).ok()
Expand All @@ -198,10 +194,8 @@ impl<T: Copy + ValueType> WasmPtr<T, Array> {
///
/// an aliasing `WasmPtr` is used to mutate memory.
pub fn get_utf8_string(self, memory: &Memory, str_len: u32) -> Option<String> {
let memory_size = memory.size().bytes().0;
if self.offset as usize + str_len as usize > memory.size().bytes().0
|| self.offset as usize >= memory_size
{
let end = self.offset.checked_add(str_len)?;
if end as usize > memory.size().bytes().0 {
return None;
}

Expand Down