fix(eip712): harden typed-data signing correctness (odd-length hex + primaryType inference)#2
Open
Nexory wants to merge 1 commit into
Open
Conversation
Two correctness fixes in the EIP-712 signing path: - util/eip712.ts: hexToBytes silently truncated an odd-length hex string (dropping the last nibble), producing a wrong hash for `bytes` values. It now throws so signing fails loudly instead of signing the wrong data. - bridges/ethers.ts: when signTypedData is called without an explicit primaryType, the ethers bridge picked the first key in `types`. That signs the wrong struct when the root type is not declared first (a dependency listed above it). It now infers the root type (the struct not referenced by any other), matching ethers.js. Adds regression tests for both.
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.
What
Two correctness fixes in the EIP-712 typed-data signing path.
Why
1.
util/eip712.ts- an odd-length hex value is silently truncatedhexToBytesallocatesnew Uint8Array(clean.length / 2), which floors a fractional length. An odd-length hex string therefore drops its last nibble, andencodeValuefor abytesvalue then hashes the truncated data - the adapter signs the wrong content with no error.2.
bridges/ethers.ts- wrong primaryType when it is omittedWhen
signTypedDatais called without an explicitprimaryType, the bridge usedObject.keys(types).find(t => t !== "EIP712Domain"), i.e. the first key. EIP-712's primary type is the struct that is not referenced as a field type by any other struct (the root of the type graph), which is what ethers.js infers. When the root is not declared first (for example a dependency struct is listed above it), the bridge signs the wrong struct.Change
hexToBytesrejects an odd-length hex string instead of truncating it.primaryTypeis omitted.Tests
Adds one regression test per fix. Against the current code both fail:
With the fixes both pass and the full suite stays green: