Skip to content

Commit affdbbc

Browse files
fix(cashc): close 2026-06 red-team compiler findings
- final-require: reject no-else `if` whose only branch ends in require (previously compiled to an UNCONDITIONAL spend) — new EnsureFinalRequireTraversal check + final_branch_no_else fixture - bool equality: correct miscompiled bool `==`/`!=` lowering - int literal precision: preserve large int literals in codegen - nulldata size: enforce OP_RETURN data-size bound - split bound: type-check split index bounds Adds compiler test fixtures + generation tests; updates examples, SECURITY_AUDIT_REPORT.md, and CHANGELOG. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 7d11a8a commit affdbbc

37 files changed

Lines changed: 1737 additions & 310 deletions

CHANGELOG.md

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,43 @@ to the corresponding finding — lives in [`SECURITY_AUDIT_REPORT.md`](./SECURIT
1414

1515
## [Unreleased]
1616

17-
Nothing yet.
17+
### 2026-06-04 red-team remediation (see [`SECURITY_AUDIT_REPORT.md` §11](./SECURITY_AUDIT_REPORT.md))
18+
19+
A fresh adversarial pass found 16 issues beyond the §1–§10 "all closed" set — including a
20+
**CRITICAL** compiler miscompile — all now fixed and regression-tested
21+
(534 tests passing, lint clean, all `dist/` rebuilt).
22+
23+
#### `@radiantscript/rxdc` (compiler) — Fixed
24+
- **[CRITICAL]** A function whose final statement was an `if` with **no `else`** compiled to an
25+
unconditional spend (the false branch fell through to an appended `OP_1`, skipping every
26+
`require`). A terminal branch now requires both an `else` and a closing `require` in each block. (§11.1/C-1)
27+
- **[HIGH]** `bool ==` / `!=` emitted bytewise `OP_EQUAL`; now `OP_NUMEQUAL`/`OP_NUMNOTEQUAL`,
28+
so non-canonical bool encodings can't bypass equality gates. (§11.1/H-1)
29+
- **[MEDIUM]** Integer literals were parsed with `parseInt` (lossy double); now parsed as `BigInt`
30+
with an 8-byte script-number range check (`IntLiteralOverflowError`). (§11.1/M-1)
31+
- **[LOW]** Tuple-assignment no longer lets a split half be re-declared at a mismatched fixed
32+
`bytesN` bound; oversized `LockingBytecodeNullData` literal chunks (>255 bytes) now error. (§11.1/L-1, L-4)
33+
34+
#### `radiantscript` (SDK) — Fixed
35+
- **[HIGH]** Full prevout verification before signing: `Transaction.build()` now fetches each
36+
input's source transaction, authenticates it (`hash256 == txid`), and asserts the prevout's
37+
value and locking script match what is being signed — so a malicious/buggy provider cannot make
38+
you sign over a wrong amount or a UTXO you don't control. Values are range-checked to Radiant's
39+
consensus `[0, MAX_MONEY]` (new `MAX_MONEY` constant). Default-on; opt out with
40+
`.withoutPrevoutVerification()` for offline signing. (§11.2/H-2, §11.5)
41+
- **[HIGH]** `Transaction.getTxDetails` verifies returned tx hex hashes to the requested txid. (§11.2/H-3)
42+
- **[HIGH]** Change-output fee now scales with `feePerByte` (was a flat 32 → underpaid on Radiant's
43+
relay floor). (§11.2/H-4)
44+
- **[MEDIUM]** `withHardcodedFee(0)` is honored as a true zero fee; `BitcoinRpcNetworkProvider`
45+
rounds/validates satoshis; all providers run returned UTXOs through `validateUtxo`;
46+
`SIGHASH_SINGLE` signers with no corresponding output are rejected. (§11.2/M-2…M-5)
47+
- **[LOW]** Mixed covenant hash types are rejected; `getBalance()` sums in bigint. (§11.2/L-2, L-3)
48+
49+
#### Examples — Changed
50+
- `examples/radiant/*` hardened (StatefulCounter, NFT, TokenSwap, FungibleToken, MultiSigVault now
51+
compile and add the missing covenant constraints) and `README.md` relabelled as audited teaching
52+
templates rather than production-safe; false "no counterparty risk" / "automatic supply
53+
conservation" claims removed. (§11.3/M-6)
1854

1955
---
2056

SECURITY_AUDIT_REPORT.md

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,3 +329,63 @@ Everything else from §1–§9 is now either FIXED, intentionally documented as
329329
---
330330

331331
*End of 2026-05-28 final remediation pass.*
332+
333+
---
334+
335+
## 11. 2026-06-04 Red-Team Pass
336+
337+
A fresh adversarial pass was run *because* §1–§10 declared everything closed — the goal was to find the bugs a "done" audit misses. It did. The headline is a **live CRITICAL compiler bug that silently produces always-spendable contracts** (§11.1/C-1) — the exact failure class as the OP_2MUL/OP_2DIV miscompile, and proof that "all findings FIXED" was premature. Sixteen findings were confirmed (3 of them proven by compiling exploit contracts through the built `rxdc 1.1.1-v2`) and **all sixteen are now fixed and regression-tested**.
338+
339+
**End state: 449 root (cashc + utils) + 96 cashscript unit = 545 tests passing, 0 failing; `npm run lint` clean; all three package `dist/` rebuilt; all five `examples/radiant/*.rxd` compile.** Baselines at start of pass were 440 root + 68 cashscript. (H-2 was subsequently upgraded from a range-check to full prevout verification — §11.5 — adding the final 11 cashscript tests.)
340+
341+
### 11.1 Compiler (`cashc` / `utils`)
342+
343+
- **[CRITICAL] C-1 — terminal `if` with no `else` compiled to an unconditional spend.** [`EnsureFinalRequireTraversal.ts:50`](packages/cashc/src/semantic/EnsureFinalRequireTraversal.ts:50). A final `BranchNode` was checked via `finalStatement.elseBlock?.statements`; with no else that is `undefined` → defaults to `[]` → `statements[-1]` is `undefined` → the pass returned silently without requiring the else path. `removeFinalVerify` then appended `OP_1` after `OP_ENDIF`, so a spend taking the false branch succeeded with **no signature/condition check**. *Proven:* `if (mode == 1) { require(checkSig(s, owner)); }` (no else) compiled to `… OP_IF … OP_CHECKSIGVERIFY OP_ENDIF OP_DROP OP_1` — spendable by anyone with `mode != 1`. **Fix:** a terminal branch must now have an `else`, and both blocks must themselves terminate in a `require` (else → `FinalRequireStatementError`). Negative fixture `final_branch_no_else.cash` + valid `final_branch_else.rxd` added. (The shipped `announcement.cash` example actually exhibited the bug — fixed with an explicit `else`.)
344+
- **[HIGH] H-1 — `bool ==` / `!=` emitted bytewise `OP_EQUAL` instead of `OP_NUMEQUAL`.** [`GenerateTargetTraversal.ts` `visitBinaryOp`](packages/cashc/src/generation/GenerateTargetTraversal.ts:463) + [`types.ts:153`](packages/utils/src/types.ts:153). `resultingType(BOOL,BOOL)` is `BOOL`, so `isNumeric` was false and bool equality compiled bytewise — two logically-true bools with different encodings (`0x01` vs `0x02`) compared unequal, letting an attacker-supplied bool param bypass an equality gate. **Fix:** treat `BOOL` as numeric for `==`/`!=` (→ `OP_NUMEQUAL`/`OP_NUMNOTEQUAL`); confirmed `TypeCheck` forbids arithmetic on bool so no other operator is affected. Generation test added.
345+
- **[MEDIUM] M-1 — integer literals parsed with `parseInt` (lossy double).** [`AstBuilder.ts` `createIntLiteral`](packages/cashc/src/ast/AstBuilder.ts). `9007199254740993` (2⁵³+1) silently became 2⁵³; oversized literals exceeded the 8-byte script-number bound with no diagnostic. **Fix:** parse value + unit multiplier as `BigInt`, range-check `±(2⁶³−1)`, throw new `IntLiteralOverflowError` on overflow; `IntLiteralNode.value` widened to `bigint`. *Proven:* 2⁵³+1 now encodes exactly `01000000000020`; 2⁶³ errors.
346+
- **[LOW] L-1 — tuple-assign bytes-bound escape.** [`TypeCheckTraversal.ts` `visitTupleAssignment`](packages/cashc/src/semantic/TypeCheckTraversal.ts:66). `bytes16 a, bytes32 b = data.split(16)` was accepted with no width enforcement. **Fix:** when the source is bounded bytes and the split index is a constant, the exact half-widths are computed and a disagreeing declared bound is rejected (`AssignTypeError`); unbounded sources / non-constant indices stay permissive so existing split contracts still compile.
347+
- **[LOW] L-4 — `LockingBytecodeNullData` size prefix only handled ≤255 bytes.** [`GenerateTargetTraversal.ts` NULLDATA](packages/cashc/src/generation/GenerateTargetTraversal.ts:401). **Fix:** literal chunks >255 bytes now throw `NullDataSizeError` at compile time; the dynamic-size branch's ≤255 limitation is documented in-code.
348+
349+
### 11.2 SDK (`cashscript`)
350+
351+
- **[HIGH] H-2 — input satoshis from the network provider were signed into the sighash unvalidated.** [`Transaction.ts`](packages/cashscript/src/Transaction.ts)[`utils.ts`](packages/cashscript/src/utils.ts). A lying server could make the SDK commit to a wrong input amount (invalid-sig griefing, or skewed covenant payout math in RadiantMM/RadiantSwap). **Fix:** every input's `satoshis` runs through `validateAmount`, **and** `build()` now performs full prevout verification before signing — see §11.5.
352+
- **[HIGH] H-3 — `getTxDetails` trusted server tx hex without checking it hashes to the requested txid.** [`Transaction.ts` `getTxDetails`](packages/cashscript/src/Transaction.ts). Enabled "confirmed but isn't" forged state. **Fix:** `binToHex(hash256(bytes).reverse())` is compared to the requested `txid`; mismatch throws (and now propagates instead of being swallowed by the retry loop).
353+
- **[HIGH] H-4 — change-output fee ignored `feePerByte`.** [`Transaction.ts`](packages/cashscript/src/Transaction.ts). The change output's cost was a flat `P2SH_OUTPUT_SIZE` (32) with no rate factor → systematic underpayment given Radiant's high relay floor. **Fix:** `change -= ceil(P2SH_OUTPUT_SIZE * feePerByte)`. Test asserts the change cost scales with the rate (mutation-verified).
354+
- **[MEDIUM] M-2 — `withHardcodedFee(0)` was silently overridden.** [`Transaction.ts`](packages/cashscript/src/Transaction.ts). Line 401 honored `0` via `??`, but three sibling branches gated on `!this.hardcodedFee`. **Fix:** single `useHardcodedFee = hardcodedFee !== undefined` used everywhere; `0` now yields a true zero fee.
355+
- **[MEDIUM] M-3 — `BitcoinRpcNetworkProvider` float-multiplied coin amounts.** [`BitcoinRpcNetworkProvider.ts`](packages/cashscript/src/network/BitcoinRpcNetworkProvider.ts). `amount * 1e8` produced non-integers that threw in `BigInt()` for honest nodes. **Fix:** `Math.round` + finite/non-negative guard, then routed through the new `validateUtxo`.
356+
- **[MEDIUM] M-4 — providers didn't validate returned UTXOs.** All four providers. **Fix:** shared `validateUtxo()` in [`utils.ts`](packages/cashscript/src/utils.ts) (txid `^[0-9a-f]{64}$`, integer `vout ≥ 0`, integer `satoshis ∈ [0, MAX_SAFE_SATOSHIS]`); `BitboxNetworkProvider` previously returned UTXOs unmapped — now mapped + validated.
357+
- **[MEDIUM] M-5 — `SIGHASH_SINGLE` input with index ≥ output count signed the zeroed output hash.** [`Transaction.ts`](packages/cashscript/src/Transaction.ts). **Fix:** `assertSingleHasOutput` throws when a `SIGHASH_SINGLE` signer has no corresponding output, on both signing paths.
358+
- **[LOW] L-2 — mixed covenant hash types.** [`Transaction.ts`](packages/cashscript/src/Transaction.ts). The on-stack preimage used only the first signer's hash type. **Fix:** differing hash types among covenant signature args now throw.
359+
- **[LOW] L-3 — `getBalance()` summed in `number`.** [`Contract.ts`](packages/cashscript/src/Contract.ts). **Fix:** sums in `bigint`; throws past `Number.MAX_SAFE_INTEGER` rather than silently rounding (public `number` return kept).
360+
361+
### 11.3 Example library (`examples/radiant/`)
362+
363+
- **[MEDIUM] M-6 — templates were marketed as a "Standard Library" but were under-constrained** (and four of five did not even compile under the 1.x grammar). **Fix (both hardened *and* relabelled):**
364+
- *StatefulCounter* — now authenticates `currentCount` against the input's own state, pins a single same-code continuation, conserves value, and binds the next state to `newCount` (limitation: fixed 4-byte count push, documented).
365+
- *NFT* — pins the carrier output to the same code script so the singleton can't escape after one hop; `transferWithData` binds `stateScript` on the ref-carrying output.
366+
- *TokenSwap* — adds `refValueSum($wantTokenRef) == tx.outputs[0].value` so the want-ref is provably on the maker's pinned output; `cancel()` now conserves the offer ref back to the maker.
367+
- *FungibleToken*`burn()` routes the burned value to a provable `OP_RETURN`; the on-chain `stateScript.split(1)` parse asserts length + leading push byte (blocks the truncation-spoofed owner).
368+
- *MultiSigVault*`spendWithMinOutput` pins `outputs[0].lockingBytecode` to a recipient and requires change back to the vault script.
369+
- `README.md` — dropped the false "No counterparty risk" / "Automatic supply conservation" claims, added a **TEACHING TEMPLATES — independently audit before production** banner, and documented the "token amount == satoshi value" semantic and the missing "output[i] carries ref X with value Y" opcode. Every `.rxd` carries an `// AUDIT:` header and precise `// LIMITATION:` notes; no guarantee was faked to compile.
370+
371+
### 11.4 Verification
372+
373+
Three exploit contracts that compiled before now behave correctly: the no-else bypass is **rejected**; `bool !=` emits `OP_NUMNOTEQUAL`; `9007199254740993` encodes exactly. Full suites green (545 passing, 0 failing), repo lint clean, all `dist/` rebuilt (the OP_2MUL/OP_2DIV regression shipped precisely because `dist/` was stale — not repeated here). Remaining residual limitations are documented in-code rather than silently assumed.
374+
375+
---
376+
377+
### 11.5 H-2 follow-up — full prevout verification before signing
378+
379+
The initial H-2 fix range-validated input satoshis but still trusted the provider's reported value/script. This follow-up closes that gap completely, because RadiantMM and RadiantSwap covenants derive payouts/splits from the value the SDK commits to the sighash. `Transaction.build()` now runs `verifyPrevouts()` **before signing** (default-on; opt out with `.withoutPrevoutVerification()` for offline signing):
380+
381+
1. **Authenticated fetch.** Each input's source transaction is fetched once per unique txid (in parallel) and its display txid is re-derived as `reverse(hash256(rawtx))`. If it doesn't equal the outpoint txid the spender already committed to, it's rejected. This is the crux: a malicious provider **cannot** forge a source tx with altered values, because the forgery would no longer hash to the committed txid — so the value/script assertions below are trustworthy even against a fully hostile provider, not just a buggy one.
382+
2. **Outpoint range.** `vout` must index an existing output of the source tx.
383+
3. **Value match.** The prevout's 8-byte LE value must equal the `satoshis` being signed (libauth 1.19 decodes output value as `Uint8Array`; compared byte-for-byte against `bigIntToBinUint64LE(satoshis)`).
384+
4. **Script match.** The prevout `lockingBytecode` must equal the script being unlocked — the contract's P2SH script (`addressToLockScript(this.address)`) for covenant inputs, or the P2PKH script of the signing key for `experimentalFromP2PKH` inputs. This rejects a UTXO that doesn't belong to the address being spent.
385+
5. **Consensus money range.** `assertMoneyRange` requires every committed value to be an integer in `[0, MAX_MONEY]`, where the new [`MAX_MONEY`](packages/cashscript/src/constants.ts) constant is Radiant's `Amount::max()` = `2,100,000,000,000,000,000` photons (21e9 RXD × 1e8 — 1000× Bitcoin; sourced from Radiant-Node `src/amount.h`).
386+
387+
The H-3 txid derivation was factored into a shared `computeDisplayTxid` used by both `getTxDetails` and `verifyPrevouts`. **11 new tests** in [`Transaction.test.ts`](packages/cashscript/test/Transaction.test.ts) cover the happy path, each rejection branch (value mismatch, wrong script, unauthenticated/forged source tx, missing vout, unfetchable source, the opt-out), and the `MAX_MONEY` boundary; the value-equality assertion was mutation-verified to fail when disabled. Existing fee/signing tests that use synthetic UTXOs call `.withoutPrevoutVerification()`. **End state: 449 root + 96 cashscript = 545 passing, 0 failing.**
388+
389+
---
390+
391+
*End of 2026-06-04 red-team pass.*

0 commit comments

Comments
 (0)