cloudflare / cloudflare/wirefilter

CompoundType's 32-layer depth cap panics instead of rejecting, reachable from JSON deserialization and the FFI type-builder

Open
#195 1 comment 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.