fix: validate raw seed length alignment - #86
localhost41 wants to merge 1 commit into
Conversation
|
Thanks @localhost41 Added this on my watchlist, I'll review it asap |
nulllpc
left a comment
There was a problem hiding this comment.
The Uint8Array case here is a raw BIP-32 seed, not BIP-39 entropy - BIP-32 only requires 128-512 bits, with no multiple-of-32-bits constraint, so the added % 4 check rejects valid seeds (e.g. 17, 33, 63 bytes).
The constructor's JSDoc wasn't clear on this - it labeled both the mnemonic-string and raw-bytes cases as "BIP-39 seed phrase," which is likely what led to this mix-up
I opened a PR for that here:
tetherto/wdk-wallet#74
|
After some discussion, I realize that the check was correct to begin with, this issue incorrectly assumed the shape of the seed bytes, so I don't think we need to change at all. Sorry for wasting your time @localhost41 thanks for the effort! |
Fixes #84
Raw seed validation currently accepts any Uint8Array from 16 through 64 bytes, including lengths that cannot represent a whole number of 32-bit words. Require byte length to be a multiple of 4 while preserving the existing 16–64-byte bounds.
Verification: