Null pointer dereference in `FFI_ArrowSchema::with_metadata` when used with empty schema

[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

FFI_ArrowSchema::empty() initializes private_data to null.

arrow-rs/arrow-schema/src/ffi.rs

Line 268 in 1583c8e

private_data: std::ptr::null_mut(),

with_metadata calls Box::from_raw on this value (unconditionally, I don't see an early return here)

arrow-rs/arrow-schema/src/ffi.rs

Lines 232 to 236 in 1583c8e

	unsafe {
	let mut private_data = Box::from_raw(self.private_data as *mut SchemaPrivateData);
	private_data.metadata = new_metadata;
	self.private_data = Box::into_raw(private_data) as *mut c_void;
	}

Minimal Reproduction

use arrow_schema::ffi::FFI_ArrowSchema;

fn main() {
    let schema = FFI_ArrowSchema::empty();
    // calling with_metadata on empty schema invokes Box::from_raw(null)
    let _ = schema.with_metadata([("key", "value")]);
}

error: Undefined Behavior: constructing invalid value of type std::ptr::NonNull<arrow_schema::ffi::SchemaPrivateData>: at .pointer, encountered 0, but expected something greater or equal to 1
  --> /usr/local/google/home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/unique.rs:88:36
   |
88 | ...er: NonNull::new_unchecked(ptr), _marker: PhantomData } }
   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
   = note: stack backtrace:
           0: std::ptr::Unique::<arrow_schema::ffi::SchemaPrivateData>::new_unchecked
               at /usr/local/google/home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/unique.rs:88:36: 88:63
           1: std::boxed::Box::<arrow_schema::ffi::SchemaPrivateData>::from_raw_in
               at /usr/local/google/home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1560:22: 1560:48
           2: std::boxed::Box::<arrow_schema::ffi::SchemaPrivateData>::from_raw
               at /usr/local/google/home/manishearth/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1323:18: 1323:48
           3: arrow_schema::ffi::FFI_ArrowSchema::with_metadata::<[(&str, &str); 1], &str>
               at /usr/local/google/home/manishearth/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/arrow-schema-57.3.1/src/ffi.rs:224:36: 224:94
           4: main
               at src/bin/repro1.rs:6:13: 6:53

Suggested Fix

Apply necessary runtime bounds checks or formalize the unsafe fn safety contract pre-conditions.

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: arrow_schema (v57)

Overall Safety Assessment

The arrow_schema crate contains a small amount of unsafe Rust code, entirely located in src/ffi.rs to support the Arrow C Data Interface. The density of unsafe code is low, but the risk is high because several safe public APIs wrap unsafe operations without adequate validation, leading to critical soundness bugs. Furthermore, safety documentation is almost entirely absent: there are virtually no # Safety sections for unsafe functions, and the few // SAFETY: comments are informal, lacking the logical rigor required by the review guidelines.

Critical Findings

Undefined Behavior / Memory Corruption in FFI_ArrowSchema::with_metadata 🔴 ⚠️

• Priority: 🔴 High

• Threat Vector: ⚠️ Accidental Misuse

• Bug Type: Null Pointer Dereference

The public, safe builder method FFI_ArrowSchema::with_metadata is unsound and can be used to trigger immediate undefined behavior in safe Rust code.

Explanation

The method is defined as:

    pub fn with_metadata<I, S>(mut self, metadata: I) -> Result<Self, ArrowError>
    where
        I: IntoIterator<Item = (S, S)>,
        S: AsRef<str>,
    {
        ...
        unsafe {
            let mut private_data = Box::from_raw(self.private_data as *mut SchemaPrivateData);
            private_data.metadata = new_metadata;
            self.private_data = Box::into_raw(private_data) as *mut c_void;
        }

        Ok(self)
    }

If this method is called on a schema constructed via FFI_ArrowSchema::empty(), self.private_data is a null pointer. The code passes this null pointer directly to Box::from_raw, which violates the standard library safety precondition (the pointer must be non-null and allocated by the global allocator with the appropriate layout). This triggers undefined behavior (manifesting as a abort/panic in debug mode with standard library checks).

Furthermore, if the schema was imported from C via from_raw, self.private_data contains an opaque pointer provided by the foreign C producer. Calling with_metadata on such a schema will cast this opaque pointer to a SchemaPrivateData and attempt to drop it or reallocate it, causing type confusion and memory corruption.

Reproduction

    #[test]
    fn test_empty_with_metadata_ub() {
        let schema = FFI_ArrowSchema::empty();
        // This will call Box::from_raw(null) inside `with_metadata`, triggering UB.
        let _ = schema.with_metadata([("key", "value")]);
    }

Recommended Fix

Make the method validate that the schema was constructed locally and is initialized:

        if self.private_data.is_null() {
            return Err(ArrowError::CDataInterface(
                "Cannot set metadata on an empty or uninitialized schema".to_string(),
            ));
        }

        if self.release != Some(release_schema) {
            return Err(ArrowError::CDataInterface(
                "Cannot set metadata on schema with external release callback".to_string(),
            ));
        }

        unsafe {
            let mut private_data = Box::from_raw(self.private_data as *mut SchemaPrivateData);
            private_data.metadata = new_metadata;
            self.private_data = Box::into_raw(private_data) as *mut c_void;
        }

---

Fishy Findings

None.

---

Missing Safety Comments

The following unsafe blocks and functions lack safety comments or have comments that are not structured as proofs:

1. FFI_ArrowSchema::from_raw 🟡

• Priority: 🟡 Low
• Bug Type: Missing Safety Documentation

    pub unsafe fn from_raw(schema: *mut FFI_ArrowSchema) -> Self {
        unsafe { std::ptr::replace(schema, Self::empty()) }
    }

• Missing safety documentation: The function has # Safety documentation for the caller, but the unsafe block inside from_raw lacks a // SAFETY: comment explaining why std::ptr::replace is sound given those preconditions.

• Proposed safety comment:

  // SAFETY:
  // - By caller precondition, `schema` is valid for reads and writes and properly aligned.
  // - Replacing the value with `Self::empty()` is safe as both types match.
  // - By replacing the schema, we set its release callback to `None` in the source location,
  //   safely moving ownership of the resource to the returned `Self`.

2. FFI_ArrowSchema::format & name 🟡

• Priority: 🟡 Low
• Bug Type: Missing Safety Documentation

    pub fn format(&self) -> &str {
        assert!(!self.format.is_null());
        // safe because the lifetime of `self.format` equals `self`
        unsafe { CStr::from_ptr(self.format) }
            .to_str()
            .expect("The external API has a non-utf8 as format")
    }

• Missing safety comment: The comments are brief and do not explain why it is safe to dereference the raw pointer (i.e. validity, null-termination).

• Proposed safety comment for format:

  // SAFETY:
  // - `self.format` is checked to be non-null.
  // - The provider of `FFI_ArrowSchema` guarantees that `format` points to a valid,
  //   null-terminated UTF-8 string that remains valid for the lifetime of `self`.

3. FFI_ArrowSchema::child 🟡

• Priority: 🟡 Low
• Bug Type: Missing Safety Documentation

    pub fn child(&self, index: usize) -> &Self {
        assert!(index < self.n_children as usize);
        unsafe { self.children.add(index).as_ref().unwrap().as_ref().unwrap() }
    }

• Missing safety comment: No safety comment justifies the unsafe operations (pointer arithmetic and double dereferencing).

• Proposed safety comment:

  // SAFETY:
  // - By FFI contract or local construction invariant, `self.children` is a valid pointer
  //   pointing to an allocated array containing at least `self.n_children` elements.
  // - Since `index < self.n_children`, `self.children.add(index)` is within the bounds of
  //   the allocated array.
  // - The pointers contained in the array are non-null and point to valid, initialized
  //   `FFI_ArrowSchema` instances that remain valid as long as `self` is live.

4. FFI_ArrowSchema::dictionary 🟡

• Priority: 🟡 Low
• Bug Type: Missing Safety Documentation

    pub fn dictionary(&self) -> Option<&Self> {
        unsafe { self.dictionary.as_ref() }
    }

• Missing safety comment: No safety comment justifies the pointer dereference.

• Proposed safety comment:

  // SAFETY:
  // - By FFI contract, if `self.dictionary` is non-null, it points to a valid,
  //   initialized `FFI_ArrowSchema` that remains valid as long as `self` is live.

5. FFI_ArrowSchema::metadata (helper functions) 🟡

• Priority: 🟡 Low
• Bug Type: Missing Safety Documentation

            fn next_four_bytes(buffer: *const u8, pos: &mut isize) -> [u8; 4] {
                let out = unsafe {
                    [
                        *buffer.offset(*pos),
                        *buffer.offset(*pos + 1),
                        *buffer.offset(*pos + 2),
                        *buffer.offset(*pos + 3),
                    ]
                };
                *pos += 4;
                out
            }

• Missing safety comment: No safety comment justifies the pointer arithmetic and dereferences.

• Proposed safety comment:

  // SAFETY:
  // - The caller is responsible for ensuring `buffer` points to a valid Arrow metadata
  //   allocation. Under Arrow metadata format, the buffer must contain at least the
  //   number of bytes required by the parsed schema structure.
  // - This reads bytes sequentially within the bounds guaranteed by the structure.

6. Drop for FFI_ArrowSchema 🟡

• Priority: 🟡 Low
• Bug Type: Missing Safety Documentation

impl Drop for FFI_ArrowSchema {
    fn drop(&mut self) {
        match self.release {
            None => (),
            Some(release) => unsafe { release(self) },
        };
    }
}

• Missing safety comment: No safety comment justifying the invocation of the C function pointer.

• Proposed safety comment:

  // SAFETY:
  // - If `self.release` is `Some`, it points to a valid release callback function.
  // - The callback expects a mutable pointer to the schema and takes ownership of releasing
  //   its internal allocations.
  // - By dropping `self`, we will not access the schema again, so transferring ownership to
  //   the release callback is sound.

7. unsafe impl Send for FFI_ArrowSchema 🟡

• Priority: 🟡 Low
• Bug Type: Missing Safety Documentation

unsafe impl Send for FFI_ArrowSchema {}

• Missing safety comment: No comment explaining why thread safety holds.

• Proposed safety comment:

  // SAFETY:
  // - `FFI_ArrowSchema` owns all its heap-allocated buffers (via `private_data` or FFI contract).
  // - It does not expose shared mutability or reference-counted pointer structures that lack
  //   synchronization.
  // - Therefore, transferring ownership of an `FFI_ArrowSchema` to another thread is sound.