fix: address 5 audit findings (receipt binding, pin handling, strict DoS bound, CI pinning)#9
Merged
Merged
Conversation
Verified reproductions for each. None change canonical output or the demo chain root, so existing signatures and cross-language vectors still verify. F1 (receipt replay, high): verify_receipt_signature now binds a receipt to the entry it is attached to (payload_hash, plus event_id when the entry declares one) after the signature check. Previously a genuine, validly signed receipt issued for one event verified Ok when copied verbatim onto a different entry, because verification only checked the receipt's own fields. Now surfaces as ReceiptError::EntryMismatch / receipt_entry_mismatch. F3 (fail-open pin, medium): the CLI rejects a malformed --trusted-pubkey as a usage error (exit 2) instead of letting the core silently fall back to record-declared keys with valid=true. F2 (pin vs unsigned, low): when a trusted pubkey is pinned, the lenient verifier flags an unsigned record as unsigned_record instead of passing it, matching the documented "must have signed every record" contract; the --trusted-pubkey help text is corrected accordingly. F4 (strict DoS bound, low): the strict verifier rejects a line longer than MAX_LINE_BYTES (16x the payload cap) BEFORE parsing it, so a single oversized line cannot force an unbounded parse or canonicalization. The MAX_PAYLOAD_BYTES doc no longer overstates that it bounds parse cost. Adds a regression test for each.
Both jobs installed wasm-pack via `curl ... init.sh | sh`, executing a live, version-unpinned remote script on every run. Replace with `cargo install wasm-pack --locked --version 0.14.0`, a reproducible checksummed install from crates.io. Verified it compiles cleanly.
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
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.
Summary
Five reported findings, each verified by a concrete reproduction before fixing. None change canonical output or the demo WAL chain root, so existing signatures and cross-language vectors still verify.
Findings and fixes (in priority order)
F1 - Receipts not bound to their entry (high)
A genuine, validly signed server receipt issued for event A verified
Okwhen copied verbatim onto a different entry B, becauseverify_receipt_signatureonly checked the receipt's own signed fields and never compared them to the entry. The receipt is also outside the chain hash, so the staple was undetectable. This defeated the receipt layer's "the server attested to THIS event" guarantee.Fix: after the signature check, bind the receipt to the entry (
payload_hash, plusevent_idwhen the entry declares one). A mismatch isReceiptError::EntryMismatch, surfaced by the lenient verifier asreceipt_entry_mismatch(sovalidbecomes false). The module doc, which previously overstated the protection, is corrected.F3 - Malformed
--trusted-pubkeyfailed open (medium)A fat-fingered pin made the lenient core warn and silently fall back to record-declared keys, returning
valid=true/ exit 0. The strict path already rejected the same input.Fix: the CLI validates the pin (64 hex, 32 bytes, optional
0x) up front and returns a usage error (exit 2) on a malformed value, consistent with strict.F2 - Pin did not require records to be signed (low)
With a pin set, the lenient verifier still accepted unsigned
(None, None)records, contradicting the--trusted-pubkeyhelp text ("MUST have signed every record").Fix: when a pin is set, an unsigned record is flagged as
unsigned_record. The help text is corrected to describe the exact behavior (and notes that--strictis the all-signed profile).F4 - Strict DoS caps applied after parse (low)
MAX_RECORDS_DEMO/MAX_PAYLOAD_BYTESonly bounded the result of parsing: a single oversized line was fully parsed (and its payload canonicalized) before the payload cap could reject it.Fix: a new public
MAX_LINE_BYTES(16x the payload cap) is checked before parsing each line, rejecting an oversized line asLineTooLargewithout parsing it. TheMAX_PAYLOAD_BYTESdoc no longer claims to bound parse cost.F5 - CI installed wasm-pack via
curl | sh(low)Both jobs piped a live, version-unpinned remote script into
sh.Fix:
cargo install wasm-pack --locked --version 0.14.0, a reproducible checksummed install from crates.io (verified it compiles cleanly).Tests and verification
cargo fmt --all --checkclean;cargo clippy --workspace --all-targets -- -D warningsclean.cargo test --workspace --all-features: all pass (97 core, 22 CLI smoke, 7 cross-language vectors, 4 parity, 3 wasm).