fix: a missing curve withholds the verdict; it must not fail the decode - #615
Merged
Conversation
A security panel found a working exploit against the HashMark reader shipped
last week. A v2 record whose label contains a newline printed an
attacker-chosen line INSIDE the block the CLI had just marked "signature
VERIFIED", in the exact form of the WAVE-identity attribution the tool prints
only for a verified signature:
label: report.pdf
WAVE identity: treasury.rxd
signer: ecd7f194...
signature VERIFIED - recovers to the committed signer
Everything there is true except the second line, which the signer wrote. A
reader attributes the file to a key holder who never signed for that name. A
U+202E in the label additionally renders "invoice<RLO>gpj.exe" as
"invoiceexe.jpg", so the description shown is not the description signed.
The decoder applied only a length cap and utf-8 decode, so every one of these
decoded OK and attested VALID - where the reference implementation and any
spec-conformant verifier return INVALID. Two verifiers disagreeing about the
same bytes is the failure mode this project cares most about, and it is how we
published HTLC vectors that accepted the exploitable ordering.
The fix is at the decoder, not the renderer, so no caller can build a record
carrying a defective label. The version split is the spec's, and it is load
bearing: a v2 label is inside the signed statement, so a non-canonical one is
not the label that was signed and the record is INVALID; a v1 label is unsigned
and forms no part of any claim, so it is WITHHELD with a stated reason and the
record stays valid - invalidating it would discard timestamp evidence to fix a
rendering problem. Canonicalisation runs one way only: we never trim or
normalise a label we read, because the string shown must be the string signed.
Sanitising at the display boundary stays as the second layer. The msg decoder
already had it; the HashMark label did not, in the same renderer. That is the
whole defect.
Tests cover every row of the rejected table for both versions, the version
split, that ZWJ/ZWNJ and ordinary text are NOT refused, and the panel's exploit
end to end through the production classifier. Verified by planting: with the
5.4 check removed, 32 of them fail.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three findings from the same panel, in one commit because fixing the first makes the second load-bearing. 1. THE WALKER REFUSED OP_PUSHDATA. data_pushes_after_op_return rejected every opcode above 0x4B under a comment asserting those were "not a direct or OP_PUSHDATA data push". 0x4C and 0x4D ARE OP_PUSHDATA1/OP_PUSHDATA2, so the code contradicted its own comment and put a cliff at exactly 76 bytes - the length at which every encoder in this repo, and HashMark 4.1's own table, switches to OP_PUSHDATA1. Measured, through the production writer: OpReturn.lock emits OP_PUSHDATA1 above 75 bytes, so pyrxd WROTE msg outputs that pyrxd's own decoder called "not a message", at 76, 100 and 255 bytes - the last being the documented cap. Spec-legal signed HashMark records with a 76-88 byte label were skipped as NOT_HASHMARK, which section 6 defines as "skip silently, not an error", so a valid signature disappeared rather than being reported. The round trip is why the new tests go through the real writer. A hand-built fixture picks lengths under 76 without meaning to, which is how this shipped. 2. THE v2 LABEL CAP WAS THE RECORD CEILING. hashmark.py capped a v2 label at 223 bytes; 223 is the whole-RECORD ceiling and the label's share of it is maxLabelBytes(digestLength) = 88 for sha256 (5.4). It was unreachable only because of (1) - no label over 75 bytes could arrive at all - so fixing the walker without this would have turned a latent bug live in the same commit. Now derived from the spec's formula rather than tabulated, so registering a longer digest shrinks the label instead of silently producing records that stop relaying. Pinned by a test asserting one byte more does NOT fit, at four digest lengths. 3. WAVE NAMES REACHED THE TERMINAL RAW. A WAVE name is attacker-chosen registration text, and --verify-wave joined and printed it directly beneath "signature VERIFIED" - the one line stating an independently checked cryptographic fact - described to the user as proof "the signing key owns these names". A name carrying \x1b[2A\x1b[0J scrolls those lines away and reprints them saying something else. Same defect as the HashMark label, in the same renderer, one line apart: the label was fixed and this was left, while every other indexer-supplied string on the path was already sanitized. Fixed at the boundary, not the renderer, so the JSON path and any future consumer get the cleaned value without having to remember. MINIMALITY IS STRICT FOR HASHMARK AND LENIENT FOR msg. 4.1 requires rejecting non-minimal pushes so every record has exactly one serialization - two verifiers disagreeing about the same bytes is the failure this project cares most about. msg has no canonical form to protect, and refusing a third-party writer's honest text over its choice of length prefix would be a guard refusing valid work. That distinction is not theoretical: making the strict rules unconditional measurably WAS such a guard. Refusing OP_0 everywhere downgraded a msg whose payload is OP_0 from "malformed message, empty push" to "not a message at all", losing the diagnostic for a record that plainly is one. Caught by an existing test, and OP_0 now sits with the other minimality rules behind the flag. Verified by planting four defects, each caught: the original 0x4B bound (15 fail), the 223 label cap, the missing WAVE sanitization, and making HashMark lenient about minimality. Every refusal case is paired with an accept case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A computed verdict that no human sees is not a feature. The pasted-script view printed the HashMark digest, signer and attestation verdict. The txid view - the DEFAULT form, and the only one that reaches a record actually on chain - printed the type label alone. Everything else was computed and thrown away, so a v2 whose signature DOES NOT VERIFY and a genuine one rendered BYTE-IDENTICALLY, with the affirmative-sounding `op_return-hashmark-v2` label surviving and the one line that distinguishes them discarded. --json carried the verdict; the terminal did not. There is now ONE renderer for both surfaces rather than a second copy. Two copies is how one of them ends up missing the line that matters - which is exactly what happened in this same file, where the HashMark label was sanitized on one path and left raw on the other. --verify-wave had the mirror-image bug: it read payload["hashmark"], which exists only for a pasted script, so on a txid it attached nothing and never said why. A flag that silently does nothing on the form most people use it with. It now resolves every record in the transaction, not just the first. Two guards in the same area that were refusing valid work: * _MIN_SCRIPT_HEX_LEN was 46 hex, calibrated to P2SH, and runs BEFORE dispatch. An OP_RETURN data carrier can be 12. So a short, perfectly valid pasted `msg` was refused with "could not classify input" while the classifier behind the floor decoded it correctly - a working decoder behind a guard that would not call it. Only the pasted-script form was affected; --fetch has no floor. * verify_attestation did not enforce the 65-byte signature length that 6.3 step 3 makes part of VERIFYING rather than only of decoding. decode_hashmark enforces it, but this is public API and a caller verifying offline from stored fields reaches it directly: a 33-byte value sliced to an EMPTY s, which is int 0 - a wrong-but-typed answer rather than a refusal - and malformed hex escaped as an uncaught ValueError instead of one of the function's own outcomes. Verified by planting four defects, each caught: removing the payload lines from the txid view (6 fail), restoring the P2SH floor, removing the length check, and reading only the top-level wave record. Also replaced a test of mine that asserted the SOURCE TEXT of _attach_wave_identity. Splitting that function broke it while the behaviour it cared about was still correct; it now drives the function and checks the stored record. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two findings, both about the signed statement rather than the signature. 1. THE CHAIN WAS HARDCODED. 6.3 step 2 says to rebuild the statement with the genesis of the chain the transaction was ACTUALLY found on - the genesis is inside the signed statement, so the same bytes on another chain are a different statement and verify against a different key. Mainnet was hardcoded, so `pyrxd --network testnet glyph inspect` attested against mainnet and announced "assuming radiant-mainnet", an answer to a question the user had explicitly not asked. GENESIS_BLOCK_HASHES already had all three networks; nothing needed inventing. Mainnet stays the DEFAULT rather than becoming an error, because a pasted script genuinely carries no context and refusing to attest it would be worse than assuming and saying so. An unknown network falls back to mainnet AND REPORTS MAINNET: printing the requested name beside a mainnet genesis would state an assumption the code did not make, which is worse than the hardcoding it replaces - the reader could not tell the verdict was against a different chain. Threading the parameter turned up two CLI forwarders that would have silently kept the old behaviour. All five call sites are now audited as a set rather than one at a time, which is what a signature change requires. 2. THE CANONICAL FORM WAS PINNED BY NOTHING INDEPENDENT. Every signed fixture is built by canonical_statement() and verified through it, so a wrong escaping rule is applied identically at sign and at verify time and the round trip closes regardless. Measured: replacing the hand-rolled _json_string with json.dumps - which escapes non-ASCII to \uXXXX and therefore CHANGES THE BYTES A SIGNATURE COVERS for any label with an accent or an emoji - passed all 84 HashMark tests. The real mainnet record is a genuinely independent vector and is why key order is pinned, but it has no label, so the label branch - the only place non-ASCII can appear - was covered by no independent bytes at all. The expected statement is now TYPED OUT from the spec's field list rather than computed, which is the same second-implementer role done by hand because no second implementation of that branch was available to borrow bytes from. Same shape for the uncompressed-signer path: every fixture and the mainnet vector use header 0x1f, so the spec's 27..30 range was exercised nowhere and a verifier ignoring the flag looked correct. Now covered, with a companion test that the compressed and uncompressed hashes actually differ - if they ever coincided the test would pass while proving nothing. Verified by planting four mutations, each now caught: json.dumps for the escaper, always-compressed recovery, a swapped key pair in the statement, and re-hardcoding mainnet. The first two previously survived the entire suite. Worth recording that my first uncompressed harness failed and looked exactly like a verifier bug - I had signed a bare double-SHA of the statement rather than the Bitcoin-signed-message framing the code uses. The code was right. A test that re-derives everything at once localises nothing when it fails, so that one borrows the framing deliberately and pins only the flag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CLAUDE.md says a fix is a change and changes introduce defects. Re-attacking this branch's own work found three, two of them in tests I had just written and verified. 1. A TEST THAT PINNED NOTHING. `test_junk_is_still_refused` claimed to guard the lowered `_MIN_SCRIPT_HEX_LEN`, and every case in it was refused by a DIFFERENT check - odd-length hex, non-hex, empty, oversize. Setting the floor to 0 passed the whole class. It now includes valid even-length hex too short to be any script, which is the only input the floor itself refuses. Measured: the same plant now fails 2. This is the failure I have written two rules about this session and then committed anyway. Planting proves the ASSERTION is load-bearing; it does not prove the CASE can only fail for the reason you think. 2. STALE TEST DOUBLES ARE FORWARDERS. Adding `network=` to `classify_raw_tx` turned three unrelated sanitiser tests red: they monkeypatch it with a double that had the exact old signature, so the new keyword raised, glue caught it, and `ok` became False. The doubles exist to inject a payload, not to pin the call shape, so they take `**_kw` now - with the reason written down, because swallowing kwargs is a real trade and not obviously right. "Grep every forwarder when a signature changes" has to include the fakes. 3. THE BROWSER'S NETWORK WAS CORRECT BY ACCIDENT. The CLI now passes its `--network` to the attestation. The page reads ONE hard-coded mainnet ElectrumX endpoint, so the mainnet default was right - but only by an invisible default, and a network selector added later would silently keep attesting against mainnet. `_PAGE_NETWORK` is now passed explicitly and cross-referenced from the endpoint constant in inspect.js, so the coupling is visible at both ends. Also, the label table is now pinned against 5.4's FULL enumeration rather than one codepoint per row. The implementation stores ranges and the old tests sampled a member of each, so an off-by-one at a range edge passed both. Both directions and both edges: a range one short under-rejects, one too wide refuses honest text. Planted both, plus ZWJ moved into the rejected set - the spec exempts it as load-bearing in Devanagari and emoji sequences - and each is caught. Checked and found CORRECT, recorded so the next round does not re-derive them: `source_cbor` does not leak into `to_cbor_dict()` (the round trip is byte-identical) or into any generic serializer; the zero-length label push returning NOT_HASHMARK rather than INVALID is right, because 6.1 step 2 fails a non-minimal push BEFORE the magic is read; and the loosened floor does not let short input swallow a txid or an outpoint. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The WAVE line printed beside a verified mark read:
WAVE identity: company.rxd
(the signing key owns these names — file matches the
digest AND was recorded by that name's holder)
That is the unsound form verbatim, and both halves of it are wrong.
TENSE. "was recorded by that name's holder" is a PAST-tense claim manufactured
from a PRESENT-tense lookup. A name resolves to whatever it points at now; the
mark was made at a past block. The moment a name changes hands this is wrong in
both directions: a genuine mark signed by the previous holder starts failing
because the name resolves elsewhere, and — worse — whoever acquires a lapsed
name can make NEW marks that verify as "signed by whoever owns company.rxd",
which is TRUE and which a reader hears as "the company made this". Radiant names
have terms and expire, so acquiring one afterwards is ordinary rather than
exotic. The timestamp stays honest; the identity inference is not.
PLACEMENT. It sat inside the attestation block, between the signature and what
that signature proves, as though the mark carried it. 7.6 allows a name only as
present-tense context, "never beside the mark as though it were part of it".
Fixed as 7.6 form 1 — two facts, separately sourced:
* the signer ADDRESS is now shown with the signature, because it is the one
identity fact the mark really does carry, and it is the form a human can
compare against a wallet (recovered hash160 re-encoded, not new evidence);
* the names appear AFTER the mark's own statement closes, at the outer indent,
under "separately, and NOT part of the mark above", marked RIGHT NOW, and
saying explicitly that this does not establish who held the name when the mark
was made nor that the named party made it.
The JSON field was `names`, which invites exactly the inference the spec
forbids. It is `names_resolving_now` with `point_in_time: false` and a caveat
string, so a downstream consumer cannot reach for it believing it is the
historical answer.
Also corrected: the wave test module's own docstring asserted the bridge answers
"was this recorded by the holder of company.rxd?" — the question it cannot
answer. That framing is where the defect came from.
NOT done here: 7.6 form 2, point-in-time resolution against the mark's own
block by verifying the chain of modification transactions rather than trusting
an index. `WaveResolver.resolve()` goes through the indexer today and pyrxd has
no mod-history walker, so that is a feature with a design rather than a patch.
The spec's own instruction for the interim is precisely form 1.
Verified by planting all three violations: the past-tense phrasing, moving the
name back inside the mark, and renaming the field to a bare `names`.
Reported by the protocol author, who also found the 13.5 worked example carried
a signature reconstructed from a pre-broadcast approval screen rather than the
one that landed. Our vectors came from a 10,000-block chain scan, so they pin
the real 1f750d18... and never contained the fictional bytes; checked rather
than assumed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
HashMark records did not classify AT ALL in the browser. `verify_attestation` imports `recover_public_key` from `pyrxd.keys`, which imports `coincurve` at module top. The inspect page runs pyrxd under Pyodide and installs only micropip and pycryptodome, so that import RAISES there - straight out of the function. The per-output `try` in `_inspect_core` caught it and the row became `type=error`. Reproduced by installing a meta-path blocker for coincurve and calling `_inspect_script` on a real v2 record. A SECOND PATH DID THE SAME THING AND IT WAS MINE. The `--network` plumbing added last week reached for `network.registry`, and importing `pyrxd.network` pulls in `electrumx -> script.type -> keys -> coincurve` - in a module whose own docstring calls itself a network-free core. So fixing only the first import would have left the browser exactly as broken, which is why both are in one change. THE SPEC ALREADY SAID WHAT SHOULD HAPPEN. Section 6: "Decoding and attestation are SEPARATE steps with separate outcomes. Decoding needs only these bytes; verifying a v2 signature additionally needs secp256k1 ... which a decoder in a dependency-free library will not have. A record that decodes is well-formed, not yet believed." So the outcome is UNVERIFIABLE. The digest, label and signer still reach the reader; only the verdict is withheld, with its reason. Reporting INVALID_SIGNATURE would be far worse - it would tell a reader a genuine mark's claim does not hold, on the strength of a missing dependency. And it REACHES A HUMAN: the CLI prints "signature NOT CHECKED - <reason>" plus "(the record is well-formed; this is not a verdict on it)". Falling through silently would leave a v2 record showing a signer and no word about its signature, which reads as "fine" far more than it reads as "unchecked". GENESIS_BLOCK_HASHES moves to `constants.py`, the dependency-free bottom layer, with `network.registry` re-exporting it so there is still exactly one definition - asserted by identity in the tests, not by comparing values. Verified by planting both: making the import fatal again fails 3, and reaching for the network package from the offline core fails 4. A test also pins that the inspect core contains no `from ..network` import at all, because that is the regression, not the symptom. Found by an audit of the inspect concept doc, which noticed the browser could not do what the doc claimed. Verified by hand before acting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both conflicts are this branch's fix against main predating it. The `..constants` import is the whole point — reaching for `..network.registry` pulls in coincurve and breaks the browser, which is the defect being fixed. And the `unverifiable` render branch has no counterpart on main because the outcome did not exist there. Re-verified on the merged tree with coincurve blocked: a v2 record classifies as op_return-hashmark-v2 with attestation=unverifiable, rather than raising.
Zyrtnin
added a commit
that referenced
this pull request
Sep 4, 2026
… from it (#617) Stacked on #604. Eight claims in the inspect tool's own concept doc, all verified against the code on that branch — none had been fixed by parallel work. | # | claim | reality | |---|---|---| | 1 | The structural qualifier is "not optional and not suppressible" | `NOTES` has 10 exact keys; `p2pkh`, `unknown`, `error` and — because the lookup is exact — `op_return-msg` and both hashmark types get nothing. `_render_txid_human` emits no qualifier on **any** row | | 2 | "The OP_RETURN classifier does not interpret the payload" | It decodes the `msg` tag to UTF-8, decodes `HASHMARK`, and calls `verify_attestation` — secp256k1 recovery | | 3 | "SRI … catches a jsdelivr compromise before WASM ever runs" | One `integrity=`, on the loader. Everything the loader then fetches from that origin carries none | | 4 | The manifest check holds "even if the GitHub Pages deploy is compromised" | Every digest comes from an unhashed same-origin `manifest.json`, written in the same job as the wheels. It catches a **partial or corrupted** deploy; it cannot defend that origin against itself | | 5 | Three "source of truth" links | Point at `glyph_cmds.py`; the code is in `glyph_inspect.py` | | 6 | "it does not contact an indexer" | The page opens a WebSocket and sends `blockchain.transaction.get` | | 7 | Seven tx-shape banner triggers | Four wrong, and **six reveal-metadata banners were undocumented entirely** | | 8 | Two type lists | One omitted nine values; the other omitted `op_return-msg`, both hashmark types and `error` | On **3 and 4** it was asked to be precise in both directions — overstating a weakness is its own inaccuracy on a page people use to decide whether to trust the tool. It states what each mechanism *does* catch as well as what it doesn't. ## It found a real code defect `verify_attestation` needs `coincurve`, which Pyodide doesn't install — so a HashMark record **did not classify at all** in the browser. Proved with a meta-path blocker. That is fixed separately in #615, which also caught a second path of the same shape that I had introduced myself. Two smaller ones, reported not fixed: `refresh-pyodide.sh` writes to a path that doesn't exist, and a banner says "WAVE support in pyrxd is currently deferred" while `pyrxd.glyph.wave` ships and `--verify-wave` uses it. ## The guard is derived, not hand-kept `test_inspect_concept_doc_matches_the_code.py` (21 tests): repo-relative links and backticked symbols come from the document; the script `type` values are **AST-extracted** from `_inspect_core.py`, including the constant head of the two f-string assignments (`p2pkh-`, `op_return-hashmark-v`) that a literal-only scan would miss. Proved load-bearing without editing the tree — fed the pre-fix doc, it fails on four separate counts. And tested against instances it was **not** built from: pointed at a scratch classifier carrying two invented types, it extracted and failed on both. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Mudwood Labs <opensource@mudwoodlabs.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Stacked on #594. HashMark records did not classify at all in the browser.
verify_attestationimportsrecover_public_keyfrompyrxd.keys, which importscoincurveat module top. The inspect page runs pyrxd under Pyodide and installs onlymicropipandpycryptodome, so that import raises there — straight out of the function. The per-outputtryin_inspect_corecaught it and the row becametype=error. Reproduced with a meta-path blocker on a real v2 record.A second path did the same thing, and it was mine. The
--networkplumbing added in #594 reached fornetwork.registry, and importingpyrxd.networkpulls inelectrumx → script.type → keys → coincurve— in a module whose own docstring calls itself a network-free core. Fixing only the first import would have left the browser exactly as broken, which is why both are here.The spec already said what should happen
So the outcome is
UNVERIFIABLE. Digest, label and signer still reach the reader; only the verdict is withheld, with its reason:Reporting
INVALID_SIGNATUREwould be far worse — it would tell a reader a genuine mark's claim does not hold, on the strength of a missing dependency. And falling through silently would leave a v2 record showing a signer and no word about its signature, which reads as "fine" far more than "unchecked".GENESIS_BLOCK_HASHESmoves toconstants.py, the dependency-free bottom layer, withnetwork.registryre-exporting it — one definition, asserted by identity rather than by comparing values.Verification
Both planted: making the import fatal again fails 3; reaching for the network package from the offline core fails 4. A test also pins that the inspect core contains no
from ..networkimport at all — that is the regression, not the symptom.Found by an audit of the inspect concept doc, which noticed the browser could not do what the doc claimed. Verified by hand before acting.
Full suite: 11,134 passed, 192 skipped, 1 xfailed.
🤖 Generated with Claude Code