mvondracek / PA193_mnemonic_Slytherin

BIP39 Mnemonic Phrase Generator and Verifier
0 stars 2 forks source link

`mnemonic.verify` and `mnemonic._generate_seed` are missing mnemonic validation. #28

Closed mvondracek closed 4 years ago

mvondracek commented 4 years ago

Related to #19 and #22, branch task-11-entropy-mnemonic-transformations.

Function __is_valid_mnemonic was removed in 545d16133398337a5e3cf9f2315f23276f4c7c7d and 0d2b5559775a08cae203868a315752e6ebb10eed and other functions were changed (e. g. _mnemonic2entropy) to raise exceptions so we can use EAP instead od LBYL.

However, public function verify on this branch validates only seed. https://github.com/mvondracek/PA193_mnemonic_Slytherin/blob/dc7016679a936978672ff20f9a00a90e1210ebaf/PA193_mnemonic_Slytherin/mnemonic.py#L206-L218

mnemonic is passed and used in pbkdf2_hmac. https://github.com/mvondracek/PA193_mnemonic_Slytherin/blob/dc7016679a936978672ff20f9a00a90e1210ebaf/PA193_mnemonic_Slytherin/mnemonic.py#L28-L39

Applications and libraries should limit password to a sensible length (e.g. 1024). https://docs.python.org/3.7/library/hashlib.html#hashlib.pbkdf2_hmac

related: 9bb460701279d441ef33b64da88313507e1f51b5

What if our program is provided with malicious mnemonic and asked to do verification ('--verify')? Our program crashes due to OverflowError: password is too long. in pbkdf2_hmac during _generate_seed.

Added test in f0b267c4f5782ce7ce73fd010880c94edbb08514, which will be failing until the bug is fixed.

sobuch commented 4 years ago

this is just https://github.com/mvondracek/PA193_mnemonic_Slytherin/issues/26 again, we already knew this (specifically https://github.com/mvondracek/PA193_mnemonic_Slytherin/pull/19#discussion_r335154650) If we dont want to deal with the classes right now, we can just add call to _mnemonic2entropy in verify and it would be fine

mvondracek commented 4 years ago

Agree it would be solved by #26. Would you please implement Entropy, Seed, and Mnemonic classes which would perform validation upon instantiation?

sobuch commented 4 years ago

alright, I will record any progress in https://github.com/mvondracek/PA193_mnemonic_Slytherin/issues/26 and close this once its finished

sobuch commented 4 years ago

fixed in dev