cloudflare / cloudflare/wirefilter
CompoundType's 32-layer depth cap panics instead of rejecting, reachable from JSON deserialization and the FFI type-builder
- Dominant language
- Rust
- Stars
- 1.2k
- Forks
- 122
- Avg merge
- 5h 20m
- Merged PRs (30d)
- 4
Description
`CompoundType::push` (`engine/src/types.rs:1006-1018`) enforces a hard 32-layer cap on nested
`Array`/`Map` types, returning `None` once `self.len >= 32`. Its only caller,
`CompoundType::from_type` (`types.rs:958-970`), treats that `None` as fatal:
```rust
// engine/src/types.rs
match match ty {
...
Type::Array(ty) => ty.push(Layer::Array),
...
} {
Some(ty) => ty,
None => panic!("Could not convert type to compound type"), // types.rs:968
}
```
Two producers feed this panic, neither validating depth beforehand:
1. `engine`'s own `Deserialize` impl for `Type`/`CompoundType`, reachable from the FFI's execution-context snapshot/restore JSON format (`$lists` entry's `type` field).
2. `ffi`'s `CType::push` (`ffi/src/lib.rs:61-69`) mirrors `CompoundType` but has no matching 32-layer guard of its own — it accumulates layer bits into a `u32`/`u8` pair indefinitely. The panic only fires when the accumulated `CType` is converted back via `impl From for Type` (`ffi/src/lib.rs:88-102`), reachable via ordinary, well-formed repeated calls to `wirefilter_create_array_type` during scheme/type construction.
None of `wirefilter_deserialize_json_to_execution_context`, `wirefilter_add_type_field_to_scheme`,
`wirefilter_add_always_list_to_scheme`, `wirefilter_add_never_list_to_scheme`, or
`wirefilter_serialize_type_to_json` wraps this in `catch_panic`.
**The real trigger is depth 34, not 33** — the recursive layer-accounting means the cap is hit one
level later than a naive count suggests (each layer's conversion pushes onto the *previous* level's
already-built `CompoundType`).
**Confirmed crossing the real C ABI boundary**, via a compiled C test program (built against this
repo's own `ffi/tests/ctests` harness) that constructs
`{"$lists":[{"type":<34 nested Array wrappers>,"data":{}}]}` and calls
`wirefilter_deserialize_json_to_execution_context`:
```
thread '...' panicked at engine/src/types.rs:968:21:
Could not convert type to compound type
thread caused non-unwinding panic. aborting.
... (signal: 6, SIGABRT: process abort signal)
```
This is a genuine process abort observed from the C caller's side, not an in-process Rust-only panic.
`wirefilter_enable_panic_catcher()` is never called in this test (defaults to disabled), consistent
with an ordinary embedding that hasn't explicitly enabled it.
**Reachability:** whether attacker-supplied JSON reaches the deserialization path (vs. only
host-produced serialized state) depends on the embedding. The FFI/`CType` path requires the ability to
construct a scheme with 34+ nested array/map layers — a schema/type-construction-time actor, not raw
per-request traffic. The wasm bindings (`wasm/src/lib.rs`) expose only `Scheme::try_from(fields)` —
scheme construction only, no JSON-ingestion API — so this doesn't apply to the wasm surface as
currently shipped.
Not memory corruption — a clean panic/abort (DoS) in both paths.
**Suggested fix:** add an explicit depth check (reject at deserialize time, or in `CType::push`, once
depth would exceed 32) that returns a normal error instead of relying on the internal cap to panic.
Give `CType::push` the same `if self.len >= 32 { None }` guard `CompoundType::push` already has, and
propagate that as an error rather than panicking downstream. Wrap the five consuming FFI functions in
`catch_panic` regardless, as defense in depth.
Found with the [rust-in-peace](https://github.com/scadastrangelove/rust-in-peace) pipeline.
Contributor guide
Research direction
Start in engine/src/types.rs at CompoundType::push and CompoundType::from_type, then inspect ffi/src/lib.rs at CType::push, the CType conversion, and the five named FFI entry points. Reproduce the 34-layer case with the ffi/tests/ctests harness and verify that deserialization and repeated type construction return errors without a panic or process abort.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c, rust
- Domain
- backend-api-design, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100