Follow-up to #53, which closed the entry-point hole. Filed to record the evidence rather than because it needs doing soon — see "Why this is parked" at the bottom.
What #53 did and didn't do
#53 added a format header and bounds-checked every count in the stream. That covers the case that actually mattered: --perform on a file that isn't an .acx at all, and the browser driver's upload button (drivers/web/play.js:601) handing the wasm deserializer whatever the player picked off their disk. Those are now refused at the door.
What it does not cover is a file that gets past the door — a valid header and valid counts, with damage further in.
The measurement
400 mutants of a valid bare.acx, 1–6 random byte flips each, a quarter of them also truncated:
|
signalled |
rejected cleanly |
loaded and ran |
| header only |
233 |
102 |
65 |
| header + bounds (merged) |
171 |
166 |
63 |
The bounds checks converted 62 crashes into clean rejections. 171 remain.
Two classes
Assertion failures (SIGABRT). Reconstruction hits an invariant the file violated:
Assertion failed: (not is_binary(op)), function UnaryOperator, file Expression.cc, line 252
These are the better outcome, and only in a debug build. Under NDEBUG the assert vanishes and the same bytes become silent undefined behaviour.
Segfaults (SIGSEGV), no output at all. Null dereference or runaway recursion while rebuilding expressions, statements, or objects.
The shape of it
Counts were one family of unchecked number; opcodes and type tags are another, read and cast without ever being asked whether they name anything:
src/Statement.cc:767 — static_cast<StatementType_e>(stmt_type_as_int)src/Statement.cc:400 — static_cast<Keywords::Reserved_e>(write_type_as_int)src/Expression.cc — operator codes, reaching the is_binary assert at :252- object ids and attribute ids throughout, none range-checked against the registry they index
The fix in each case is the same shape as readCount: validate before believing. It is not hard, only broad — every tag needs a "is this a value of the enum" gate and every id a "does this exist" gate.
Why this is parked
The threat model here is a corrupted .acx, not a foreign file, and the severity is low in both places it could land:
- Browser. A wasm module's out-of-bounds write stays inside its own linear memory. Worst case is a crashed tab or a scrambled game, not an escape.
- Native CLI. Someone would have to be handed a hostile
.acx and choose to --perform it — a local binary they elected to run.
Nobody has ever reported a corrupted save. Deferred deliberately: the door is locked, and this is about what happens to someone who is already inside with a damaged file.
If it is ever picked up, the fuzz harness is trivial to reconstruct from the table above, and the crash rate is the metric to move.
Follow-up to #53, which closed the entry-point hole. Filed to record the evidence rather than because it needs doing soon — see "Why this is parked" at the bottom.
What #53 did and didn't do
#53 added a format header and bounds-checked every count in the stream. That covers the case that actually mattered:
--performon a file that isn't an.acxat all, and the browser driver's upload button (drivers/web/play.js:601) handing the wasm deserializer whatever the player picked off their disk. Those are now refused at the door.What it does not cover is a file that gets past the door — a valid header and valid counts, with damage further in.
The measurement
400 mutants of a valid
bare.acx, 1–6 random byte flips each, a quarter of them also truncated:The bounds checks converted 62 crashes into clean rejections. 171 remain.
Two classes
Assertion failures (SIGABRT). Reconstruction hits an invariant the file violated:
These are the better outcome, and only in a debug build. Under
NDEBUGthe assert vanishes and the same bytes become silent undefined behaviour.Segfaults (SIGSEGV), no output at all. Null dereference or runaway recursion while rebuilding expressions, statements, or objects.
The shape of it
Counts were one family of unchecked number; opcodes and type tags are another, read and cast without ever being asked whether they name anything:
src/Statement.cc:767—static_cast<StatementType_e>(stmt_type_as_int)src/Statement.cc:400—static_cast<Keywords::Reserved_e>(write_type_as_int)src/Expression.cc— operator codes, reaching theis_binaryassert at :252The fix in each case is the same shape as
readCount: validate before believing. It is not hard, only broad — every tag needs a "is this a value of the enum" gate and every id a "does this exist" gate.Why this is parked
The threat model here is a corrupted
.acx, not a foreign file, and the severity is low in both places it could land:.acxand choose to--performit — a local binary they elected to run.Nobody has ever reported a corrupted save. Deferred deliberately: the door is locked, and this is about what happens to someone who is already inside with a damaged file.
If it is ever picked up, the fuzz harness is trivial to reconstruct from the table above, and the crash rate is the metric to move.