ferry-security-audit-2026-09-11 - #5
Merged
Merged
Conversation
… (F01) Audit finding F01 (High): a Bitcoin participant could pay less than the agreed amount and still have a Zenon initiator's Auto Mode redeem the contract once the payment confirmed. The redeem publishes the initiator's secret, which is what lets the participant unlock the full Zenon HTLC -- so the attacker collected all the ZNN for a Bitcoin payment they chose the size of. Refresh only logged the shortfall; neither Manager.Redeem, the Redeem button, nor Auto Mode checked the amount. The rule now lives in one place, Swap.RedeemHeldForShortFunding: the contract holds less than AmountSats AND this side's redeem would be the first publication of its secret (initiator, receiving leg). Manager.Redeem refuses on it, whoever asked, with an error that says what it protects and names the deliberate way round it (the Recover page, once the Zenon HTLC has expired and been reclaimed or was never created). The swap view carries the same flag; the card withholds the button on it, which is the gate Auto Mode already goes through, and explains why in a warning. The participant's redeem of the same short contract is deliberately NOT held: their secret came off the counterparty's Zenon unlock and is already public, so taking whatever the contract holds gives nothing away. Regression test covers all three shapes: initiator short (refused, swap untouched), initiator full (redeems), participant short (redeems). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
From the Codex Security review of 422d169 (APPROVE WITH NITS): - The card's next-step line said "Redeem the Bitcoin" for a swap whose redeem button is withheld. It now says the funding is short and names what clears it; the stage track and the waiting-on-them rule follow. - The refusal, the card and the comments now say precisely what clears the hold: a single output covering the agreed amount. Refresh never adds short outputs together, so "wait for the full amount" was too loose. "ZNN" becomes "the Zenon leg" where the token is a term of the trade, and the secret goes in a signature script, not a witness. - The recovery route is named with its cost: broadcasting that redeem publishes the secret. - Tests: the stub backend now counts broadcasts and the refusal asserts zero. A second test drives the hold the way a live swap reaches it -- Refresh adopts a confirmed short output, Redeem is refused, two short outputs that only cover the amount together still hold, and a single covering output clears it with exactly one broadcast. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
Audit finding F02 (High): when Bitcoin initiated the swap, the participant was offered the Zenon create -- and Auto Mode took it -- as soon as an UNCONFIRMED output appeared at the contract. planCreate checked that the contract was audited and nothing about the money in it. The initiator, who holds the secret, could replace the mempool payment, keep the BTC, and unlock the participant's HTLC for the full ZNN. The rule lives in Go. Swap.ZenonCreateWaitsOnBtc names the one shape that answers the counterparty's funding (participant, receiving BTC); for it, Swap.FundingCommitBlocker judges the record as of the last refresh -- missing, spent, short of the agreed amount, in the mempool, or fewer than commitConfirmations (1) deep -- and says which. planCreate refuses on that, then re-reads the chain: the funding outpoint must still be unspent at the contract address, still cover the amount, still be mined, and still be deep enough, counted against the tip now. A chain that cannot be read is a refusal, not a pass. The Zenon-initiated ordering never consults the chain: that leg goes first by design and has no Bitcoin to wait for. The swap view carries fundingCommitted and fundingCommitBlocker. The card does not offer the create until committed -- which is what makes Auto Mode wait here instead of halting on the refusal -- and shows the blocker in the panel's place, with a note that the same applies to znn-cli. The wallet panel discards a prepared block if the funding stops being committed, and sign() re-reads the chain through Refresh immediately before the block is handed to the wallet. scripts/wallet-devnet.mjs and the console recipe in EXTENSION-WALLET.md built the Bitcoin-initiated participant's create with nothing funded, which is now exactly the refused case. Both switch to the Zenon-initiated shape for the block they take apart, and the script asserts the refusal for the other. The devnet script was not executed here (no devnet); it parses. Tests cover the record verdicts, the shapes that must never ask the chain (against a backend that fails every call), and the fresh chain check: still there, gone, replaced by a short output, unmined again, unreadable, and the depth threshold at one and two blocks. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
From the Codex Security review of fafac17 (Daybreak, REQUEST CHANGES): 1. The printed znn-cli create command was still generated while the card was warning that the funding was not settled. The gate lives in Go and a terminal is the one path it cannot reach, so the text now carries the refusal: while the funding is not committed, the htlc.create line is replaced by a comment naming the blocker, and the reclaim stays. A new scripts/zenon-commands.mjs pins that for missing, mempool, short and spent funding, that the command returns once committed, and that the Zenon-initiated and unlock shapes are untouched; it runs in CI and in `npm test`. 2. The pre-dispatch re-check in ZenonWallet.sign() went through Refresh, which is a projection that keeps what it last knew when a read fails -- the opposite of what a check before an irreversible step needs. It now calls a new `fundingCheck` method that runs the same fail-closed requireCounterLegFunding as planCreate: exact outpoint, full value, mined, deep enough against the tip now, unspent, and a chain that cannot be read is a refusal. The plan is discarded on any refusal. Covered through the call table, offline, in Go and in the smoke suite. Nits taken as well: the blocker notice moved out of the Zenon section, which does not appear until a funding record exists, so a swap with nothing paid yet gets the explanation the previous commit claimed it did; the tip-unreadable branch of the chain check is tested; and docs/SECURITY.md documents the one-confirmation policy, what it does and does not protect against, and how to raise it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
From the second Codex Security review (Daybreak, REQUEST CHANGES): the printed htlc.create command was decided off the swap's cached fundingCommitted, which is only as current as the last refresh -- it did not block an expired Bitcoin leg, and a confirmed status survives a refresh that could not read the chain. The review reproduced it by calling the shipped generator with a cached-committed record. The generator no longer consults the record for that decision. Where the create answers the counterparty's Bitcoin funding, znnCommands prints the command only when its caller passes createAllowed, and the card sets that from the same fail-closed fundingCheck the wallet panel runs before dispatch -- asked again on every swap change, which includes every refresh tick, and whenever the commands panel or the Bitcoin settings change. Absent or refused means withheld, with the reason in the command's place. The Zenon-initiated shape is untouched. The record's own verdict gains the two states the review named: an expired Bitcoin leg (theirs to take back), and a locktime too close to fit a Zenon leg that must expire before it. Tests: the Node command check now starts from the review's reproduction (a cached-committed record prints nothing) and covers expired and unreadable alongside the earlier cases, plus a committed record not overriding a live refusal. Go covers expired and too-close in the record verdicts, and the fundingCheck call is exercised end to end through a stub Esplora served by httptest -- settled funding passes, a vanished output is refused -- rather than only the helper. Nits: walletSent's JSDoc back beside walletSent; the smoke script's whole-file reformat undone, leaving only the added section. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
…d expiring From the third Codex Security review (Daybreak, REQUEST CHANGES), two defects in the card's gate on the printed znn-cli create -- the engine's own gate was confirmed sound: 1. A passed check stayed in force while a re-check was pending, an older check finishing late could overwrite a newer refusal, and a refresh that failed changed no prop and so never re-asked. The lifecycle now lives in core/cli-create-gate.ts, a state machine with no Vue in it: asking revokes at once, only the latest answer counts (a sequence number), and a pass is good for 45 seconds and no longer. The card re-asks on every swap, panel or Bitcoin-settings change as before, and on a 20-second timer besides, which is what a failed refresh cannot suppress; the timer also re-reads the verdict, so an expired pass stops printing even before the next answer. 2. The blocker text goes into a shell comment, and only its first line was prefixed. An Esplora error body with a newline in it -- a node this browser was pointed at, or traffic to it -- would have produced an uncommented, pasteable line; the review reproduced it. Every line of any foreign text now gets its own "# ", with CR and CRLF treated as line breaks. Tests, under Node with no browser: the four lifecycle cases (pass then pending, older pass after newer refusal, pass then revoke, pass then expiry), a failed check carrying its reason, and newline / CR / CRLF in the blocker leaving no uncommented line but the reclaim. Nit taken: the Node scripts that import TypeScript pass --experimental-strip-types, so they run on Node 22 as well as the Node 24 CI pins. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
Four Codex Security reviews in a row have approved the engine's gate on locking ZNN -- planCreate reads the chain when it builds the block, the wallet panel reads it again before signing -- and each has found a fresh stale moment in the gate on the PRINTED command: a cached flag, then a cached check, then a pending check that kept its old answer, then an expiry the rendered value did not see and a timer that could supersede a slow answer forever. The pattern is the lesson. A command copied into a terminal passes through no check, and a gate on the text that depends on a remembered answer always has a moment where the answer is stale. So for the participant in a Bitcoin-initiated swap the create is not printed as a command at all, whatever the record says and whatever a caller passes. The block prints the leg's terms as comments -- recipient, token, amount, hours, hash type, hashlock -- and the last refresh's verdict, with the instruction to check the funding on one's own node immediately before running anything, and points at the wallet button, which is the path with the check in it. The reclaim is still printed. The Zenon-initiated leg and the unlock shapes are unchanged: they answer no Bitcoin funding. That removes the card's cached authorisation, its timer, and the cli-create-gate module, and with them the class of defect. The comment escaping stays: every line of any text placed in the block gets its own "# ", CR and CRLF included. Tests: the shape's create is withheld across a committed record, every blocker, and a caller claiming to have checked; the terms and verdict are present as comments with nothing executable but the reclaim; the other shapes print as before; LF, CR and CRLF injection leave no uncommented line. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
… amounts From the fifth Codex Security review (Daybreak, REQUEST CHANGES): the withheld-create block prints the leg's terms as comments, and the amount line interpolated amountDisplay -- which comes from the counterparty's offer, unchecked for line breaks -- with only the start of the line prefixed. A line break inside a stranger's offer amount was an uncommented, pasteable line. The review reproduced it for LF, CR and CRLF. Two fixes, one at each end. Every term line now goes through comment() whole, so no value that reaches the block from outside the page can end a comment. And a Zenon amount is now a plain decimal or it is refused, at the two doors it comes in by: DecodeOffer and Create. The shape is digits with an optional point and more digits -- no sign, no exponent, no separator, nothing that is not a character of a number. Tests: Go refuses line breaks, exponents, signs and separators in canonicalZenonAmount, through an encoded offer, and through Create, and accepts the plain forms; the fresh chain check refuses a different transaction at the same address and the same transaction's other output. The Node command check now poisons amountDisplay, peerAddress, tokenStandard, secretHashHex and fundingCommitBlocker with LF, CR and CRLF and asserts nothing executable but the reclaim survives. Nits: SECURITY.md no longer says the locktime is re-read from the chain (it is the audited contract's, from the record); the workflow comment and the card's tooltip say the create is never printed for this leg rather than withheld until funding settles. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
…F03) Audit finding F03 (High): the Zenon addresses and amount are optional at creation, and Verify treated a blank expectation as a check switched off. An incoming HTLC with the right hashlock, token, amount and expiry that paid the COUNTERPARTY's own address verified clean on a swap created without an address of its own; planUnlock then encoded the preimage on the strength of that verdict, and the counterparty took back their ZNN with the secret that opens the Bitcoin leg. A blank agreed amount skipped the amount comparison the same way. A check switched off reads exactly like a check that passed, so now it reads like a refusal. VerifyParams gains MissingTerms, and Verify reports each as a problem -- not a pending one: no node answering later can supply a term the user never agreed. zenonVerifyParams records the missing recipient (this user's own address for an incoming HTLC, the counterparty's for their own); zenonExpectations records a missing amount. VerifyZenon, FindZenonHtlc, session hand-offs and walletSent all run through those expectations, so every route to `verified` closes at once, and planUnlock already requires `verified`. Records created without those terms need a way to get them, and the remediation is specific that it must not be the address the entry happens to pay. Manager.SetZenonTerms (call table: zenonTerms) fills a blank term from the user -- own address, counterparty's address, amount -- validates it, refuses to change a term already recorded, and un-verifies the leg when anything changes. The card shows a form for whichever terms are missing, ahead of the Verify row it unblocks, with a button to take the own address from the connected Syrius wallet. canonicalZenonAmount is the same block, at the same place, as on the F02 branch, so the two merge as one. Tests: Verify refuses a missing term and does not mark it pending; through the manager against a stub node, an HTLC paying the counterparty is refused on a swap with no own address, refused by name once the address is added, verified once it pays this user, and refused on a swap with no agreed amount; SetZenonTerms fills blanks only, validates, un-verifies, and refuses to move an agreed term. The smoke suite exercises the call offline. README names the new refusal. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
…ounts before they bind From the Codex Security review of 6f34bcc (Daybreak, REQUEST CHANGES): 1. A `verified` reached by the release that had this finding survives an upgrade in localStorage and a backup import, the card hid the repair form behind it, and planUnlock trusted it. Now Swap.MissingZenonTerms is the one rule -- recipient for the leg's direction, and a positive amount -- and a stored verdict reached over missing terms is withdrawn on every load, with the reason left on the record and said once in the log. The repair form appears, because the record no longer reads as verified. planUnlock refuses over missing terms regardless of the flag, and then reads the entry from the node again and holds it to today's expectations immediately before the preimage is packed; a node that cannot answer is a refusal. 2. An amount becomes immutable the moment it is recorded, so SetZenonTerms now refuses zero and anything the agreed token cannot represent, which needs the token's decimals from a node -- without one the amount is refused rather than recorded. The verifier treats a zero agreed amount as a missing term. The handler carries settings for the node. 3. The two amount doors from the F02 branch -- the create form and offer decoding -- are applied here verbatim, so the validator's comment is true on this branch too and the branches merge as one. Tests: the own-HTLC direction refuses without the counterparty's address; a stale verified record is withdrawn on load and after export/import; planUnlock refuses over a missing term without packing, refuses a complete record whose entry now pays the attacker, and plans the unlock once the entry pays this user; SetZenonTerms refuses zero, over-precise and node-less amounts. The smoke suite covers zero, no-node, and the stale import offline. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
… and repairable From the second Codex Security review of this branch (Daybreak, REQUEST CHANGES): 1. The printed znn-cli unlock carried the real preimage and whatever HTLC id the record held, verified or not. The wallet button will not build that block until the HTLC passes verification and reads the entry off the node again before the preimage goes in; a terminal does none of that, and a gate on the text that depends on a remembered verdict has the stale moment the F02 rounds found five times. So the runnable line is not printed. The id is, as a comment, with the instruction to use the wallet button, or to press Verify HTLC first and compose the command from the id and the preimage the card already shows. receiveAll stays. 2. Go called a zero amount missing; the card only called an empty string missing; SetZenonTerms refused to replace any non-empty value; Create still accepted zero. Now the swap view carries missingZenonTerms -- each with its field and reason -- and the card's repair form is built from that rather than from its own reading of the fields, so the two cannot disagree. A recorded zero is treated as blank by SetZenonTerms, which is the one way an amount may move, because zero was never an amount; and zero is refused at both doors, the create form and an offer. Nit taken: planUnlock over an entry the node cannot answer for leaves the block empty, tested. Tests: every record state of the unlock shape prints no runnable unlock and never contains the preimage; a zero recorded before the rule is withdrawn on load, reported as the missing term, and repairable; zero is refused at create and in an offer; the smoke suite checks the view's missing terms by field before and after a repair. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
…rinted HTLC id too Two nits from the approving review: the card's notice read "This swap has no your Zenon address", so it now lists the reasons Go supplies; and the injection matrix covers the HTLC id that the unlock shape prints as a comment, with LF, CR and CRLF. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
Refuse to redeem a short-funded contract when that reveals the secret (F01)
Audit finding F04 (High): ParseContract compared opcodes at the template's fixed positions and, at the five parameter positions, accepted any push whose payload had the right length -- the tokenizer hands back the payload whatever opcode carried it. A hash or key wrapped in OP_PUSHDATA1/2/4 parsed as the right bytes, audited clean under the agreed keys and hash, and hashed to a different P2SH address. The victim paid that address (or locked ZNN against it); their redeem, built by this program's own signer under standard policy, was then refused for the non-minimal push, while the counterparty's refund branch worked. The remediation's first option: after the terms are parsed, the contract is rebuilt from them the one way this program builds it, and anything but that byte string is refused, with the reason. Canonical is the only encoding whose spends this program has proven, and now the only one it will audit. AuditContract already returned before touching the swap, so a refused contract neither enables a Zenon create nor displaces an accepted one -- now tested. Tests: every parameter position under OP_PUSHDATA1, 2 and 4 is refused at parse for its encoding while the canonical script is accepted; the signer refuses the wrapped contract's redeem and accepts the canonical one's, which is the finding's premise; a refused contract leaves the swap untouched before and after a good one. The smoke suite audits a canonical contract, refuses its OP_PUSHDATA1 twin, and checks the accepted one is kept. README's audit step names the new refusal. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
Lock ZNN only against confirmed, full, unspent Bitcoin funding (F02) Conflicts were additive in all three files: F01 and F02 guard disjoint shapes of the same leg -- F01 holds the redeem for the initiator who is receiving BTC, F02 holds the Zenon create for the participant in a Bitcoin-initiated swap -- so both sides of every conflict are kept. - ui/src/types/index.ts, wasm/api.go (fields): each side adds its own flags to SwapView after fundingShort. Both kept, in that order. - wasm/swap.go: each side appends its own functions after SecretArrivesOnZenon. Both blocks kept. - wasm/api.go (the swapView literal) was the only hand merge. F01 moved the FundingShort expression into Swap.FundingShort(); F02 still had the inline copy. The method wins -- same value, one definition -- and F02's two fields join it. Verified: GOOS=js GOARCH=wasm go build/vet, go test ./... (including F01's short-funding tests and F02's TestCounterLegFundingGate), and the 129-check smoke suite. The merge is a whole-tree union of both changesets: diffing it against either parent reproduces the other PR's patch exactly, modulo gofmt realignment of two struct keys. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012B1CdWnUE6TWRMRGHsA6be
Refuse to verify a Zenon HTLC against terms the swap never recorded (F03) PR #3 is stacked on PR #2, so with #2 already merged the only conflict left is against F01 -- and it is the same one file, the same literal, as last time: wasm/api.go's swapView. F01 moved the FundingShort expression into Swap.FundingShort(); this branch still carried the inline copy. The method is kept and F03's MissingZenonTerms joins the other flags. Everything else merged clean, including the four files all three findings touch. That is not luck: each guards a distinct shape -- F01 the redeem for an initiator receiving BTC, F02 the Zenon create for a participant in a Bitcoin-initiated swap, F03 the verify for a swap missing its terms -- so the flags, the refusals and the card's notes sit side by side rather than on top of each other. canonicalZenonAmount landed identically on both branches, as F03's commit message promised, and merged as one. Verified: GOOS=js GOARCH=wasm go build/vet, go test ./..., 141/141 smoke checks and the zenon-commands suite. As with #2, the merge is a whole-tree union: diffed against either parent it reproduces the other side's patch exactly, modulo gofmt realignment. The UI typecheck was not run -- npm here refuses the nom-ui git dependency, so node_modules cannot be installed; CI covers it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012B1CdWnUE6TWRMRGHsA6be
…d trips Three nits from the approving review: the refusal no longer says which branch suffers, since a non-minimal locktime or refund push hurts the refund branch; the wrapped-contract test now claims only what it proves, that the signer refuses to build the spend at parse; and the canonical rebuild is round-tripped at the threshold, 0x7fffffff, 0x80000000 and the top of the 32-bit field, across the four-to-five-byte encoding change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
Audit only the canonical encoding of the contract (F04) F04 branches off v0.1.0 rather than stacking, so it met F01, F02 and F03 at once -- and still touched almost nothing they did. Its subject is wasm/htlc.go, which none of the other three opened. One conflict, in scripts/smoke.mjs: F02, F03 and F04 each appended a section to the end of the suite, so the three landed on the same line. Both sides are kept, the merged branch's sections first and F04's after, which is the order a rebase would have produced. The blocks are independent -- no const collides across them, and the only name F04 takes from earlier in the file, `receiver`, is declared at line 315 in the shared base and read for an immutable field. README merged clean: F04 extends the audit step's sentence, and nothing else had rewritten that paragraph. Verified: GOOS=js GOARCH=wasm go build/vet, go test ./..., 145/145 smoke checks (F04's four among them), the zenon-commands suite, and vue-tsc --noEmit. This merge is an exact union in both directions -- the diff against either parent reproduces the other side's patch byte for byte, with no gofmt drift this time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012B1CdWnUE6TWRMRGHsA6be
Contributor
|
I checked the relay-delivery path against 19f5913 using offline tests. I am preparing a focused follow-up for event deduplication, with verification remaining in WASM and regression coverage for asynchronous delivery in sessions and the board. I will keep the patch separate from the contract and financial-state changes in this audit and open it as a draft for review. Please flag any overlapping work or a preferred target branch. |
…o what it pays (F05) Audit finding F05 (Medium): a session peer could send a second, valid contract after the first was funded. AuditContract re-audited it, stored it beside the original funding outpoint, and every spend from then on was built for a script the output does not pay -- refused by the network, and not by the local engine, which was fed the record's contract rather than the chain's. The initiator kept the refund branch of the funded contract. Two rules, one at each end. At the door: Swap.ContractCommitted says whether anything has been staked on the contract as it stands -- funding seen or sent to its address, a refund pre-signed, a Zenon HTLC created against its locktime, a state past waiting for funding -- and what. Past that point the contract's bytes are the swap's identity. AuditContract answers a byte-identical resend (a session retransmission, a re-sync, a second paste) with the swap as it is, logging nothing, and refuses different bytes however well they audit, naming the stake. Before a commitment a re-audit still replaces, which is how a counterparty rebuilding with a better locktime gets a second chance. SetCounterpartyPKH, the funding side's mirror, freezes the same way. At the signer: FundingOutput records the script the output actually pays, read from the funding transaction when the funding is adopted and filled in from the chain before signing when an older record lacks it; a chain that cannot be read is a refusal. buildSpend refuses a contract that does not hash to that script, so the check holds for the Recover page and a restored backup too. Redeem and Refund run the binding before building. Tests: identical resend idempotent, different contract refused with the record untouched, every kind of stake freezing, re-audit before a stake still replacing; the pubkey-hash mirror; the funding bound on adoption from a real transaction, a swapped-out contract refused before signing both through Redeem and through the bare signer, an unbound record bound from the chain before signing, and an unreadable chain refusing. The smoke suite checks a repeated audit adds nothing. README's session section says a staked contract's bytes are the swap's identity. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
…here From the Codex Security review of 10aa139 (Daybreak, REQUEST CHANGES): 1. Every call into this module runs on a goroutine of its own, and the store locked Load and Save separately, so an audit that loaded before a refresh recorded the funding could save after it -- erasing the stake, and the freeze with it. Swap.Version now counts saves and Save is a compare-and-set on it under the store's lock: a record whose version is not the one in the store is refused with ErrStaleWrite, and the loser looks again. Every load-decide-save path is covered at once. A test makes the interleaving happen on demand, through a storage that lets a refresh through mid-audit, and shows the audit losing and the funding and original contract standing. 2. An absent binding was treated as a pass. Now the refund is not pre-signed until the funding is bound (a chain that cannot say what the output pays holds it, with one log line, and it is tried on every poll); Refund binds before broadcasting even a pre-signed refund, which may date from a release that did not bind; and the Recover page, which reaches no node, refuses a file that does not record the binding unless the user ticks "build anyway", and then says on the result what it assumed. Recovery files written from now on carry the binding. Tests, as the review listed them: a stale copy refused and a reloaded one accepted; the audit-versus-refresh race; identical contract and pubkey hash resends performing no write, counted; Refresh with an unreadable funding transaction not pre-signing, once logged, then binding and signing when readable; a covering output replacing a short one, unbound and un-signed while unreadable, bound and signed when not; a pre-signed refund over a mismatched or unreadable funding not broadcast; a recovery file without the binding refused unasked, built with a warning on consent, and refused regardless when its binding names another script. The smoke suite covers the consent path offline. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
…mage out of unbound redeems From the second Codex Security review of this branch (Daybreak, REQUEST CHANGES), four required items: 1. Two tabs are two modules over one localStorage, each with a mutex of its own, so the version check could not see the other tab's write. The Web Locks API is the one primitive a browser offers across tabs of an origin, so every call into the module is now made under a lock named for this instance's storage prefix (main_js.go underStoreLock): one at a time, across every tab. The callback hands the browser a promise and does the work on a goroutine, because Go that blocks on the event loop deadlocks the module. Where the API is absent -- Node in the smoke test -- the call runs unlocked as before. A Go test cannot exercise a browser lock; what Go proves is the in-module version check, and the lock is what extends it. 2. The Recover page's "build anyway" reached BuildRedeem, and an invalid redeem submitted anywhere still shows its preimage. Consent now covers refunds only, which reveal nothing; a redeem from a file without the binding is refused with no way round, and the page's wording says so. Consent resets whenever the file changes. 3. Import decided "absent" and wrote separately; delete judged the risk and removed separately. Store.SaveIfAbsent and Store.DeleteIf do each under the store's lock, and Import and handleDelete use them. 4. A stale write after a broadcast reported failure for a transaction the chain already had. Redeem and Refund now record the outcome through saveOutcome, which on a stale write reloads, applies the outcome to the record as it now is, and saves that -- the concurrent change kept, the outcome not lost, nothing broadcast twice. The session applies a value again on a stale write before reporting a refusal; the background refresh lets the next tick take it. Nits taken: the version advances on the caller only once the write has happened, so a failed write does not poison the next attempt; the funding transaction a backend serves must hash to the id asked for. bindsTo stays open on an absent script, because every path that signs binds first and the bare signer is what the tests call directly. Tests: add-if-absent not overwriting an existing record; delete refusing under its check and removing when allowed; a failed write not advancing the version; an outcome surviving a concurrent archive with exactly one broadcast; a substituted transaction not binding; an unbound redeem refused with and without consent while an unbound refund builds with a warning and a bound redeem builds. The script-mismatch fixture now uses a real transaction under its own id. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
…o resurrect From the third Codex Security review of this branch (Daybreak, REQUEST CHANGES), five items: 1. A browser without the Web Locks API ran calls unlocked, which is the two-tab race this branch exists to close. Now only an environment with no document -- Node running the smoke test -- runs unlocked; a browser without the API is refused, by name. 2. The receiving leg's funding was marked and shown as funded before it was bound, and only the sending leg's refund block retried the binding. Refresh now tries to bind on every poll for either leg, once in the log, and the swap view carries fundingBound: the card offers no Zenon action and no redeem against a funding that is not yet read off its own transaction, and says so beside it. 3. Add-if-absent treated a stored value that no longer parses as a record, so a backup could not restore it. A corrupt value is absent. 4. saveOutcome re-applied a spend to whatever the reload returned. It now refuses a record naming a different funding output or contract, refuses to bring back a record deleted in the meantime -- Save itself refuses a versioned record whose key is gone -- and carries the transaction id in the error either way, so what happened on the chain is not lost. 5. The stale-write error is named on the wire (code "stale"), the client throws an EngineError carrying it, and the session retries exactly once on that code rather than on the text. Nit taken: the output index is compared without a narrowing conversion. Tests: the receiving leg's funding not actionable until bound and bound on a later poll; an outcome refused over a swapped funding and over a deleted record, one broadcast either way; a backup restoring a corrupt record and leaving a healthy one alone; the stale code on the wire and absent from an ordinary error. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
From the fourth Codex Security review of this branch (Daybreak, REQUEST CHANGES), one required item and a real bug: bindFunding recorded the script it read BEFORE checking it against the contract, so on a mismatch the error was logged but the record was saved with the script set, the view called it bound, and the card offered the Zenon create -- the loss the finding describes, for a record whose contract had already been replaced or a backend that lies. Now the script lands on the record only once it is known to be this contract's, and a mismatch is said once in its own words. Swap.FundingBound is the one predicate -- script present AND this contract's, judged against the contract the record holds now -- and the view reports that. planCreate refuses to lock ZNN while a funding on the record is not bound, whether unread or mismatched, before any node is asked. Tests: a real transaction paying another contract leaves the funding unbound and unrecorded, logs the mismatch once over two polls, and is refused by planCreate, as is a funding merely unread; a script recorded for one contract does not bind another; the outcome identity check exercised for the output index alone and the contract alone. Nit taken: main_js.go's comment says what the fallback now is. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
Nit from the approving review: the adoption block logged every binding failure as a transaction that could not be read, a mismatch included, on top of the mismatch's own one-time note. A mismatch is now its own error kind and the misleading line is not written for it; tested. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
The branch is rebased onto 092026-audit, where F01 through F04 have been merged. Where two branches inserted at one anchor both sides are kept; this commit is what the overlap needed beyond that: - planCreate asks whether the funding is even this contract's (F05's binding) before how deep it is (F02's commitment), so a mismatched or unread funding is refused by name rather than as unconfirmed. - F01's redeem fixtures carry the binding, and its covering-output test serves real transactions so the adopted outputs bind; F05's freeze test gives the swap complete Zenon terms so F03 does not withdraw the verdict on load; F03's amount loop saves one record per case under versioned saves. - The swap view lists each field once. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
…ts terms are real (F06) Audit finding F06 (Medium): an offer's Zenon amount reached the printed znn-cli command unquoted, so `1$(id)` in a stranger's offer was a command substitution in the victim's terminal. On the audit branch the amount itself is already closed -- canonicalZenonAmount at offer decoding and at swap creation admits digits and a point and nothing else -- and neither the create for the leg that answers Bitcoin funding nor the unlock is printed as a command any more. What this commit adds is the rest of the remediation, so that validation is not the only thing between a field and a shell. Every value in a runnable line is one POSIX shell word: single-quoted, with a quote inside written as the quote-escape-quote idiom. Nothing in a single-quoted word is special to a shell, so whatever a value holds, the shell hands it to znn-cli as one argument. That covers the counterparty's address, the token, the amount, the hours, the hashlock, the HTLC id, the user's own address and the node URL. The placeholders a person fills in themselves stay unquoted and angle-bracketed, so they look like what they are. A create is runnable only once every term of the trade is on the swap; with one still missing the same line is printed as a comment to complete, because `<amount>` in a shell is a redirection, not a hint. A reclaim with no HTLC id is a comment the same way. The offer's Zenon address and token are held to their shape at decoding, as its amount already is: every field of an offer that can reach a command is checked at the door it comes in by. Tests: a non-executing POSIX reader over the whole runnable block, the way a terminal receives a paste, proves for seven fields and seven hostile values (substitutions, a backtick, a semicolon, a space, a quote, a newline, a variable) that no metacharacter is bare and the value arrives as exactly one argument; a create with any term missing, and a reclaim with no id, are comments; DecodeOffer refuses an address or token that does not parse. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
…alue Nits from the approving review: the test's reader now drops comments the way a shell does -- a bare # where a word would start, outside any quote -- rather than by line prefix, so a quoted value with a newline and a # is text; every argument carrying a hostile value must be that value whole and alone; the hours join the hostile matrix; and a missing hashlock, like any other missing term, leaves the create a comment. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
… (F07) Audit finding F07 (Low): a take is stamped by whoever signs it, a relay this browser was pointed at delivers whatever a key will sign, and the inbox formatted every take's stamp with toISOString() inside one shared render. A stamp Go's int64 holds happily but a JavaScript Date does not threw there, and the throw took every legitimate take down with the bad one -- their Accept and Dismiss controls included. Two changes, one at each end. At the read boundary, OpenTake refuses a stamp outside a window nothing real falls outside of -- before 2020, after 2100, or more than a day ahead of this reader's clock -- before the box is opened; a post's own creation and expiry times are held to the same window when the post is validated. On the page, the board's dates go through a total formatter: a time a Date can hold prints as before, and anything else prints a dash, never a throw. The inbox and both post rows use it, so Go's refusal is not the only thing between a stamp and a blank inbox. Tests: a take re-stamped and re-signed beyond what a Date can hold, in the far future, before the board existed, negative, or two days ahead is refused, while now and a day ahead (a wrong clock) are opened; a post expiring beyond any Date or created before the board is refused. A new Node check (npm run dates, in CI and npm test) pins that the formatter does not throw and prints a dash for a value beyond a Date, a hugely negative one, NaN, infinity, undefined and a string, and formats an ordinary time correctly. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
Nit from the approving review: the floor and ceiling are admitted, one second beyond either is not, and the take's future bound is tested five seconds inside and five seconds beyond. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYCAfS9BXS9N9eCe39BRCg
Quote every value in a runnable command, and hold the command until its terms are real (F06)
Admit only plausible board times, and print any time without throwing (F07)
…pe stripping Two findings from review of the branch (edgepillar, PR #7). The Web Lock wrapper discarded the promise that locks.request returns. A request the manager rejects before granting -- the specification allows a SecurityError -- never invokes the holder, so nothing answered the call and it hung past the API timeout, which starts only once the lock is held. The rejection is now handled: before a grant it answers the call as a refusal, naming the reason; once the holder is running it changes nothing, because the work is under way and its save may already have landed, so the only true answer is the work's own. The answer goes through a once, so no path can send twice on the channel from a browser callback, and a manager that throws synchronously releases both callbacks it never took. The smoke suite stands in a lock manager for each shape: refused before grant, granting in order and serialising two calls, rejecting after the holder has released, rejecting while it still runs, and throwing; and it counts any rejection left unhandled as a failure. EngineError declared its code as a constructor parameter property, which Node's type stripping refuses. Nothing under Node reached wasm.ts on this branch alone, but #6's relay test loads useSession.ts, which now imports isStaleWrite from it, so the combined tree failed to load. The field is declared and assigned instead. Verified by merging #6 on top and running its relay:delivery on Node 24: fails before, 12/12 after. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TiMc1yHDcGagiEPA6udCjm
Codex's review of the previous commit found that the rejection handler given to locks.request was released only when it fired, so every successful call left one callback registered in Go for the life of the tab. The request now gets a handler for each outcome and whichever fires lets both go, under a once that the synchronous-throw path shares. The smoke suite stands in a thenable manager that keeps the handlers the bridge attaches and invokes them after the call has answered: Go logs "call to released function" for a released callback rather than running it, and the check counts both. Scenario 3's comment no longer claims a conforming manager rejects after the holder has completed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TiMc1yHDcGagiEPA6udCjm
Round two of Codex's review found the synchronous-throw path did not ask whether the lock had been granted: a manager that invokes the holder and then throws left the work running while the call answered with the exception, which is the late-rejection shape by another route. The recovery now releases the request's handlers and, once granted, leaves the answer to the work. The smoke suite adds that manager over a call that reaches for a node, so the work yields and the throw really does arrive first; a thenable that refuses, to probe that both handlers are released on that path too; and narrower wording for what the section claims, since a request that never settles is the one shape nothing in the bridge can rescue. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TiMc1yHDcGagiEPA6udCjm
Round three of Codex's review approved with one nit: the smoke suite showed the request's handlers released but took the holder's release on trust from the source. The thenable managers now keep the holder as well and invoke all three after the call, on fulfilment and on refusal. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TiMc1yHDcGagiEPA6udCjm
Freeze the contract once anything is staked on it, and bind funding to what it pays (F05)
Deduplicate relay events after verification
sol-znn
marked this pull request as ready for review
September 15, 2026 12:17
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A security audit of Ferry and its remediation. Seven findings (F01–F07) were
raised against
master; all seven are fixed here, each with tests, and eachreviewed independently before merge. The branch also adds the audit bundle
itself, three new CI check suites, and verified relay delivery.
Security fixes
F01 — Short funding could buy the whole swap (High)
A Bitcoin participant could pay less than the agreed amount and still have a
Zenon initiator's Auto Mode redeem the contract once the payment confirmed.
The redeem publishes the initiator's secret, which unlocks the full Zenon
HTLC — so the attacker collected all the ZNN for a Bitcoin payment they chose
the size of. Refresh only logged the shortfall.
Swap.RedeemHeldForShortFundingis now the single rule: the contract holdsless than
AmountSatsand this side's redeem would be the firstpublication of its secret.
Manager.Redeemrefuses on it, whoever asked, andnames the deliberate way round it.
F02 — ZNN locked against an unconfirmed payment (High)
When Bitcoin initiated, the participant was offered the Zenon create — and
Auto Mode took it — as soon as an unconfirmed output appeared. The
initiator, who holds the secret, could replace the mempool payment, keep the
BTC, and unlock the participant's HTLC for the full ZNN.
The funding must now be present, cover the agreed amount, be mined at least
commitConfirmations(1) deep, and still be unspent — re-read from the chainwhen the block is built and again immediately before it reaches the wallet.
An unreadable chain is a refusal. Documented as a named risk in
docs/SECURITY.md§6, including why one block is not finality.F03 — A blank term read as a passing check (High)
The Zenon addresses and amount were optional at creation, and
Verifytreated a blank expectation as a check switched off. An HTLC paying the
counterparty's own address verified clean,
planUnlockencoded thepreimage on that verdict, and the counterparty took back their ZNN with the
secret that opens the Bitcoin leg.
VerifyParamsgainsMissingTerms; each is reported as a problem, not apending one — no node answering later can supply a term the user never
agreed. Stale verdicts are withdrawn and re-verified before unlocking.
F04 — Non-canonical contract encodings (High)
ParseContractaccepted any push whose payload had the right length. A hashor key wrapped in
OP_PUSHDATA1/2/4audited clean under the agreed terms buthashed to a different P2SH address. The victim paid that address; their own
redeem was then refused for the non-minimal push while the counterparty's
refund branch worked.
The contract is now rebuilt from its parsed terms the one way this program
builds it, and anything but that byte string is refused by reason.
F05 — A second contract after funding (Medium)
A session peer could send a second valid contract after the first was funded;
every later spend was built for a script the output does not pay.
Swap.ContractCommittedfreezes the bytes once anything is staked on them,and a funding binds only once its script is known to be this contract's —
Swap.FundingBoundis the single predicate the view andplanCreateread.F06 — Shell injection in the printed
znn-clicommands (Medium)An offer's Zenon amount reached a printed command unquoted, so
1$(id)in astranger's offer was a command substitution in the victim's terminal. Every
value in a runnable line is now one single-quoted POSIX word; a create whose
terms are incomplete prints as a comment rather than a line where
<amount>is a redirection; offer addresses and tokens are validated at decoding.
F07 — One bad timestamp blanked the board (Low)
A relay delivers whatever a key will sign, and the inbox formatted every
stamp with
toISOString()in one shared render — so a stamp outside what aDateholds threw and took every legitimate take down with it, Accept andDismiss included.
OpenTakenow refuses implausible stamps at the readboundary, and the page's date formatter is total: unformattable values print
a dash.
Other changes
verification (an id from a relay is only a claim until then), and a pending
verification can no longer apply a message to a room joined in the meantime.
versioned, and a stale write is retried against the record as it now is
rather than surfaced as a refusal.
npm run commands(printed-command shell safety),
npm run dates(total date formatting),npm run relay:delivery. All are innpm testtoo.manager_test.go,verify_test.go,walletblock_test.go,htlc_test.go,board_test.go)plus the Node check scripts.
audit/ferry-security-audit-2026-09-11-be879e1b/:findings, coverage, SARIF export, hardening proposals and report.
v0.3.2.