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.

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

Unsoundness in parse_arguments #190

Open
lwz23 opened this issue Nov 25, 2024 · 1 comment
Open

Unsoundness in parse_arguments #190

lwz23 opened this issue Nov 25, 2024 · 1 comment

Comments

@lwz23
Copy link

lwz23 commented Nov 25, 2024

I notice the following function may have unsound problem

unsafe { slice::from_raw_parts(arguments, argc as usize).to_vec() }

pub fn parse_arguments(argc: Argc, arguments: *const AnyObject) -> Vec<AnyObject> {
    unsafe { slice::from_raw_parts(arguments, argc as usize).to_vec() }
}

Describe the bug
The parse_arguments function is unsound because it uses slice::from_raw_parts in an unsafe block without validating the input pointer (arguments) or the length (argc). This allows invalid parameters, such as null pointers or arbitrary memory addresses, to cause undefined behavior (UB). While I was unable to run the original environment due to missing Ruby dependencies, I created a simulated environment to replicate the behavior, and the issue persists.

To Reproduce
Steps to reproduce the behavior:

  1. Create a simulated version of parse_arguments with a mocked Value struct to avoid requiring Ruby.
  2. Pass an invalid pointer and length to parse_arguments.
  3. Observe that the program crashes due to memory access violations.
    Example code:
use std::{ptr, slice};

// Mock `Value` struct for simulation
#[derive(Clone, Debug)]
#[repr(C)]
pub struct Value {
    value: u64, // Placeholder field
}

// Type alias for argument count
pub type Argc = u32;

// Define `AnyObject` with a `Value` field
#[derive(Clone, Debug)]
#[repr(C)]
pub struct AnyObject {
    value: Value,
}

// Function to parse arguments
pub fn parse_arguments(argc: Argc, arguments: *const AnyObject) -> Vec<AnyObject> {
    unsafe { slice::from_raw_parts(arguments, argc as usize).to_vec() }
}

fn main() {
    // Simulating invalid pointer
    let invalid_ptr = 0xdeadbeef as *const AnyObject; // Arbitrary invalid address
    let argc = 10; // Length of the array

    // Call the function with invalid inputs
    let result = parse_arguments(argc, invalid_ptr);

    // Access the result to trigger undefined behavior
    println!("{:?}", result);
}

Expected behavior
The function should validate the pointer (arguments) and ensure it is non-null, properly aligned, and points to a valid memory region. Additionally, it should verify that argc does not exceed the accessible memory bounds. If invalid inputs are provided, the function should return an error instead of allowing unsafe operations.
output:

thread 'main' panicked at core\src\panicking.rs:223:5:
unsafe precondition(s) violated: slice::from_raw_parts requires the pointer to be aligned and non-null, and the total size of the slice not to exceed `isize::MAX`
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread caused non-unwinding panic. aborting.
error: process didn't exit successfully: `target\debug\lwz.exe` (exit code: 0xc0000409, STATUS_STACK_BUFFER_OVERRUN)
PS E:\Github\lwz>

Additional context
Due to the inability to install Ruby on my local machine, I simulated the environment by creating a mock Value struct as a placeholder. The original implementation in the rutie crate uses crate::rubysys::types::Value, which I replaced with the following mock:

#[derive(Clone, Debug)]
#[repr(C)]
pub struct Value {
    value: u64, // Placeholder field
}

This simulation replicates the behavior of parse_arguments without requiring Ruby dependencies.
Issue Analysis
The root cause of the issue is the unchecked usage of slice::from_raw_parts in the following line:
unsafe { slice::from_raw_parts(arguments, argc as usize).to_vec() }
This function assumes:
arguments is a valid pointer to a contiguous array of AnyObject.
argc is a valid length that does not exceed the allocated memory.
The memory referenced by arguments is accessible and lives long enough to create the slice.
Violating any of these assumptions leads to undefined behavior, as demonstrated by the runtime crash (STATUS_ACCESS_VIOLATION) in the simulated environment.

@lwz23
Copy link
Author

lwz23 commented Dec 1, 2024

ping?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant