fix: verify a creator signature against the bytes the creator signed - #596
Open
Zyrtnin wants to merge 1 commit into
Open
fix: verify a creator signature against the bytes the creator signed#596Zyrtnin wants to merge 1 commit into
Zyrtnin wants to merge 1 commit into
Conversation
verify_creator_signature re-derived the signed form by calling to_cbor_dict() on the DECODED metadata. Decoding is deliberately lossy: _cbor_str drops a wrong-typed field to "" rather than raising, because Photonic mints `loc` as an INTEGER on mainnet and refusing those tokens surfaced as "metadata: NONE". So the verifier compared the signature against bytes the creator never signed, and reported an HONEST, correctly-signed token as a forgery. Measured, with a verifying baseline and a matched control - the first attempt at this proof had a harness whose baseline ALSO failed, which would have attributed the mismatch to the wrong cause: in-memory, pyrxd-signed -> (True, '') TEXT loc, honestly signed -> (True, '') INTEGER loc, honestly signed -> (False, 'signature mismatch') <-- the bug `loc` is one instance; the defect is the class. ANY field the decoder normalises, now or the next time it learns a leniency, silently becomes a forgery verdict - so the failure gets WORSE as the decoder gets more forgiving, which is the opposite of how leniency should behave. Fixed at that level: the decoded object carries the exact bytes it was decoded from, and verification rebuilds the unsigned form from those, leaving every field with the type and value it had on chain. The re-encoded path remains for in-memory metadata, where the object IS the original. WHAT NOT TO TRADE IT FOR. A signature valid over on-chain bytes does not mean the creator signed what is being DISPLAYED. The old behaviour had that property by construction, at the cost of the false negative above, and dropping it silently would swap a visible defect for an invisible one. So when the decode normalised anything, the verdict says so explicitly rather than returning a bare "valid". Detected by comparing the two forms, not by tracking each field as it is normalised - a hand-kept list of lossy fields goes stale the first time the decoder learns a new leniency, which is how this class of bug arrives. source_cbor is compare=False: two tokens with identical fields are the same token regardless of which arrived over a wire, so existing equality assertions hold. REACHABILITY. verify_creator_signature has no caller inside pyrxd - it is exported public API, so its tests ARE its production entry point. Every case goes through decode_payload rather than building metadata by hand, which is the only way the defect is expressible at all. Verified by planting both halves: restoring the decoded-object derivation fails 8 of the new tests, and removing the lossy-decode disclosure fails 1. Every rescued case is paired with a forgery that must still be refused, including a forgery on the lossy path - the fix must not open a bypass for exactly the records it saved. 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
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.
verify_creator_signaturere-derived the signed form by callingto_cbor_dict()on the decoded metadata. Decoding is deliberately lossy —_cbor_strdrops a wrong-typed field to""rather than raising, because Photonic mintslocas an integer on mainnet and refusing those tokens surfaced as "metadata: NONE". So the verifier compared the signature against bytes the creator never signed, and reported an honest, correctly-signed token as a forgery.Measured, with a verifying baseline and a matched control:
(True, '')loc, honestly signed(True, '')loc, honestly signed(False, 'signature mismatch')Worth noting my first attempt at this proof had a harness whose baseline also failed — which would have attributed the mismatch to the wrong cause. A finding isn't isolated until the control passes.
The fix is at the class, not the instance
locis one field. Any field the decoder normalises — now, or the next time it learns a leniency — silently becomes a forgery verdict. The failure gets worse as the decoder gets more forgiving, which is the opposite of how leniency should behave.So the decoded object now carries the exact bytes it was decoded from, and verification rebuilds the unsigned form from those, leaving every field with the type and value it had on chain. The re-encoded path remains for in-memory metadata, where the object is the original.
What not to trade it for
A signature valid over on-chain bytes does not mean the creator signed what is being displayed. The old behaviour had that property by construction, at the cost of the false negative above — dropping it silently would swap a visible defect for an invisible one.
So when the decode normalised anything, the verdict says so explicitly rather than returning a bare "valid". Detected by comparing the two forms, not by tracking each field as it is normalised: a hand-kept list of lossy fields goes stale the first time the decoder learns a new leniency, which is how this class of bug arrives in the first place.
Reachability
verify_creator_signaturehas no caller inside pyrxd — it is exported public API, so its tests are its production entry point. Every case goes throughdecode_payloadrather than building metadata by hand, which is the only way the defect is expressible at all.Verification
Both halves planted: restoring the decoded-object derivation fails 8 of the new tests; removing the lossy-decode disclosure fails 1. Every rescued case is paired with a forgery that must still be refused — including a forgery on the lossy path, since the fix must not open a bypass for exactly the records it saved.
Full suite: 11,007 passed, 192 skipped, 1 xfailed.
🤖 Generated with Claude Code