Skip to content

Five FFI value-ingestion setters have missing/inconsistent null-pointer validation — two produce a real SIGSEGV, not a panic #194

Description

@scadastrangelove

Five extern "C" functions in ffi/src/lib.rs ingest a per-request field value into an
ExecutionContext from a raw pointer (or, for the two IP setters, a Rust reference) supplied by the
embedding host. Their null-pointer validation is inconsistent, and for two of the five, absent:

Function Null check? Consequence of NULL
wirefilter_add_json_value_to_execution_context none std::slice::from_raw_parts(json_ptr, json_len) (line 549) reads from address 0
wirefilter_add_ipv4_value_to_execution_context none — takes value: &[u8; 4] dereferenced unconditionally via IpAddr::from(*value) (line 620)
wirefilter_add_ipv6_value_to_execution_context none — takes value: &[u8; 16], same shape not independently reproduced, expected identical by code-shape
wirefilter_deserialize_json_to_execution_context assert!(!json_ptr.is_null()) (line 523), unconditional regardless of json_len any (NULL, 0) call panics; no catch_panic wrapper
wirefilter_add_bytes_value_to_execution_context assert!(!value_ptr.is_null()) (line 593), same shape same as above

Within four of these five (all but deserialize_json, which takes no separate name argument), the
name_ptr argument is validated via the to_str! macro's assert!(!$ptr.is_null()) — so the
inconsistency is present inside a single function, not just across the file.

Impact — two of the five are a real crash, not a controlled panic. Confirmed via a compiled C
caller linked against the built cdylib (not a Rust-constructed value):

calling wirefilter_add_ipv4_value_to_execution_context(ctx, "ip.src", NULL) ...
(process exits with signal 11, SIGSEGV: invalid memory reference)

Confirmed in both debug and release. A Rust-level PoC (constructing the null value in Rust rather
than via a real C caller) shows the same result for the JSON setter:

$ cargo run --release --example poc_f018_add_json_value_null_oob
(json_ptr=NULL, json_len=16)
exit: 139   (= 128 + SIGSEGV)

The other two setters (deserialize_json, add_bytes_value) produce a controlled SIGABRT on
(NULL, 0) in both profiles — a process-killing DoS, not memory-unsafety.

Separately: wirefilter_enable_panic_catcher() is never called automatically anywhere in this crate
(PANIC_CATCHER_ENABLED defaults to false), so even those two assert!-guarded setters' panics
reach an uncaught process abort in any embedding that never explicitly enables it.

Reachability: requires the embedding host to pass a null pointer/reference through — plausibly by
representing an empty or absent field (no cookie, an empty header) as (NULL, 0), a common enough C
convention that it's an easy embedding mistake. Not demonstrated as network-reachable end-to-end;
whether a specific embedding's request-handling path can be driven into passing a null value depends
on that embedding layer.

Suggested fix: add the same assert!(!ptr.is_null()) (or a Result-returning check) that
to_str! and the two partially-guarded setters already use, to the JSON setter and both IP setters —
and treat length == 0 as "empty" (skip the pointer check) for the (ptr, len)-style setters rather
than an error. Wrap all five in the existing catch_panic helper, matching
wirefilter_parse_filter/wirefilter_match.

Found with the rust-in-peace pipeline.

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