Cryptographic security audit — `witness-dao`

Scope: src/core/canonical.ts, src/core/hash.ts, src/core/merkle.ts, src/verify/offline.ts, src/anchor/{ots,rfc3161,rekor,anchorer}.ts, plus the report/pipeline consumers that determine what the signatures actually protect. Method: code reading plus executable proofs-of-concept run against the real modules (no mocks of primitives). Existing suite: 753 tests, all passing. Nothing in the repository was modified. Threat model: a DAO faction or litigant who supplies the verification bundle and the report inputs, holds no legitimate signing key, and can freely use public anchoring services.

Verdict

No. In its current state the cryptographic core is not sound enough to underwrite enterprise compliance evidence, and the gap is not in the primitives — it is in what the primitives are wired to. The individual building blocks are, with two exceptions, better than average for hand-rolled code: the RFC 6962 inclusion-proof recomputation is exactly correct against an independent reference over 820 vectors, the DER length handling is genuinely strict, and RFC 8785 canonicalisation gets the hard parts (UTF-16 code-unit key ordering across surrogate pairs, lone-surrogate rejection, sparse-array holes) right. But the layers above them do not consume what was signed. The liability-defence report — the commercial artefact — derives every substantive claim (who acted, what the binding confidence was, whether a tally was complete) from a caller-supplied, unsigned GovernanceEvent, never from the signed decision.inputs copy sitting inside the record, and nothing cross-checks the two; I flip a named subject from PROVEN participant to PROVEN non-participant using byte-identical ANCHORED_VALID records and zero caveats (F-01). Independently, anchorCoversRecord accepts a Merkle inclusion proof on string equality of leaf and root alone — verifyMerkleProof is exported to third parties but never called in any verification path — so an attacker who anchors any 32 bytes of their own choosing (free, and genuinely verifiable on Bitcoin) can claim that anchor covers a record that was never in the tree (F-02). Add a chain-continuity bypass that converts affirmative tamper detection into a clean pass by adding one decoy record (F-03), a compliance gate enforced on an unsigned field that fails open when the field is absent (F-04), and an anchor layer that writes state: 'confirmed' for fabricated self-signed timestamps and counts them toward quorumMet (F-05), and the composite result is that a motivated adversary can construct a fully self-consistent evidence pack that this library grades PROVEN. Every one of these is fixable without touching the primitives, and the codebase's own comments show the authors were reasoning about exactly these failure modes — the discipline just did not survive composition.

Findings

Per-finding detail

F-01 — Critical — The report reads its substance from an unsigned channel

What it is. normaliseEvidence takes each EvidenceInput and derives actors, binding, kind, occurredAt and the tally detail from input.event — a GovernanceEvent the caller passes in alongside the record:

• src/report/governance.ts:389 — actors = event.actors.map(...)
• src/report/governance.ts:398 — const binding = input.binding ?? event?.binding;
• src/report/liability.ts:337 — const detail = r.event?.detail ?? {}; (drives assessTally, hence complete_tally_sealed)
• src/report/liability.ts:543 — group.records.filter((r) => r.actors.includes(subject)) — the participation/non-participation decision

The sealer does write the same data into the signed body (src/seal/sealer.ts:157-167, decision.inputs.{actors,binding,kind,occurredAt,detail}), and src/pipeline.ts:353-363 reads decision.inputs.eventId — but only to look the event up in a map. The signed copy of actors, binding, kind, occurredAt and detail is never read and never compared to the unsigned copy. The record's signature therefore protects recordId, chainKey, seq, sealedAt, signedBy, prevHash, governingRule.ruleId/ruleVersion and decision.verdict (which verdictFor hard-codes to RECORDED, src/seal/sealer.ts:284-287) — and nothing else that the report actually prints.

Why it matters for evidence integrity. This is the product. strength.ts:161-165 reserves PROVEN for ANCHORED_VALID × onchain_confirmed on the stated grounds that "both halves are independently checkable by a third party who trusts no participant". The onchain_confirmed half is read from an object the party presenting the pack authored. The liability.ts header calls the honest treatment of absence "the whole engineering problem" — and absence is decided by r.actors.includes(subject), on unsigned actors.

PoC. Two buildLiabilityDefenceReport calls over the same two records, whose signed decision.inputs.actors contains the subject and whose signed binding is onchain_confirmed; both records verify ANCHORED_VALID. Only the event array differs:

SIGNED decision.inputs.actors  : ["0xabab…abab"]      <- the subject
SIGNED decision.inputs.binding : "onchain_confirmed"
offlineVerify verdicts         : [ 'ANCHORED_VALID', 'ANCHORED_VALID' ]

--- HONEST report ---
participation   : [ 'vote.cast/PROVEN', 'vote.cast/PROVEN' ]
nonParticipation: []
caveats         : 0

--- FORGED report (identical records; only the UNSIGNED event array changed) ---
participation   : []
nonParticipation: [ 'complete_tally_sealed/PROVEN' ]
caveats         : []            <- no caveat, no warning, nothing

Binding laundering in the governance report, same records:

claimed onchain_confirmed -> byStrength: {"UNSUPPORTED":0,"WEAK":0,"STRONG":0,"PROVEN":2}
claimed unbound           -> byStrength: {"UNSUPPORTED":2,"WEAK":0,"STRONG":0,"PROVEN":0}

Fix. In normaliseEvidence, derive actors, binding, kind, occurredAt and detail from record.decision.inputs, and treat input.event as corroboration only. Where an event is supplied, canonical-hash {kind,eventId,daoId,occurredAt,actors,binding,detail} from both sources and fail('REPORT_EVENT_RECORD_MISMATCH', …) on divergence — refusing is consistent with this module's stated "no silent omission" policy. Delete the EvidenceInput.binding override, or restrict it to lowering confidence (weakestStrength) so it can never launder upward. Add a test asserting that mutating event.actors without re-sealing makes the report throw.

F-02 — Critical — Anchor coverage accepted on string equality; the Merkle path is never checked

What it is. src/verify/offline.ts:595-609:

const proof = (attestation as unknown as Record<string, unknown>)['inclusionProof'];
if (proof && typeof proof === 'object') {
  const p = proof as { leaf?: unknown; root?: unknown };
  if (typeof p.leaf === 'string' && digestsEqual(p.leaf, record.bodyHash)) {
    // The anchor layer is responsible for validating the proof arithmetic …
    return typeof p.root === 'string' && digestsEqual(p.root, attestation.digest);
  }
}

The comment delegates the arithmetic to "the anchor layer". It is not done there either. verifyMerkleProof / verifyRecordInclusion appear only in src/anchor/anchorer.ts:613-619 and the public export lists (src/index.ts:120, src/anchor/index.ts:31); a repository-wide grep shows no call site in any verification flow. AnchorAttestation (src/core/types.ts:202-216) has no inclusionProof field at all — the verifier reads it through an as unknown as Record<string, unknown> cast, so the type system offers no help either.

Why it matters. Full forgery chain, no legitimate key and no cryptanalysis required:

1. Generate an Ed25519 keypair and publish/hand over a matching DID document (the bundle is caller-supplied — F-08).
2. Mint a hash-chained run of records from seq: 0, saying whatever you like.
3. Anchor any 32 bytes of your choosing — costs nothing, and Bitcoin/Rekor will genuinely timestamp it. The resulting attestation is authentically verifiable.
4. Attach inclusionProof: { leaf: <your record's bodyHash>, root: <the digest you anchored> }. No path, or garbage path.
5. Any honest anchorVerifier correctly reports the anchor as verified — it is verified. anchorCoversRecord accepts coverage because two strings line up.

Result: ANCHORED_VALID, and with F-01, PROVEN.

PoC.

=== anchorCoversRecord accepts a BOGUS inclusion proof ===
anchors = [{ kind:'bitcoin-ots', state:'confirmed', digest:'ff…ff',
             inclusionProof:{ leaf:<bodyHash>, root:'ff…ff', hashes:['00…00'], nonsense:true } }]
verdict: ANCHORED_VALID | anchorsVerified: [ 'bitcoin-ots' ]

The module docstring at src/verify/offline.ts:591-593 states: "A bare root with no inclusion proof does NOT count — that would let any record claim coverage from any anchor." A bogus proof does count, so precisely that is what happens.

Fix. Add inclusionProof?: { leaf: string; leafIndex: number; proof: MerkleProofStep[] } to AnchorAttestation and have anchorCoversRecord call verifyMerkleProof(record.bodyHash, proof.proof, attestation.digest). Reject any inclusionProof whose path is absent or empty unless attestation.digest === record.bodyHash (the single-leaf case — see F-14). Add a test asserting that a proof with a tampered sibling yields SIGNED_PENDING, not ANCHORED_VALID.

F-03 — High — One decoy record disables the hash-chain check

What it is. src/verify/offline.ts:510, at the end of the continuity loop:

if (current.seq === target.seq) break;

The loop iterates a seq-sorted copy. If any other record shares the target's seq and sorts before it, the loop breaks on that record and the target's own prevHash → predecessor.bodyHash link is never evaluated. Membership was established earlier by recordId (:445), so the target is still "in the chain".

Why it matters. The hash chain is the only mechanism that catches a party who holds the signing key and rewrites history — the repository's own fixtures say so (test/support/records.ts:196-197: "Models an actor who holds the signing key and rewrites history — the case where only the hash chain can catch the change."). That mechanism can be switched off by appending one extra record.

PoC.

CONTROL  evil alone at seq 2       : BROKEN | chain_prevHash_mismatch_at_seq_2
ATTACK   decoy@seq2 sorts first    : SIGNED_PENDING | chainValid: true
ATTACK   evil@seq1 + decoy@seq1     : SIGNED_PENDING | chainValid: true
ATTACK   orphan (no prevHash)@seq2  : SIGNED_PENDING | chainValid: true

The last line is the sharpest: a record with no prevHash at all, at a duplicated seq, reports chainValid: true.

Fix. Two independent changes, both needed:

1. Break on identity, not on seq: if (current.recordId === target.recordId) break; — or drop the early break and validate the whole supplied chain.
2. Reject duplicate seq values within one chainKey up front. Two records at the same seq are an equivocation, not an incomplete bundle; that is a BROKEN-class fact (see F-12).

F-04 — High — The compliance gate is enforced on an unsigned, fail-open field

What it is. src/pipeline.ts:313 calls assertCompliancePosture(record, …) for every record before it can enter a report. That function (src/seal/witness-client.ts:436) throws only when:

if (record.anchorState === 'not_anchored') { throw … COMPLIANCE_POSTURE_UNSAFE }

anchorState is the third entry in DEFAULT_ENVELOPE_FIELDS (src/verify/offline.ts:62), i.e. deliberately excluded from the signed body. test/verify/offline.test.ts:335-340 asserts that changing it does not break verification, and test/seal/witness-client.test.ts:135 asserts the guard "tolerates an absent anchorState". Both behaviours are individually defensible; together they mean the guard is enforced on a field the attacker may rewrite, and it fails open when the field is simply deleted.

Why it matters. assertCompliancePosture's own docstring calls this "the single most dangerous misuse of this library: presenting unanchored, Sealed-tier evidence as compliance-grade" and says such records "must not be relied on as regulatory or litigation evidence". The guard against that misuse is a === against an unsigned string.

PoC.

honest not_anchored: guard THROWS COMPLIANCE_POSTURE_UNSAFE
relabelled anchorState=anchored verifies as: SIGNED_PENDING | sigValid: true
  guard: PASSES  <-- BYPASS
anchorState DELETED  -> guard PASSES  <-- BYPASS (fail-open on absent field)
DEFAULT_ENVELOPE_FIELDS includes anchorState: true

Fix. Stop deriving compliance posture from a record field. Gate on the verifier's own verdict, which is computed from cryptographic material: require offlineVerify(...).state === 'ANCHORED_VALID' (which, once F-02 and F-05 are fixed, means something). Keep assertCompliancePosture as an advisory pre-flight only, and make it fail closed on an absent anchorState (if (record.anchorState !== 'anchored') throw). Never let a SealedRecord field be the sole basis for the posture decision.

F-05 — High — `confirmed` conflates asserted with verified, and the quorum ignores the caveat

What it is. Three separate places map an explicitly unverified result to state: 'confirmed':

• src/anchor/rfc3161.ts:1366-1379 — asserted_unverified → state: 'confirmed', with the caveat only in error.
• src/anchor/rekor.ts:863-877 — same pattern.
• src/anchor/ots.ts:1513-1520 — state = 'confirmed' when hasBitcoinAttestation(tree) is true; verifyOtsProof is called at :1506 with no blockHeaderLookup, so its verdict can only be asserted_unverified, and that verdict is discarded — only report.bitcoin[0]?.height is read.

evaluateQuorum (src/anchor/anchorer.ts:169-177) then counts kinds by state === 'confirmed' alone and never inspects error. src/pipeline.ts:208-216 sets complete from quorum.met. src/cli/output.ts:295 prints immediate quorum: met in green.

Crucially, no chainValidator, pinnedPublicKeyPem, or blockHeaderLookup is wired anywhere in src/cli, src/mcp or src/config.ts — grep returns nothing outside the anchor modules' own definitions. Out of the box, every anchor this library produces is asserted_unverified, and every one of them is stored as confirmed and reported as quorum-met.

Why it matters. src/anchor/anchorer.ts:26-29 promises: "No over-claiming. quorumMet is computed only from attestations whose state is confirmed … an attestation that carries an error string … keeps that caveat all the way into the report." The caveat survives into AnchorAttestation.error; the decision does not read it.

PoC. A fully fabricated TimeStampResp, self-signed with a locally minted "Totally Legit TSA" certificate, over a digest of the attacker's choosing, with genTime back-dated 26 years:

verdict            : asserted_unverified
granted            : true
genTime (attacker) : 1999-01-01T00:00:00.000Z
digestMatches      : true
contentDigestMatch : true
signature          : valid        <- verified against the cert INSIDE the token
chain              : not_checked

AnchorAttestation state produced by anchorRfc3161 : confirmed
confirmedAt written into the evidence trail       : 1999-01-01T00:00:00.000Z
evaluateQuorum(default policy, [fabricated]).met  : true

Offline re-verification of the stored fabricated proof also returns asserted_unverified with nonceEchoed: undefined (F-19).

Fix. Split the state: add state: 'asserted' between pending and confirmed, or add a required verification: 'verified' | 'asserted' field, and have evaluateQuorum count only confirmed && verification === 'verified'. Return both evaluations (as is already done for permanence) so nothing is hidden. Make MultiAnchorer refuse to construct an rfc3161 target with no chainValidator, a rekor target with no pinnedPublicKeyPem, and an ots target with no blockHeaderLookup, unless the caller passes an explicit allowAssertedOnly: true. Ship the Sigstore TUF Rekor key as a pinned default. Have the CLI render an asserted quorum in yellow, not green.

F-06 — High — Distinct integers, one signed digest

What it is. serializeNumber (src/core/canonical.ts:100-113) is RFC 8785-conformant: it emits ECMAScript Number::toString. But the input has already lost information, because JSON.parse maps every number onto an IEEE-754 double. Above 2^53 that is not injective.

Why it matters. DAO governance is full of integers above 2^53: token amounts in wei (1 token = 10^18), vote weights, treasury transfers. Two treasury.transfer decisions differing by 1 wei — or by 1000 tokens, at the right magnitude — produce identical canonical bytes, an identical bodyHash, and one signature that covers both. That is a signature-forgery primitive: present record A for sealing, present record B in court, and every check in this library passes for both.

PoC.

JSON.parse('{"amountWei":1000000000000000000001}') -> {"amountWei":1e+21}
JSON.parse('{"amountWei":1000000000000000000000}') -> {"amountWei":1e+21}
COLLISION: true | same bodyHash: true

JSON.parse('{"votes":9007199254740993}') -> {"votes":9007199254740992}   (2^53+1 collides with 2^53)

test/core/canonical.test.ts:142-151 has a test named "distinct strings never share canonical bytes (injectivity)" — for strings only. :187-191 asserts number-value collisions as desirable. The module header at canonical.ts:2-11 and the test header at :5-8 both claim injectivity as a property the product depends on.

Fix. Canonicalisation cannot fix this — the loss happens before it. Reject at the schema boundary: validate every incoming GovernanceEvent.detail and decision.inputs and refuse any number outside Number.isSafeInteger range (canonicalize already refuses BigInt with exactly this guidance at :39-44). Require all on-chain quantities to be carried as decimal strings. Add CANONICAL_UNSAFE_INTEGER for Number.isInteger(n) && !Number.isSafeInteger(n), and soften the injectivity claim in both headers to "injective over the safe-integer domain".

F-07 — Medium — `__proto__` is a smuggling channel through the signed-body reconstruction

What it is. defaultSignedBody (src/verify/offline.ts:416-422) rebuilds the body with body[k] = v. For k === '__proto__' that invokes the Object.prototype setter and sets the prototype instead of creating a property, so Object.entries(body) never sees it and it is silently excluded from the canonical form.

Why it matters. An attacker can append a field literally named __proto__, carrying arbitrary content, to a genuinely signed record. The record still verifies with bodyHashMatches: true, signatureValid: true — yet canonicalize(record) does include the field, so any consumer that serialises or reads the "verified" record generically sees attacker data presented as part of verified evidence. It is also an unintended prototype write on a local object with attacker-controlled content, which is one refactor away from being worse.

PoC.

own keys include __proto__: true
with smuggled __proto__ field: SIGNED_PENDING | bodyHashMatches: true | sigValid: true
  -> canonicalize(record) DOES see it: true
  -> but the recomputed signed body silently DROPS it

canonicalize itself handles __proto__ correctly (Object.entries + serializeString emit it as an ordinary key, no pollution) — the bug is only in the reconstruction.

Fix. Use Object.create(null) for body, or Object.defineProperty(body, k, { value: v, enumerable: true }). Separately, reject any record whose own keys include __proto__, constructor or prototype with a dedicated code — there is no legitimate reason for a governance record to carry one. Add a regression test.

F-08 — Medium — Key selection is fragment-only; the DID is never bound

What it is. selectVerificationMethod (src/verify/offline.ts:541-542) falls back to methods.find((m) => fragment(m.id) === fragment(signedBy)), where fragment (:631-634) returns everything from # onward. The DID portion of record.signedBy is never compared to didDocument.id, and method.controller is never checked.

Why it matters. The DID document is caller-supplied, and that is inherent to offline verification — a verifier who does not pin the DID out of band cannot be helped. My PoC confirms the unavoidable baseline: a wholly forged record plus a matching attacker DID document verifies (SIGNED_PENDING, signatureValid: true). That is not a bug. The bug is that the code does not communicate the requirement and does not enforce the checks it can enforce. OfflineVerifyBundle.didDocument (:80) has no doc comment at all; OfflineVerifyOptions has no expectedDid; the module header (:1-32) discusses the anchor trust boundary at length and never mentions that the DID must be pinned. Meanwhile a caller who does pin the document correctly still gets a silent cross-DID match:

record says signedBy=did:web:witness.getvda.ai#key-1, doc id=did:web:attacker.example
  -> SIGNED_PENDING | sigValid: true

Fix. Add a required-by-default expectedDid option and fail INSUFFICIENT_PROOF / did_not_pinned when it is absent, so pinning is opt-out rather than opt-in. Enforce didDocument.id === didOf(record.signedBy) and method.controller === didDocument.id where present. Match on full id first and treat a fragment-only match as acceptable only when the DID prefixes already agree. Document the pinning requirement in the module header next to the anchor discussion.

F-09 — Medium — "Starts at genesis" is a `seq` check, not a genesis check

What it is. src/verify/offline.ts:457 rejects a chain whose earliest supplied seq !== 0 — correctly, and the comment explains it well. But nothing requires a seq: 0 record to have no prevHash, and the prevHash link is only checked from i === 1 onward (:488).

Why it matters. A truncated chain can be laundered into a "complete" one by minting a fresh seq: 0 record whose prevHash points at a body nobody will ever see. The verifier reports chainValid: true, which the reports read as continuity established back to the beginning.

PoC.

seq:0 record carrying a prevHash pointing nowhere: SIGNED_PENDING chainValid= true

(seq is inside the signed body, so this needs a signing key — trivially satisfied under F-08.)

Fix. In verifyChain, require ordered[0].seq === 0 && ordered[0].prevHash === undefined, and return BROKEN / genesis_record_has_prevHash otherwise — a genesis record with a predecessor pointer is an affirmative inconsistency, not missing proof.

F-10 — Medium — Exponential allocation from a ~100-byte OTS proof

What it is. applyOp for hexlify (src/anchor/ots.ts:857) returns TextEncoder().encode(bytesToHex(msg)) — exactly double the input length. The op is a single byte on the wire (0xf3), and the parser accepts chains up to MAX_TREE_DEPTH = 256 (:129). walk (:884-919) applies them with no size ceiling on the intermediate message.

Why it matters. Verifiers are precisely the parties who process .ots proof material handed to them by someone else. A DoS on the verifier is a DoS on the ability to challenge evidence.

PoC.

10 hexlify ops | input  86 bytes | msg grows to 32*2^n =        32,768 bytes |  70 ms
16 hexlify ops | input  92 bytes | msg grows to 32*2^n =     2,097,152 bytes |   8 ms
20 hexlify ops | input  96 bytes | msg grows to 32*2^n =    33,554,432 bytes | 587 ms
30 hexlify ops | input 106 bytes | msg would require    34,359,738,368 bytes

Fix. Add a MAX_MESSAGE_LENGTH (8 KB is generous — real Bitcoin proofs never exceed a few hundred bytes) and fail OTS_LIMIT_EXCEEDED in applyOp when hexlify, append or prepend would exceed it. Consider refusing hexlify entirely below the file-hash op, where it has no legitimate use.

F-11 — Medium — `toJSON` TOCTOU between the hashed bytes and the signed bytes

What it is. offlineVerify canonicalises the body twice: once for the digest comparison (src/verify/offline.ts:152) and again for the signature check (:218). canonicalize honours toJSON (src/core/canonical.ts:72-77). A value whose toJSON returns different data on successive calls therefore has its hash checked over one body and its signature checked over another. Separately, seen.delete(obj) at :75 before the toJSON recursion defeats cycle detection through toJSON, so a self-returning toJSON overflows the stack.

Why it matters. Unreachable through JSON — a parsed document cannot carry a function — but reachable for any in-process caller of the library API (a plugin, an adapter, a Proxy, an ORM row object with a lazy toJSON). It also breaks the documented invariant that every failure is a WitnessDaoError with a code.

PoC.

call 1: {"decision":{"verdict":"FAIL"}}
call 2: {"decision":{"verdict":"PASS"}}
SAME OBJECT, DIFFERENT SIGNED BYTES: true

self-returning toJSON -> threw: RangeError | code: (none) | is WitnessDaoError: false

Fix. Canonicalise once in offlineVerify and reuse the bytes for both the hash and the signature (also removes redundant work). In canonicalize, keep obj in seen across the toJSON recursion so a cycle raises CANONICAL_CIRCULAR. Consider restricting toJSON to Date instances (value instanceof Date) — that is the only case the comment at :72 actually cites, and honouring arbitrary toJSON on trust-critical bytes is a caller-controlled hook into the signed preimage.

F-12 — Medium — No fork / equivocation detection

What it is. verifyChain validates a single supplied path. Two records at the same seq with the same prevHash and contradictory content each verify cleanly and independently; nothing compares them.

Why it matters. A hash-chained log's headline property is that history cannot be rewritten undetectably. Here the detection is entirely up to the caller: whoever holds the key can maintain two divergent branches and present whichever suits, and this library will call each one chainValid: true. Combined with F-03, presenting both branches in one bundle also passes.

PoC.

fork branch B presented as the chain: SIGNED_PENDING chainValid= true
BOTH fork branches in one bundle    : SIGNED_PENDING (chainValid true, no reason)

Fix. Reject duplicate (chainKey, seq) pairs and duplicate prevHash values within a bundle with BROKEN / chain_fork_detected_at_seq_N. Document that single-bundle verification cannot detect a fork the caller was never shown, and that cross-bundle equivocation detection requires an external monitor — the same honesty the Rekor module already applies to log splitting (src/anchor/rekor.ts:65-66).

F-13 — Low — OTS varuint malleability, and a false round-trip claim

decodeVaruint (src/anchor/ots.ts:259-292) bounds the byte count and the value but does not reject overlong encodings.

00                 -> value 0, 1 byte
8000               -> value 0, 2 bytes
808000             -> value 0, 3 bytes
808080808080808000 -> value 0, 9 bytes

serializeTimestamp re-emits canonical lengths, so the documented invariant at :174-176 (serializeTimestamp(parseTimestamp(b)) === b) and the "byte-for-byte lossless" claim at :74-76 are both false:

overlong varbytes len  in = 000588960d73d71901810001
                       out= 000588960d73d719010101   | LOSSLESS: false
overlong header version                              | round-trip identical: false

Impact: multiple distinct .ots byte strings denote the same proof (dedup/identity confusion in mergeTimestamps), and upgradeOtsAttestation silently rewrites the stored proof bytes even when nothing was upgraded. Fix: reject any varuint whose final group is 0x00 while bytesRead > 1, and any where a shorter encoding exists; either restore the claim or delete it.

F-14 — Low — No leaf/node domain separation in `core/merkle.ts`

hashPair (src/core/merkle.ts:107-114) is SHA-256(left || right) with no prefix. The header at :14-16 claims CVE-2012-2459-class ambiguity is avoided by promoting odd nodes rather than duplicating them. Promotion does remove the duplicate-tail collision — I confirmed no root collisions across 18 leaf sets including tail-duplicated variants, and no cross-validating proofs across n=1..12. But the actual RFC 6962 remedy, domain separation, is absent, so a leaf digest and an internal node digest are indistinguishable:

root of [a, b, c]      = 6f3229c7…e450
root of [H(a||b), c]   = 6f3229c7…e450     STRUCTURAL COLLISION: true
1-leaf tree: root === leaf : true
verifyMerkleProof(a, [], a): true          <- empty proof trivially accepted

Weaponising this against real records needs a SHA-256 preimage, so severity is Low — but the doc overstates the protection, and verifyMerkleProof is a public API third parties are told to use with attacker-supplied proof. Fix: adopt RFC 6962's prefixes (0x00 leaf, 0x01 node) — rekor.ts:662-676 already implements exactly this — and reject an empty proof unless the caller explicitly opts into the single-leaf case. Then narrow the comment to what promotion actually buys.

F-15 — Low — `digestsEqual`'s constant-time claim is not accurate (over-claim)

src/core/hash.ts:50-57: "Constant-time comparison of two hex digests." It (a) returns early on length mismatch, (b) calls .toLowerCase() on both inputs first, and (c) compares 64-byte hex strings as UTF-8 rather than the 32 raw bytes. None of those is constant-time in the sense a reader would assume.

It does not matter operationally — every value it compares (bodyHash, prevHash, attestation.digest, computed digests) is public, and there is no secret for a timing side channel to leak. I report it because for a compliance product an overstated security property in a comment is itself a defect: it is the sort of line an expert witness will read aloud. Fix: reword to "length-checked, then timingSafeEqual over the normalised hex; all compared values are public digests, so the constant-time property is defence-in-depth rather than a requirement." If you want the claim to be true, hexToBytes both sides and compare 32 bytes.

F-16 — Low — DER accepts non-minimal INTEGER and OID encodings

The length handling is strict and correct — indefinite length, non-minimal long form, leading-zero long length, truncation, high-tag-number form, trailing bytes, over-long length octets and depth/node bombs are all rejected with specific codes (verified by PoC). But decodeInteger (:484-497) accepts leading 0x00 padding and decodeOid (:448-481) accepts overlong base-128 arcs:

INTEGER 00 00 05 (non-minimal) -> 5      (ACCEPTED)
OID     2a 80 01 (overlong arc) -> 1.2.1 (ACCEPTED, same as canonical 2a 01)

These are permissive rather than forgeable — a malformed certificate still yields not_checked, not a false pass. Impact is stored-proof encoding malleability. Fix: reject content.length > 1 && content[0] === 0x00 && (content[1] & 0x80) === 0 in decodeInteger, and a 0x80 first byte of any arc in decodeOid.

F-17 — Low — `evaluateQuorum` ignores the digest

evaluateQuorum (src/anchor/anchorer.ts:163-197) groups by kind and state and never looks at attestation.digest:

evaluateQuorum(DEFAULT_POLICY, [{kind:'rekor', digest:'totally-unrelated', state:'confirmed'}]).met -> true

MultiAnchorer.attemptAll does enforce the digest (:658-663), so the internal flow is safe — but evaluateQuorum is exported public API. Fix: take the expected digest as a parameter and ignore attestations that do not match it.

Everything else I probed here holds up: pending and failed never satisfy a requirement, duplicate same-kind attestations cannot double-count an anyOf group (the Set dedupes), and a vacuous policy is rejected by assertValidPolicy with ANCHOR_BAD_POLICY.

F-18 — Low — Unbounded branch count per OTS node

MAX_TREE_NODES = 20_000 is incremented in readTimestamp (:595), i.e. once per node. Attestation branches do not create nodes, so a single node's 0xff-separated branch list is unbounded:

3,600,011 input bytes -> 300,001 branches | 162 ms | 28 MB heap (node counter stayed at 1)

~8× amplification, linear — not severe, but it defeats the intent of the cap. Fix: count branches in the same counter, or add MAX_BRANCHES_PER_NODE.

F-19 — Low — Nonce not persisted, so offline re-verification cannot check the echo

buildTimeStampReq generates a nonce and parseTimeStampResp checks the echo when expectedNonce is supplied (:850-859) — good replay protection at request time. But anchorRfc3161 (:1366-1374) does not store the nonce on the attestation, and verifyRfc3161Attestation (:1416-1419) passes only expectedDigest, so nonceEchoed is permanently undefined on every re-verification. The header at :72 lists nonce echo among the things "verified locally, no trust required" without qualifying that it is verified once and never again. Fix: persist the nonce (hex) on the attestation and pass it through on re-verification.

F-20 — Low — `base64ToBytes` silently discards invalid input

src/core/hash.ts:40-44 relies on Buffer.from(s, 'base64'), which skips invalid characters rather than erroring: base64ToBytes('AAAA!!!!AAAA') → 000000000000. Signatures are length-checked afterwards (:82), which contains the immediate risk, but proof blobs elsewhere are not. Fix: validate against /^[A-Za-z0-9+/_-]*={0,2}$/ and confirm the round-trip length before returning.

F-21 — Low — Two accepted signed-message forms

src/verify/offline.ts:219-221 accepts a signature over the canonical body bytes or over the ASCII bodyHash. I looked for a cross-form confusion attack and did not find one: the two forms are effectively domain-separated (a 64-character lowercase-hex string versus canonical JSON, which always begins {), and both are bound to the same bodyHash that has already been recomputed and compared. So this is not currently exploitable. It is still a gratuitous doubling of the accepted message set for a deployment variant nobody in this repository uses. Fix: make it opt-in (acceptBodyHashSignature?: boolean, default false) and record in the result which form matched, so a report can state it.

F-22 / F-23 — Info

F-22. displayName is excluded from the signed body (:69) but consumed nowhere in src/report, src/pipeline, src/cli or src/mcp. An unsigned field with no consumer is pure attack surface waiting for a UI. Remove it from DEFAULT_ENVELOPE_FIELDS so it falls inside the signed body by default.

F-23. No Unicode normalisation is performed. This is correct per RFC 8785, which deliberately does not normalise, and it is the safe direction (distinct bytes → distinct digests, never the reverse). The residual risk is presentational: André (NFC) and André (NFD) render identically but hash differently, and Cyrillic homoglyphs let two decisions look identical in a rendered report while being cryptographically distinct. Worth one sentence in the report renderer's caveats, not a code change.

Also confirmed benign, matching JSON.stringify semantics: {a: undefined} ≡ {}, [1, undefined, 3] ≡ [1, null, 3].

What is genuinely well done

Not faint praise — several of these are things comparable codebases get wrong.

• RFC 6962 inclusion-proof recomputation is exactly right (src/anchor/rekor.ts:685-736). I implemented MTH/PATH independently from the RFC text and ran 820 positive vectors for n=1..40 with every leaf index, plus tampered-sibling, short-proof, long-proof and wrong-index negatives. Zero failures. The sn > 0 loop plus the i !== proofHashes.length check is a correct restatement of the RFC's "for each p in audit_path … then compare sn to 0", including the subtle inner right-shift loop. This is the single easiest place in the repository to be off by one, and it is not.
• DER length handling is strict and correct. Indefinite length, non-minimal long form, leading-zero long length, truncation, high-tag-number form, trailing bytes, implausible length-octet counts, depth and node caps: all rejected with distinct codes. Malformed certificates degrade to not_checked, never a false pass. The signedAttrs re-tagging from [0] IMPLICIT to SET (:1089) — the detail that most CMS verifiers botch — is right, and the comment says so.
• The lone-surrogate rejection is correct and the reasoning in the comment is exactly right (src/core/canonical.ts:138-149). TextEncoder would map \uD800, \uDC00 and U+FFFD to the same bytes; failing closed is the only defensible choice. Iterating by code point means astral characters are correctly not caught by the surrogate check — verified.
• UTF-16 code-unit key ordering is right, including for surrogate pairs, and the test suite contains the RFC 8785 §3.2.3 vector and an explicit "code-unit ordering is not code-point ordering" regression test. Verified independently.
• Sparse arrays. The comment at :60-63 documents a real bug that was found and fixed ([1,,3] once emitted invalid JSON that was nevertheless signed). That is the right instinct, written down for the next reader.
• The verified / asserted_unverified distinction inside each anchor module is honest and well argued. verifyOtsProof will not say verified without a block-header source; verifyRekorEntry refuses a key fetched from the log being verified as circular; parseTimeStampResp requires a chain validator. The reasoning is written out rather than assumed. The defect (F-05) is that the orchestrator above discards this distinction — the modules themselves get it right.
• anchorVerifier handling is defensive in exactly the right ways (src/verify/offline.ts:326, :611-621): a truthy non-boolean does not count, and a throwing verifier is never read as success. Both are tested.
• The BROKEN vs INSUFFICIENT_PROOF discrimination is careful and, outside F-03, correct. Truncated chain → INSUFFICIENT_PROOF / chain_does_not_start_at_genesis; seq gap → INSUFFICIENT_PROOF / chain_gap_between_…; prevHash mismatch → BROKEN. Shuffled input order verifies correctly (the sort is applied). The comment at :452-456 documents a previous over-claim that was found and fixed. This is the right taxonomy and it is mostly implemented.
• The strength matrix has load-bearing invariants asserted at module load (src/report/strength.ts:459-523): exactly one PROVEN cell, the whole BROKEN row and unbound column pinned to UNSUPPORTED, monotonicity along both axes, and agreement with BINDING_CONFIDENCE_RANK. A typo in the table fails at import rather than in a legal filing. Every cell carries its own written rationale.
• Key-valid-at-signing-time selection (:559-584) correctly handles created/revoked windows and refuses to guess when several keys exist and the record names none.
• Quorum hygiene: pending/failed never satisfy a requirement, a vacuous policy is rejected, duplicate targets are rejected, same-kind attestations cannot double-count an anyOf group, and both the immediate and permanence evaluations are always returned.
• The test suite is real. 753 tests, all passing, with genuine Ed25519 keys, genuine canonicalisation and genuine SHA-256 in the fixtures rather than mocked primitives; test/support/records.ts:9-13 deliberately rebuilds the signed body from an explicit field list instead of reusing DEFAULT_ENVELOPE_FIELDS, so a divergence between sealer and verifier breaks the tests instead of being tautologically hidden. That is a well-designed fixture. The reMint helper even names the key-holder-rewrites-history threat that F-03 defeats.

Over-claims

For a product sold as compliance evidence, a comment asserting a property the code does not deliver is as serious as a bug — it is what an expert witness will quote. Each of these should be either fixed or reworded.

Suggested remediation order

1. F-01 — make the report consume record.decision.inputs and reject any divergence from a supplied event. Nothing else matters until the signatures cover what the report prints.
2. F-02 — call verifyMerkleProof in anchorCoversRecord; add inclusionProof to the AnchorAttestation type.
3. F-03 — break on recordId, and reject duplicate (chainKey, seq).
4. F-04 / F-05 — gate compliance posture on the verifier's verdict, not a record field; split confirmed from asserted and make evaluateQuorum require verification.
5. F-06 — reject unsafe integers at the schema boundary; require on-chain quantities as strings.
6. F-07 – F-12 — the Medium set; each is a small, local, independently testable change.
7. Rewrite the twelve over-claims to match the code. Then add a regression test for every finding above — most of them are one assert away from being permanently closed, and several are properties the comments already claim but nothing exercises.