fix(qr-link): remove legacy UserData with BLAKE3 domain separation flaw#1147
fix(qr-link): remove legacy UserData with BLAKE3 domain separation flaw#1147danielle-tfh merged 5 commits intomainfrom
Conversation
UserData and DataPolicy are superseded by AppAuthenticatedData from orb-relay-messages. They were exported but unused by any downstream crate (verified in both orb-software and priv-orb-core). The removed hasher_update() concatenated variable-length fields without length prefixes, allowing trivial hash collisions by shifting bytes between adjacent fields. The fix for the production code path (AppAuthenticatedData) will land in orb-relay-messages. Also removes now-unused blake3 and serde dependencies from orb-qr-link, and cleans up non-compiling doc examples that referenced removed types.
Add encode/decode support for QR v5, which uses the length-prefixed BLAKE3 hash from orb-relay-messages to prevent domain separation collisions. V4 (legacy) remains supported for backward compatibility. - decode_qr_with_version now accepts both '4' and '5' version prefixes - Add encode_static_qr_v5 for generating v5 QR codes - Update orb-relay-messages to rev with hash_with_length_prefix - Add tests for v5 roundtrip, rejection, and cross-version isolation
| /// Shared by v4 (legacy hash) and v5 (length-prefixed hash) — the payload | ||
| /// format is identical, only the hash function differs. |
There was a problem hiding this comment.
If v4 and v5 are identical, then why don't we just release the new lib with the length-prefixed hash and keep the version the same? If I understand correctly the length check is a bug right?
There was a problem hiding this comment.
They are not identical :( but thats a confusing comment
The length-prefix fix changes the hash output — hash() and hash_with_length_prefix() produce different bytes for the same AppAuthenticatedData. So if an app starts using the new hash but the orb only knows v4, verification silently fails. The version byte lets the orb distinguish which hash function was used and call the right verify method. Does that make sense, or do you think trying both hashes without a version bump would be better?
2 participants
Context
The
hasher_update()inUserDataconcatenated variable-length fields into BLAKE3 without length prefixes or separators, allowing hash collisions by shifting bytes between adjacent fields (Security Report #3612933).UserDatais superseded byAppAuthenticatedDatafromorb-relay-messages, which has the same flaw fixed in worldcoin/orb-relay-messages#98.Changes
UserData,DataPolicy, and theblake3/serdedependencies they requiredhash_with_length_prefix()methodQR v5
The version prefix in the QR code tells the orb which hash verification to use:
'4'AppAuthenticatedData::verify()(legacy)'5'AppAuthenticatedData::verify_with_length_prefix()The payload format (UUID + hash bytes) is the same for both versions.
Dependencies