Pointer Offset Overflow in onig

[cuiwenhao123]

A pointer offset overflow vulnerability has been discovered in the Region::pos method of the rust-onig library. The method performs an unchecked conversion from usize to isize, which can lead to address calculation overflow in the subsequent ptr::offset operation.

Crash Information:
Error Type: unsafe precondition(s) violated: ptr::offset requires the address calculation to not overflow
Location: region.rs in Region::pos
Root Cause: Conversion of usize values greater than isize::MAX to isize causes integer overflow

The issue is here.

[djc]

For all of your advisories, please ask the maintainer whether they would be okay with an advisory being published.

[cuiwenhao123]

@djc Yes, the crate maintainer has agreed. Please see here

[djc]

Are you actually going to fix it upstream? How would this be triggered and what would the result be?

[cuiwenhao123]

YES, a very large usize value will overflow when converted to isize; Rust permits this conversion without triggering a panic. However, when such a value is later used with unsafe operations like ptr::offset, the resulting address calculation overflow is detected.

[djc]

Did you write code that can trigger this? Here's the code:

    /// Returns the start and end positions of the Nth capture group.
    ///
    /// Returns `None` if `pos` is not a valid capture group or if the
    /// capture group did not match anything. The positions returned
    /// are always byte indices with respect to the original string
    /// matched.
    pub fn pos(&self, pos: usize) -> Option<(usize, usize)> {
        if pos >= self.len() {
            return None;
        }
        let pos = pos as isize;
        let (beg, end) = unsafe { (*self.raw.beg.offset(pos), *self.raw.end.offset(pos)) };
        if beg != onig_sys::ONIG_REGION_NOTPOS {
            Some((beg as usize, end as usize))
        } else {
            None
        }
    }

As such, pos is checked against self.len(), which converts the onig_sys::OnigRegion::num_regs (an i32) to usize, after which it's converted back to usize. As such it seems to me like by construction, pos always fits into isize (and it is extremely unlikely self.len() is larger than isize::MAX anyway?).

[cuiwenhao123]

Thank you for your response. While your logic holds true for standard use cases, the issue I've identified arises from an integer overflow/truncation during the FFI call in reserve, which subsequently invalidates the safety check in pos.

Here is trigger code and the follow is step-by-step breakdown of how the safety check is bypassed:

let _local0 = onig::Region::with_capacity(_param0);
let result = onig::Region::pos(&_local0, _param1);

1. Truncation in reserve: In the reserve method, new_capacity: usize is cast to onig_sys::onig_region_resize(..., new_capacity as c_int). If new_capacity is larger than i32::MAX (e.g., 4076199192145508835 from my logs), it overflows and becomes a negative i32 (e.g., -1852753437).

2. C-side Storage: The Oniguruma C library sets region->num_regs to this negative value.

3. Sign Extension in len(): When Rust calls self.len(), it performs self.raw.num_regs as usize. Casting a negative i32 to usize results in a massive positive value due to sign extension (e.g., 18446744071856798179).

4. Bypassed Check in pos: Because self.len() is now a massive value (much larger than isize::MAX), the check if pos >= self.len() passes even for extremely large pos values.

5. Invalid Pointer Arithmetic: Finally, self.raw.beg.offset(pos) is called with a pos that is far beyond the actual allocated memory, triggering a panic in Rust's pointer overflow check.

Here is the output after executing the code:

Same root cause, but resulting in a different bug and security impact. Please see here.

I can provide a PoC if you're interested.