merland / seedpicker

Create your own BIP39 seed phrase, securely and transparently.
http://seedpicker.net
MIT License
44 stars 20 forks source link

Trailing Space Breaks xpub/Zpub/XFP Calculation! #29

Closed mflaxman closed 3 years ago

mflaxman commented 3 years ago

This tool is smart about checking that each word is in the BIP39 wordlist, but then seems to allow spaces to affect its calculation.

I didn't test leading space, but I suspect it would have the same problem.

cc @merland since this one is potentially quite serious.

mflaxman commented 3 years ago

Screenshot of zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo (notice the trailing space) for reference.

Screenshot_2020-09-16 SeedPicker

This is the result it should get: https://github.com/merland/seedpicker/issues/23#issuecomment-683936740

merland commented 3 years ago

Nice catch, Indeed serious. I'll look into it asap. There are unit tests to prevent this from happening, but I suspect some logic has leaked to the presentation layer.

merland commented 3 years ago

Ok, should be fixed now. The raw phrase was mistakenly being used instead of the "cleaned" one, in one place. Thanks for the report @mflaxman !! I will do some more tests do make sure no more similar bugs exist.

mflaxman commented 3 years ago

Thanks @merland for fixing this so quickly! I'm doing my first (of many) podcast interviews this afternoon and am excited to start promoting seedpicker :)

merland commented 3 years ago

Wow, cool! Where can I listen to those interviews?

mflaxman commented 3 years ago

Just follow me on Twitter! IIRC you already do. Same handle as GitHub: "@mflaxman".