fix(mir): MUTABLE @list of struct grew through dangling ptr (UAF)#21
fix(mir): MUTABLE @list of struct grew through dangling ptr (UAF)#21
Conversation
|
Bencher Report| Branch | hotfix-list-append-buffer-uaf |
| Testbed | ubuntu-latest |
⚠️ WARNING: No Threshold found!Without a Threshold, no Alerts will ever be generated.
Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the--ci-only-thresholdsflag.
Click to view all benchmark results
| Benchmark | leak-build-ms | Measure (units) x 1e3 | leak-count | Measure (units) | leak-run-ms | Measure (units) |
|---|---|---|---|---|---|---|
| benchmarks/concurrent/04_fanout_fanin/bench | 📈 view plot | 4.52 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 3,633.61 units |
| benchmarks/concurrent/09_kvstore/bench | 📈 view plot | 4.50 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 60,003.32 units |
| benchmarks/concurrent/14_nested_lock/bench | 📈 view plot | 4.42 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 365.74 units |
| benchmarks/inter-clear/02_concurrent_fsm_vs_stackful/bench_fsm | 📈 view plot | 4.40 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 135.01 units |
| benchmarks/inter-clear/02_concurrent_fsm_vs_stackful/bench_stackful | 📈 view plot | 4.40 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 367.36 units |
| benchmarks/sequential/11_pipeline_overhead/bench | 📈 view plot | 4.39 units x 1e3 | 📈 view plot | 0.00 units | 📈 view plot | 11,576.28 units |
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #21 +/- ##
==========================================
- Coverage 89.87% 89.87% -0.01%
==========================================
Files 183 183
Lines 47636 47679 +43
Branches 11598 11620 +22
==========================================
+ Hits 42812 42850 +38
- Misses 4824 4829 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Original hotfix (commit
|
A helper function declaring `MUTABLE xs: T[]@list` and calling
`.append(StructLit{...})` would corrupt the caller's list buffer or
segfault on a subsequent `.append`. The pattern:
FN record!(MUTABLE items: TraceItem[]@list, step: Int64) RETURNS !Void ->
items.append(TraceItem{ step: step, ... });
RETURN;
END
FN main!() RETURNS !Void ->
MUTABLE items: TraceItem[]@list = List[];
record!(items, 1_i64); # buffer allocated in record!'s frame
junk = burnFrame!(...); # bump arena advances past freed bytes
record!(items, 2_i64); # writes through dangling items.ptr -> SEGFAULT
...
END
The bug surfaced after the recent `MUTABLE @list` pointer-pass commits
(1872ae4 / fcedfbe) made this calling pattern compile; the
`:receiver_storage` allocator-routing in std_lib.rb / mir_lowering.rb
predates pointer-passing and assumes a single-frame world.
Two coordinated fixes:
* **escape_analysis.rb (Condition 9)** -- when a `@list` is passed
to a `MUTABLE @list` parameter, escape-promote the caller's
binding to `:heap` at decl. Mirrors the existing GIVE-to-TAKES
promotion (Condition 8). Without this, the caller's cleanup uses
`frameAlloc` while the callee's `.append` heap-allocates, leaking
the buffer on deinit.
* **mir_lowering.rb (resolve_alloc_sym)** -- when a method call's
receiver is a `@list` parameter that arrived via pointer-pass
(in `@current_fn_collection_params`), force `:heap` for the
`:receiver_storage` resolution. The receiver's static storage tag
on the *param* doesn't reflect the heap promotion that happened
on the caller's binding (storage doesn't propagate across the
function boundary), so the callee-side resolver has to make the
call locally based on "did I receive this by pointer?".
Both edges are necessary: escape-promote alone leaves the helper's
`.append` using the helper's `frameAlloc()` (UAF persists); the
lowering-side guard alone heap-allocates the buffer but the caller's
`frameAlloc` cleanup leaks it.
Test 380 covers the deterministic repro: cross-frame `.append` of a
struct, with `burnFrame!` calls between record!s to force the
segfault on origin/master.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…F class The previous hotfix (parent of this branch) plugged a UAF discovered in production: a `MUTABLE T[]@list` parameter let a callee `.append` into a buffer whose growth allocator was the *callee's* frame, which got rewound on return — leaving the caller with a dangling pointer. That hotfix added Condition 9 to escape_analysis.rb (auto-promote the caller's binding to :heap) and a guard in mir_lowering.rb's `resolve_alloc_sym` (force :heap when the receiver is a pointer- passed param). That hotfix unblocked the immediate crash — but it did so by adding two coordinated lowering-side decisions, with no corresponding MIR Checker invariant. The two introducing commits (1872ae4 / fcedfbe) likewise landed the pointer-pass functionality without a matching checker rule. Both commits were rushed out to close short-term gaps (callees couldn't grow caller @lists, then forwarded @list params were re-`&`-wrapped); neither came with a structural fence in the safety system. The result: a memory-safety promise was silently weakened, and the only thing standing between users and the UAF was that two specific lines in mir_lowering.rb routed `:heap` correctly. A future MIR refactor or a different pointer-pass path (closures, channel sends, async captures) would have re-introduced the same UAF without the checker noticing. This branch adds the structural fence the previous fixes should have included from the start. **INV-CROSS-FRAME-PARAM-ALLOC** in `src/mir/mir_checker.rb`: An InlineZig op whose target_var is a pointer-passed parameter must NOT use the `:frame` allocator. Pointer-passed params carry a lifetime that extends past the current function's mark/restore; a frame allocation here would die when the function returns, leaving the caller with a dangling buffer pointer. The checker independently re-derives "is this param pointer-passed?" from a new `MIR::Param.pointer_passed` field (lowering tags it). We can't infer it from the Zig type string alone — collection params lower to `anytype` for polymorphism — so the lowering tags it explicitly and the checker reads the flag. Defense in depth: if `resolve_alloc_sym` or escape_analysis Condition 9 ever regresses, the checker catches the resulting bad MIR before codegen. Lowering's `mutable_scalar_params` was incorrectly classifying MUTABLE collection params as scalars (because `transpile_type` returns `"anytype"` for collections, which doesn't match the [] / * prefix check). The mis-classification renamed `items` to `_m_items` in MIR::Param.name, while `MIR::InlineZig.target_var` kept the original `items`. The names didn't match, and the new invariant couldn't find the param. Excluded `collection?` and `needs_pointer_passing?` types from the scalar classifier; the rename is meant for true scalars only. Defense-in-depth verified end-to-end: 1. Disabled the lowering-side fix (commented out the two `@current_fn_collection_params&.include?(...)` lines in `resolve_alloc_sym`). Re-built test 380. The new MIR Checker invariant rejected the build with: [CROSS_FRAME_PARAM_ALLOC] record::items -- operation alloc is :frame but 'items' is a pointer-passed parameter (lifetime extends past this function's frame mark; buffer would dangle on return). Use :heap. Tests 374, 378, and 380 all failed to transpile (correct behavior — the lowering's choice is bad MIR). 2. With the lowering's fix restored, all three tests pass, the full transpile-tests corpus (521 tests) passes, and the unit suite (4136 specs) passes. This commit is Phase 2 of the plan I drafted (the highest-leverage piece, the structural fence). Phase 1 (annotator-layer rejection with explicit user opt-in via `@heap` / `COPY` / non-MUTABLE) and Phase 3 (audit of other escape paths — BG capture by pointer, channel sends, etc.) remain. Phase 4 (ASAN job in CI) is a separate PR. 6 new specs in `spec/mir_checker_spec.rb` cover the invariant: the positive case (frame on pointer-passed param fires), the negative case (heap on pointer-passed param passes), local bindings (no false positives), every allocator key (key_alloc / val_alloc / shard_alloc all checked), empty-params functions, and slice (non-pointer-passed) params. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cuzzo
force-pushed
the
hotfix-list-append-buffer-uaf
branch
from
62223f1 to
d5db63d
Compare
2 participants
A helper function declaring
MUTABLE xs: T[]@listand calling.append(StructLit{...})would corrupt the caller's list buffer or segfault on a subsequent.append. The pattern:FN record!(MUTABLE items: TraceItem[]@list, step: Int64) RETURNS !Void ->
items.append(TraceItem{ step: step, ... });
RETURN;
END
FN main!() RETURNS !Void ->
MUTABLE items: TraceItem[]@list = List[];
record!(items, 1_i64); # buffer allocated in record!'s frame
junk = burnFrame!(...); # bump arena advances past freed bytes
record!(items, 2_i64); # writes through dangling items.ptr -> SEGFAULT
...
END
The bug surfaced after the recent
MUTABLE @listpointer-pass commits (1872ae4 / fcedfbe) made this calling pattern compile; the:receiver_storageallocator-routing in std_lib.rb / mir_lowering.rb predates pointer-passing and assumes a single-frame world.Two coordinated fixes:
escape_analysis.rb (Condition 9) -- when a
@listis passed to aMUTABLE @listparameter, escape-promote the caller's binding to:heapat decl. Mirrors the existing GIVE-to-TAKES promotion (Condition 8). Without this, the caller's cleanup usesframeAllocwhile the callee's.appendheap-allocates, leaking the buffer on deinit.mir_lowering.rb (resolve_alloc_sym) -- when a method call's receiver is a
@listparameter that arrived via pointer-pass (in@current_fn_collection_params), force:heapfor the:receiver_storageresolution. The receiver's static storage tag on the param doesn't reflect the heap promotion that happened on the caller's binding (storage doesn't propagate across the function boundary), so the callee-side resolver has to make the call locally based on "did I receive this by pointer?".Both edges are necessary: escape-promote alone leaves the helper's
.appendusing the helper'sframeAlloc()(UAF persists); the lowering-side guard alone heap-allocates the buffer but the caller'sframeAlloccleanup leaks it.Test 380 covers the deterministic repro: cross-frame
.appendof a struct, withburnFrame!calls between record!s to force the segfault on origin/master.