Skip to content

Unsoundness in PtrIter::next #52

@lwz23

Description

@lwz23

Hello, thank you for your contribution in this project, I am scanning the unsoundness problem in rust project.
I notice the following code:

pub struct PtrIter<'a, T> {
    offset: usize,
    len: usize,
    ptr: *const T,
    _marker: PhantomData<&'a ()>,
}

impl<'a, T> Iterator for PtrIter<'a, T>
where
    T: Copy,
{
    type Item = T;

    fn next(&mut self) -> Option<T> {
        if self.offset >= self.len {
            return None;
        }
        let item = unsafe { *self.ptr.add(self.offset) };
        self.offset += 1;
        Some(item)
    }
}

impl<'a, T> PtrIter<'a, T> {
    pub fn new(len: c_int, ptr: *const T) -> Self {
        Self {
            offset: 0,
            len: len as usize,
            ptr,
            _marker: PhantomData,
        }
    }
}

Considering that pub mod iter, and new is also a pub function. I assume that users can directly call this function. This potential situation could result in *self.ptr being dereference a null pointer, and directly dereferencing it in next method might trigger undefined behavior (UB). For safety reasons, I felt it necessary to report this issue. If you have performed checks elsewhere that ensure this is safe, please don’t take offense at my raising this issue.
I suggest Several possible fixes:

  1. If there is no external usage for PtrIter or new, they should not marked as pub, at least its new should not marked as pub
  2. new method should add additional check for null pointer.
  3. mark new method as unsafe and proper doc to let users know that they should provide valid Pointers.

here is my PoC:

//! Iterator over sqlite argc/argv pair
use core::ffi::c_int;
use core::marker::PhantomData;
use std::ptr;

/// Iterator over sqlite pointers
#[derive(Debug)]
pub struct PtrIter<'a, T> {
    offset: usize,
    len: usize,
    ptr: *const T,
    _marker: PhantomData<&'a ()>,
}

impl<'a, T> Iterator for PtrIter<'a, T>
where
    T: Copy,
{
    type Item = T;

    fn next(&mut self) -> Option<T> {
        if self.offset >= self.len {
            return None;
        }
        let item = unsafe { *self.ptr.add(self.offset) };
        self.offset += 1;
        Some(item)
    }
}

impl<'a, T> PtrIter<'a, T> {
    pub fn new(len: c_int, ptr: *const T) -> Self {
        Self {
            offset: 0,
            len: len as usize,
            ptr,
            _marker: PhantomData,
        }
    }
}
fn main() {
    let mut a: PtrIter<'_, i32> = PtrIter::new(100, ptr::null());
    let b = a.next();
    println!("{:?}", b);
}

result:

PS E:\Github\lwz> cargo run
   Compiling lwz v0.1.0 (E:\Github\lwz)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.19s
     Running `target\debug\lwz.exe`
error: process didn't exit successfully: `target\debug\lwz.exe` (exit code: 0xc0000005, STATUS_ACCESS_VIOLATION)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions