Skip to content

Soundness: Unchecked Python FFI allocation failure in pystring_ascii_new leads to null pointer write #261

Description

@Manishearth

Note

This finding was identified during an agentic unsafe Rust code review performed by Gemini AI, followed by human review and verification.

The Issue

In pystring_ascii_new, pyo3::ffi::PyUnicode_New is invoked to allocate a compact ASCII string buffer on the Python heap:

/// Faster creation of PyString from an ASCII string, inspired by
/// <https://github.com/ijl/orjson/blob/3.10.0/src/str/create.rs#L41>
///
/// # Safety
///
/// `s` must be ASCII only
pub unsafe fn pystring_ascii_new<'py>(py: Python<'py>, s: &str) -> Bound<'py, PyString> {
unsafe {
#[cfg(not(any(PyPy, GraalPy, Py_LIMITED_API)))]
{
let ptr = pyo3::ffi::PyUnicode_New(s.len() as isize, 127);
// see https://github.com/pydantic/jiter/pull/72#discussion_r1545485907
debug_assert_eq!(pyo3::ffi::PyUnicode_KIND(ptr), pyo3::ffi::PyUnicode_1BYTE_KIND);
let data_ptr = pyo3::ffi::PyUnicode_DATA(ptr).cast();
core::ptr::copy_nonoverlapping(s.as_ptr(), data_ptr, s.len());
core::ptr::write(data_ptr.add(s.len()), 0);
Bound::from_owned_ptr(py, ptr).cast_into_unchecked()
}
#[cfg(any(PyPy, GraalPy, Py_LIMITED_API))]
{
PyString::new(py, s)
}
}
}

PyUnicode_New returns NULL (0x0) and sets a MemoryError exception if heap memory allocation fails. The implementation fails to verify whether ptr is NULL before proceeding. In release builds (where debug_assert_eq! is stripped), PyUnicode_DATA(ptr) evaluates pointer arithmetic (PyASCIIObject *)NULL + 1, computing address offset sizeof(PyASCIIObject) (typically 0x30 bytes).

Subsequently, core::ptr::copy_nonoverlapping attempts to copy s.len() bytes from the source string to memory address 0x30, followed by core::ptr::write placing a null terminator at 0x30 + s.len().

This is UB. It's unlikely to be a very exploitable UB, but it's UB nevertheless.

Suggested Fix

Verify that ptr is non-null before performing pointer arithmetic or copying bytes. If ptr is null, propagate the Python exception or return an allocation error:

 let ptr = pyo3::ffi::PyUnicode_New(s.len() as isize, 127);
+if ptr.is_null() {
+    return Err(PyErr::fetch(py));
+}
 debug_assert_eq!(pyo3::ffi::PyUnicode_KIND(ptr), pyo3::ffi::PyUnicode_1BYTE_KIND);

Note

The full audit report below also contains additional minor findings (such as missing safety comments or undocumented FFI assumptions) that are probably worth fixing as well but not the primary goal of this issue. The audit report has not been human-reviewed, it may contain misleading claims.

Full Gemini Codebase Audit Report Appendix

Unsafe Rust Review: jiter (v0_11)

Overall Safety Assessment

jiter is a high-performance iterable JSON parser designed primarily to accelerate JSON deserialization in Python via PyO3. The crate exposes three primary interfaces: JsonValue (an AST representation), Jiter (a streaming pull-iterator), and PythonParse (direct construction of Python objects from JSON data).

The density of unsafe code in jiter is moderate and localized across three distinct architectural surfaces:

  1. String Decoding Safety Boundary (StringOutput): To optimize PyO3 object instantiations, string scanning tracks whether JSON string chunks contain exclusively ASCII characters (<= 0x7F). StringOutput::data and StringOutput::tape encapsulate this boolean flag as an unsafe safety invariant.
  2. SIMD Acceleration (simd_aarch64.rs): On ARM64 (aarch64), jiter utilizes NEON vector intrinsics to scan 16-byte chunks of input for digits or string delimiter masks. This involves raw transmutations between arrays and SIMD vector registers, unaligned pointer loads (vld1q_u8), and unchecked slice indexing (get_unchecked).
  3. Python C API / PyO3 FFI (python.rs, py_string_cache.rs): Direct invocations of Python C API functions (PyDict_SetItem, PyUnicode_New) to construct Python dictionaries and string objects directly on the Python heap, bypassing standard PyO3 binding overhead.

Architecturally, the core parsing algorithms (both fallback and SIMD) correctly maintain UTF-8 and ASCII invariants. However, from a strict proof-obligation standpoint, the auditing revealed critical vulnerabilities and documentation deficiencies. Most severely, raw FFI allocations via PyUnicode_New lack out-of-memory (NULL) checks, resulting in deterministic segmentation faults or memory corruption upon Python heap allocation failure. Furthermore, the codebase exhibits widespread omission of safety documentation, with almost no // SAFETY: proof comments justifying unsafe blocks across the SIMD and Python FFI modules.

Critical Findings

1. Unchecked OOM allocation failure in pystring_ascii_new leading to deterministic Segmentation Fault / UB (src/py_string_cache.rs:255-261) 🔴 🚨

  • Priority: 🔴 High
  • Threat Vector: 🚨 Untrusted Input
  • Bug Type: Unchecked FFI Allocation Failure

In pystring_ascii_new, pyo3::ffi::PyUnicode_New is invoked to allocate a compact ASCII string buffer on the Python heap:

let ptr = pyo3::ffi::PyUnicode_New(s.len() as isize, 127);
// see https://github.com/pydantic/jiter/pull/72#discussion_r1545485907
debug_assert_eq!(pyo3::ffi::PyUnicode_KIND(ptr), pyo3::ffi::PyUnicode_1BYTE_KIND);
let data_ptr = pyo3::ffi::PyUnicode_DATA(ptr).cast();
core::ptr::copy_nonoverlapping(s.as_ptr(), data_ptr, s.len());
core::ptr::write(data_ptr.add(s.len()), 0);
Bound::from_owned_ptr(py, ptr).cast_into_unchecked()

In the Python C API specification, PyUnicode_New returns NULL (0x0) and sets a MemoryError exception if heap memory allocation fails. The implementation fails to verify whether ptr is NULL before proceeding. In release builds (where debug_assert_eq! is stripped), PyUnicode_DATA(ptr) evaluates pointer arithmetic (PyASCIIObject *)NULL + 1, computing address offset sizeof(PyASCIIObject) (typically 0x30 bytes).

Subsequently, core::ptr::copy_nonoverlapping attempts to copy s.len() bytes from the source string to memory address 0x30, followed by core::ptr::write placing a null terminator at 0x30 + s.len(). On standard desktop operating systems (Linux, Windows, macOS), address 0x30 resides in the unmapped null guard page, triggering an immediate segmentation fault (SIGSEGV). In embedded environments, custom allocators, or runtimes where low memory address mapping is permitted, this causes arbitrary memory corruption.

Under Rust safety contracts, safe APIs (PythonParse::parse -> StringMaybeCache::get_key) must remain memory-safe under all possible runtime conditions, including memory allocation exhaustion. Triggering Undefined Behavior or abnormal process termination via SIGSEGV on OOM violates safety expectations for safe public APIs.

Fishy Findings

1. Unsafe pointer read in load_slice relying on stripped debug_assert_eq! (src/simd_aarch64.rs:257-260) 🟡 ⚠️

  • Priority: 🟡 Low

  • Threat Vector: ⚠️ Accidental Misuse

  • Bug Type: Erased Debug Assertion

fn load_slice(bytes: &[u8]) -> SimdVecu8_16 {
    debug_assert_eq!(bytes.len(), 16);
    unsafe { simd_load_16(bytes.as_ptr()) }
}

load_slice performs an unaligned 16-byte SIMD vector load (vld1q_u8) from a raw pointer derived from bytes. To establish bounds safety, it relies on debug_assert_eq!(bytes.len(), 16). However, debug_assert_eq! is completely erased in release compilations. While current internal callers in simd_aarch64.rs pass slices derived from .get(index..index + 16), enforcing a critical memory safety bound using debug_assert_eq! on a dynamically sized slice &[u8] is fragile and hazardous during refactoring. The helper should accept a statically sized array reference &[u8; 16] or use bytes.try_into().unwrap().

2. Typo in # Safety docstrings across StringOutput constructors (src/string_decoder.rs:77, 87) 🟡 ⚠️

  • Priority: 🟡 Low

  • Threat Vector: ⚠️ Accidental Misuse

  • Bug Type: Documentation Typo

The docstrings defining the safety contract for StringOutput::tape and StringOutput::data misspell parameter ascii_only as accii_only:

/// `accii_only` must only be set to true if the string is ascii only

While minor, imprecise terminology in formal safety contracts undermines rigorous proof checking.

3. Reliance on assert_ne! for FFI error handling in PyDict_SetItem (src/python.rs:191-195) 🟡 🚨

  • Priority: 🟡 Low
  • Threat Vector: 🚨 Untrusted Input
  • Bug Type: Improper FFI Error Propagation

In parse_object, dictionary insertion executes unsafe { ffi::PyDict_SetItem(...) } followed by assert_ne!(r, -1, "PyDict_SetItem failed"). The author included a helpful comment explaining that panicking here provides a 14% speedup over returning a JsonResult. While PyDict_SetItem will only fail on allocation failure or unhashable keys (and keys here are guaranteed to be PyString), documenting how this performance tradeoff interacts with PyO3 exception handling will make the design choice clear and self-documenting for future maintainers.

Missing Safety Comments

src/string_decoder.rs 🔴

-src/string_decoder.rs:126: Calling unsafe { StringOutput::data(s, ascii_only) }.

// SAFETY: `decode_chunk` scanned `data[start..index]` with `ascii_only` initially `true`. Any byte >= 0x80 or unicode escape sequence would set `ascii_only` to `false`. Thus if `ascii_only` is true, `s` contains exclusively ASCII bytes (<= 0x7F).

-src/string_decoder.rs:166: Calling unsafe { StringOutput::tape(s, ascii_only) }.

// SAFETY: `tape` accumulated bytes during string scanning. `ascii_only` tracks whether any byte pushed or extended onto `tape` was non-ASCII. Therefore if `ascii_only` is true, `s` contains exclusively ASCII characters.

-src/string_decoder.rs:177: Calling unsafe { StringOutput::tape(s, ascii_only) }.

// SAFETY: `tape` accumulated bytes during string scanning. `ascii_only` tracks whether any byte pushed or extended onto `tape` was non-ASCII. Therefore if `ascii_only` is true, `s` contains exclusively ASCII characters.

-src/string_decoder.rs:187: Calling unsafe { StringOutput::tape(s, ascii_only) }.

// SAFETY: `tape` accumulated bytes during string scanning. `ascii_only` tracks whether any byte pushed or extended onto `tape` was non-ASCII. Therefore if `ascii_only` is true, `s` contains exclusively ASCII characters.

src/python.rs 🔴

-src/python.rs:191: Calling unsafe { ffi::PyDict_SetItem(...) }.

// SAFETY: `dict`, `key`, and `value` are valid `Bound` references guaranteeing valid non-null pointers to PyObject/PyDict instances. The `py: Python<'py>` token guarantees the Python GIL is currently held. `PyDict_SetItem` does not steal references to key or value.

src/py_string_cache.rs 🔴

-src/py_string_cache.rs:80: Calling unsafe { pystring_fast_new_maybe_ascii(...) }.

// SAFETY: `StringOutput::ascii_only()` is a safety invariant guaranteeing that `string_output.as_str()` contains exclusively ASCII characters when it returns `true`.

-src/py_string_cache.rs:88: Calling unsafe { pystring_fast_new_maybe_ascii(...) }.

// SAFETY: `StringOutput::ascii_only()` is a safety invariant guaranteeing that `string_output.as_str()` contains exclusively ASCII characters when it returns `true`.

-src/py_string_cache.rs:139, 141: Inside unsafe fn cached_py_string_maybe_ascii, calling unsafe fn get_or_insert and unsafe fn pystring_fast_new_maybe_ascii.

// SAFETY: `cached_py_string_maybe_ascii` requires its caller to uphold that if `ascii_only` is true, `s` is ASCII only. This invariant is directly forwarded to `get_or_insert` and `pystring_fast_new_maybe_ascii`.

-src/py_string_cache.rs:255-261: Inside unsafe fn pystring_ascii_new, calling raw FFI functions and pointer operations (PyUnicode_New, PyUnicode_KIND, PyUnicode_DATA, ptr::copy_nonoverlapping, ptr::write, Bound::from_owned_ptr, cast_into_unchecked).

// SAFETY: `PyUnicode_New` allocates a compact ASCII buffer of `s.len() + 1` bytes. (Assuming `ptr` is verified non-NULL): `PyUnicode_DATA(ptr)` points to a writeable buffer of at least `s.len() + 1` bytes. `copy_nonoverlapping` copies `s.len()` valid bytes from `s.as_ptr()`, and `write` places the null terminator at index `s.len()`. `Bound::from_owned_ptr` assumes a valid non-null reference.

src/simd_aarch64.rs 🔴

-src/simd_aarch64.rs:55: Inside macro simd_const!, calling unsafe { transmute($array) }.

// SAFETY: All call sites pass byte or integer arrays whose byte sizes exactly match the target NEON vector type (`uint8x8_t` = 8 bytes, `uint8x16_t` = 16 bytes, etc.).

-src/simd_aarch64.rs:81: Calling unsafe { full_calc(byte_vec, 16) }.

// SAFETY: `full_calc` requires `last_digit` to be between 9 and 16. Here literal `16` is passed. `byte_vec` contains valid ASCII digit characters as verified by `is_zero(digit_mask)`.

-src/simd_aarch64.rs:91: Calling unsafe { first_half_calc(byte_vec, last_digit) }.

// SAFETY: `first_half_calc` requires `last_digit <= 8`. This is guaranteed by the preceding branch condition `else if last_digit <= 8`.

-src/simd_aarch64.rs:95: Calling unsafe { full_calc(byte_vec, last_digit) }.

// SAFETY: `full_calc` requires `9 <= last_digit <= 16`. Since `is_zero` was false, `last_digit <= 16`. Since `last_digit <= 8` was false, `last_digit >= 9`.

-src/simd_aarch64.rs:107: Inside get_digit_mask, calling unsafe { simd_or_16(...) }.

// SAFETY: NEON intrinsics require `aarch64` target architecture, guaranteed by `#[cfg(target_arch = "aarch64")]` on this module.

-src/simd_aarch64.rs:115: unsafe fn first_half_calc. Missing # Safety doc comment on function declaration explaining caller preconditions (last_digit <= 8), as well as internal transmute calls at lines 122 and 147.

// SAFETY: Transmuting NEON vector types to byte arrays of identical byte size (8 bytes).

-src/simd_aarch64.rs:150: unsafe fn full_calc. Missing # Safety doc comment on function declaration explaining caller preconditions (9 <= last_digit <= 16), as well as internal transmute call at line 178.

// SAFETY: Transmuting 16-byte NEON vector registers to `[u64; 2]` (16 bytes).

-src/simd_aarch64.rs:184: Inside next_is_float, calling unsafe { data.get_unchecked(index) }.

// SAFETY: `next_is_float` is called from `decode_int_chunk` after verifying `data.get(index..index + 16).is_some()`. `last_digit` is the byte offset of the first non-digit byte within that 16-byte slice. Since `is_zero(digit_mask)` was false, there is at least one non-digit byte in the slice, so `last_digit < 16`. Thus `index + last_digit < index + 16 <= data.len()`.

-src/simd_aarch64.rs:210: Calling unsafe { transmute(byte_vec) }.

// SAFETY: `uint8x16_t` and `[u8; 16]` both have a size of exactly 16 bytes.

-src/simd_aarch64.rs:227: Inside string_ascii_mask, calling unsafe { simd_or_16(...) }.

// SAFETY: NEON intrinsics require `aarch64` target architecture, guaranteed by module conditional compilation.

-src/simd_aarch64.rs:242: Inside find_end, calling unsafe { transmute(digit_mask) }.

// SAFETY: `uint8x16_t` and `[u64; 2]` both have a size of exactly 16 bytes.

-src/simd_aarch64.rs:253: Inside is_zero, calling unsafe { transmute(vec) }.

// SAFETY: `uint8x16_t` and `[u64; 2]` both have a size of exactly 16 bytes.

-src/simd_aarch64.rs:259: Inside load_slice, calling unsafe { simd_load_16(bytes.as_ptr()) }.

// SAFETY: `simd_load_16` (`vld1q_u8`) requires `bytes.as_ptr()` to be valid for reading 16 contiguous bytes. `load_slice` is an internal helper called only when `bytes` is a 16-byte slice obtained via `.get(index..index + 16)`.

tests/python.rs 🔴

-tests/python.rs:77: Calling unsafe { pystring_ascii_new(py, json) }.

// SAFETY: Literal `"100abc"` is ASCII only.

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