sandbox: charge the zlib codecs against max_bytes (#292 follow-up)

[InauguralPhysicist]

inflate/deflate (and their zlib_* duals) are on the sandbox_run
allowlist but charged nothing against max_bytes, so an allowlisted call
allocated straight past the cap that exists to protect the host from
untrusted code.

Measured, identical 8 MiB budget:

zeros of 1e7 (charged) ok=0, peak RSS 13 MiB
inflate 9.7KB (uncharged) ok=0, peak RSS 853 MiB

The charged path refuses before allocating; the uncharged one consumed
~840 MiB and only then returned {ok:0} — the refusal arrived after the
damage, so it provided no protection.

Amplification is why the existing rationale did not cover these. Every
other allocator makes the caller NAME a size, which is what the charge
reads; a compressed blob names nothing and amplifies ~1000x, so
"bounded by loop-iteration cap x per-op size" — true for the remaining
uncharged allocators — does not hold. Both the codec's own output buffer
(charged as it grows) and the list of Values built from it are now
charged; the list dominates at ~80 bytes per decompressed byte.

Also: sandbox_run no longer blames a callable for a result it merely
could not scan. The boundary scan is node/depth budgeted and fails
closed, which is right, but every exhaustion was reported as "sandbox
result contains a callable" — so a large list of plain numbers was
attributed to a function that was never in the result, sending the
caller after a phantom. Budget exhaustion now says it could not scan;
the literal claim is reserved for an actual sighting. Both still refuse.

Behavior outside a budgeted sandbox_run is unchanged (sandbox_charge is
a no-op there); codec round-trips are untouched.

Verified: release suite 3752/3752; zlib suite 3775/3775 (all 24
inflate/deflate checks, incl. new D23/D24); ASan+UBSan suite with
detect_leaks=1 3756/3756 with the leak tally still 0; an ad-hoc
ASan+zlib build runs test_inflate.eigs and the bomb repro leak-clean,
covering the new charge-failure free paths that make asan compiles
out; tools/doc_drift_check.sh clean.

Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01PwqKj3L2sLprenBfJgxRHL

[Copilot]

Pull request overview

This PR tightens sandbox_run’s memory safety guarantees by charging the zlib codecs (inflate/deflate and zlib_* variants) against max_bytes, and improves sandbox boundary-scan error reporting so budget exhaustion is not misreported as “contains a callable”.

Changes:

• Charge zlib codec internal buffers and the list-of-Values result against the sandbox byte budget.
• Distinguish “result too large to scan” from “result contains a callable” in sandbox_run error reporting.
• Add/extend tests and update docs/changelog and test-runner accounting.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/test_sandbox_allow.eigs	Adds regression coverage for “too large to scan” vs actual callable detection.
tests/test_inflate.eigs	Adds sandboxed inflate budget regression test and confirms non-sandbox behavior unchanged.
tests/run_all_tests.sh	Updates DEFLATE section accounting from 22 → 24 checks.
src/vm.c	Updates sandbox budget commentary to include zlib codecs.
src/builtins.c	Implements callable-scan message split; adds sandbox charging for zlib buffers and output list.
docs/BUILTINS.md	Documents codec charging and new boundary-scan message distinction.
CHANGELOG.md	Records the sandbox budget fix and improved boundary-scan messaging.

Suppressed comments (1)

src/builtins.c:4850

• cap is computed as src_n * 3 + 64 and then used for sandbox charging and allocation sizing. That arithmetic can overflow size_t (especially on 32-bit), which would wrap cap to a small value and break both budgeting and buffer growth logic. Compute cap with safe_size_mul() and handle overflow explicitly before clamping to EIGS_INFLATE_MAX_OUT.

    size_t cap = src_n * 3 + 64;
    if (cap > EIGS_INFLATE_MAX_OUT) cap = EIGS_INFLATE_MAX_OUT;
    /* #292: charge the codec's own buffer as it grows, so a bomb is refused
     * before the memory is touched rather than after. EIGS_INFLATE_MAX_OUT
     * bounds this at 256 MiB, which is the *default* whole-run budget — a

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[InauguralPhysicist]

Reviewed — the fix is correct and the diagnostic split is a genuine improvement. I verified the charge does what it claims: same D23 shape, peak RSS 6.5 MB charged vs 178 MB uncharged (/usr/bin/time -v), with the charged run refused during inflate ("budget exceeded") and the uncharged run only after building the whole result ("too large to scan"). The fix lands.

One blocker before merge: D23 is vacuous — it stays green with the fix reverted. sb_small["ok"] == 0 doesn't discriminate, because the 2 M-element result also trips the callable-scan node budget (SANDBOX_RESULT_MAX_NODES = 100000), a refusal that predates this PR. I flipped all three sandbox_charge calls off, rebuilt make zlib, and D23's shape still returns ok=0 — via the "too large to scan" path, after the 178 MB was already allocated. So a future refactor dropping the charge keeps D23 green while re-opening exactly this hole. (This is the one-sided-verifiability trap: the assertion is satisfied by a refusal the change didn't introduce.)

Two ways to put the fault in the binary, ideally both:

1. Assert the message, not just ok. With the charge on, the budget refusal fires first (during inflate, before the scan), so sb_small["error"]["message"] contains budget exceeded; reverted, the message is too large to scan. That single contains check flips red without the fix.

2. Add a scannable-but-over-budget case, which is the cleaner planted fault — a result small enough to scan so the only thing that can refuse it is the charge:

# 50000 elems = 50001 nodes < scan budget; 50000*80 = 4MB > 1MB budget.
sb_iso is buffer of 50000
iso_blob is deflate of sb_iso
iso_desc is [SB_ABI, [SB_GET_NAME,0,0, SB_CONST,1,0, SB_CALL,1,0, SB_RETURN], ["inflate", iso_blob]]
iso is sandbox_run of [iso_desc, 1000000, 1048576]
check of ["D25 charge (not scan) refuses a scannable over-budget result", iso["ok"], 0]

I measured this one directly: ok=0 charged, ok=1 reverted — the result crosses the boundary intact when the charge is gone. That's the assertion that actually guards #292.

Everything else (the codec-buffer delta charge, the deflate symmetry, the scan-message split and its test_sandbox_allow coverage) looks right to me.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/builtins.c:4819

• The zlib codecs now charge their output buffers/results, but the input-shape conversion in zlib_bytes_arg still allocates a fresh src byte array via xmalloc without charging it. In a budgeted sandbox_run this can still exceed max_bytes by ~n bytes (e.g., a large buffer/list argument duplicates into src unaccounted), undermining the intent of closing all codec allocation gaps.

    /* #292: the result is `n` fresh number Values at sizeof(Value)+sizeof(Value*)
     * each — ~80 bytes per decompressed BYTE. Charging only the codec's own
     * output buffer would therefore miss 98% of the cost, so charge the list
     * here too, with the same accounting range/zeros use. Without this an
     * allowlisted `inflate` allocates straight past max_bytes: the budget

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/builtins.c:4852

• zlib_bytes_result computes the sandbox charge as (size_t)n * (...) and then casts n to int for make_list((int)n). If n exceeds INT_MAX or the multiplication overflows size_t, the sandbox accounting can wrap (undercount) and the list length truncation can lead to incorrect allocation/behavior. Add explicit bounds/overflow checks and use a single validated size_t/int length for both charging and list creation.

     * blob names nothing. */
    if (!sandbox_charge((size_t)n * (sizeof(Value) + sizeof(Value *))))
        return make_null();
    Value *result = make_list((int)n);

examples/ui_canvas_events.eigs:205

• This PR is described as a sandbox/zlib accounting + sandbox_run error-message change, but it also changes the UI example by removing a ui._layout call. Even if redundant (since ui.app_loop already performs a layout pass), it’s an unrelated behavior change that should either be explained in the PR description or moved to a separate PR/commit to keep the security fix narrowly scoped.

define on_tick(r) as:
    sb.text is status

ui.app_loop of [root, on_key, on_tick]