Skip to content

Fix unreachable error check in Mnemonic.to_entropy() - #146

Open
mapaba79 wants to merge 1 commit into
trezor:masterfrom
mapaba79:fix/to-entropy-unreachable-check
Open

mapaba79 wants to merge 1 commit into
trezor:masterfrom
mapaba79:fix/to-entropy-unreachable-check

Conversation

@mapaba79

Copy link
Copy Markdown

Problem

to_entropy() looks up each word's index with list.index():

ndx = self.wordlist.index(self.normalize_string(word))
if ndx < 0:
    raise LookupError('Unable to find "%s" in word list.' % word)

list.index() never returns a negative value — when the element is
not found, it raises ValueError directly instead of returning -1.
As a result, the if ndx < 0 branch is unreachable: it can never be
true, because if the word isn't in the wordlist the exception is
already raised two lines earlier, before the check even runs.

Impact

When an invalid/misspelled word is passed to to_entropy(), callers
get a raw ValueError with Python's generic list-lookup message
(e.g. 'notaword' is not in list) instead of the intended,
descriptive LookupError (Unable to find "notaword" in word list.). This is misleading for end users debugging a bad mnemonic,
and breaks any caller that specifically catches LookupError
(matching how the rest of this library reports lookup failures)
without also catching ValueError.

Reproduction

from mnemonic import Mnemonic
m = Mnemonic("english")
m.to_entropy("abandon " * 11 + "notaword")
# ValueError: 'notaword' is not in list

Fix

Wrap the .index() call in a try/except ValueError and raise the
intended LookupError from there, instead of checking a condition
that can never be true:

try:
    ndx = self.wordlist.index(self.normalize_string(word))
except ValueError:
    raise LookupError('Unable to find "%s" in word list.' % word)

Testing

  • Reproduced the original issue against a clean checkout: confirmed
    a raw ValueError was raised instead of LookupError.
  • Applied the fix and re-ran the same reproduction: confirmed
    LookupError: Unable to find "notaword" in word list. is now
    raised as intended.
  • Ran the full existing test suite (pytest tests/) before and
    after the change: all 7 tests pass in both cases, confirming no
    regression to existing checksum/entropy behavior.

This is a pure error-handling fix; the entropy/checksum computation
logic is unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant