SFT-8171: bounds accounting in BIP39 prefix matching
What changed, and why it matters
This commit fixes a buffer-size accounting bug in the Passport hardware wallet's BIP39 word lookup feature. The function that returns matching seed-phrase words could write slightly past the end of its output buffer in some edge cases, and a negative match count could be misinterpreted as 'unlimited matches.' The patch corrects the size math, caps matches inside the loop, and rejects negative counts. The commit message says current callers are unaffected, and new unit tests verify the bounds.
Treat as a security-relevant hardening fix. Merge the patch, run the new unit tests, and consider whether any other language bindings cast user-supplied counts directly to unsigned types without validation.
Security signals we found
Off-by-one / separator accounting error in buffer filling
Signed-to-unsigned conversion of user-supplied count in language binding
Potential buffer over-write in BIP39/bytewords prefix helper
Addition of unit tests covering bounds and negative-count rejection
Evidence from the diff
In get_words_matching_prefix(), the original code counted only the length of each converted word but not the trailing comma separator, and it stopped matching after the fact rather than in the loop condition. This meant that for certain buffer sizes and match counts the separator could be written beyond the allocated buffer. The patch pre-checks total_written + len + 1 > matches_len, moves the max_matches cap into the for condition, and returns early when matches_len == 0. The MicroPython binding now rejects negative max_matches before casting to uint32_t, preventing a signed-to-unsigned wrap to a near-unlimited count. Unit tests exercise buffer limits, truncation, zero/negative counts, and the bytewords table.
Changed components
extmod/foundation/bip39_utils.cextmod/foundation/modfoundation-bip39.hBIP39 prefix matching bindingBytewords prefix matching bindingInspect captured patch +115 / −11
### extmod/foundation/bip39_utils.c
@@ -98,26 +98,32 @@ void get_words_matching_prefix(char* prefix,
uint32_t num_matches = 0;
uint32_t total_written = 0;
- for (uint32_t i = 0; i < num_words; i++) {
+ if (matches_len == 0) {
+ // Not even room for the terminator
+ return;
+ }
+
+ // Don't do more work than requested
+ for (uint32_t i = 0; i < num_words && num_matches < max_matches; i++) {
snprintf(candidate_keypad_digits, MAX_WORD_LEN + 1, "%"PRIu32, word_info[i].keypad_digits);
if (starts_with(candidate_keypad_digits, prefix)) {
- // This is a match, so convert the offsets to a real string and append to the buffer
- uint32_t len = word_info_to_string(candidate_keypad_digits, word_info[i].offsets, pnext_match);
- if (total_written + len > matches_len - 1) {
+ uint32_t len = strlen(candidate_keypad_digits);
+
+ // Room for the word plus the separator that follows it, which the terminator
+ // later overwrites, so this accounts for the terminator too
+ if (total_written + len + 1 > matches_len) {
// Don't write this one, as there is not enough room
break;
}
- total_written += len;
+
+ // This is a match, so convert the offsets to a real string and append to the buffer
+ word_info_to_string(candidate_keypad_digits, word_info[i].offsets, pnext_match);
+ total_written += len + 1;
pnext_match += len;
*pnext_match = ',';
pnext_match++;
num_matches++;
-
- // Don't do more work than requested
- if (num_matches == max_matches) {
- break;
- }
}
}
### extmod/foundation/modfoundation-bip39.h
@@ -28,7 +28,12 @@ STATIC mp_obj_t mod_foundation_bip39_get_words_matching_prefix(size_t n_args, co
mp_check_self(mp_obj_is_str_or_bytes(args[0]));
GET_STR_DATA_LEN(args[0], prefix_str, prefix_len);
- int max_matches = mp_obj_get_int(args[1]);
+ mp_int_t max_matches = mp_obj_get_int(args[1]);
+ if (max_matches < 0) {
+ // The count is unsigned in get_words_matching_prefix(), so a negative value
+ // would become an effectively unlimited one
+ mp_raise_ValueError(MP_ERROR_TEXT("max_matches must not be negative"));
+ }
// Must be "bip39" or "bytewords"
mp_check_self(mp_obj_is_str_or_bytes(args[2]));
### ports/stm32/boards/Passport/modules/tests/test_unit.py
@@ -40,6 +40,10 @@ def test_psbt_sighash(test):
assert test('psbt_sighash.py') == b'OK'
+def test_bip39_prefix_matching(test):
+ assert test('bip39_prefix_matching.py') == b'OK'
+
+
def test_psbt_multisig_approval(test):
assert test('psbt_multisig_approval.py') == b'OK'
### ports/stm32/boards/Passport/modules/tests/unit/bip39_prefix_matching.py
@@ -0,0 +1,89 @@
+# SPDX-FileCopyrightText: © 2026 Foundation Devices, Inc. <hello@foundation.xyz>
+# SPDX-License-Identifier: GPL-3.0-or-later
+#
+# The bounds accounting in get_words_matching_prefix() has to hold for any buffer
+# size and any requested match count. The binding hands it a 160 byte buffer, so
+# the longest string it can return is 159 characters.
+
+from foundation import bip39
+
+MATCHES_LEN = 160
+MAX_RESULT_LEN = MATCHES_LEN - 1
+
+# Longest BIP39 word, so the largest entry is MAX_WORD_LEN + 1 with its separator.
+MAX_WORD_LEN = 8
+
+KEYPAD_LETTERS = ('abc', 'def', 'ghi', 'jkl', 'mno', 'pqrs', 'tuv', 'wxyz')
+
+LETTER_TO_DIGIT = {}
+for _digit, _letters in enumerate(KEYPAD_LETTERS):
+ for _letter in _letters:
+ LETTER_TO_DIGIT[_letter] = str(_digit + 2)
+
+
+def to_digits(word):
+ return ''.join([LETTER_TO_DIGIT[letter] for letter in word])
+
+
+def must_reject(call):
+ try:
+ call()
+ except ValueError:
+ return
+
+ raise RuntimeError('expected ValueError')
+
+
+def matching(prefix, max_matches, word_list='bip39'):
+ '''Call the binding and check the invariants that hold for every result.'''
+
+ result = bip39.get_words_matching_prefix(prefix, max_matches, word_list)
+
+ assert len(result) <= MAX_RESULT_LEN, \
+ 'result is {} bytes, the buffer is {}'.format(len(result) + 1, MATCHES_LEN)
+
+ if result == '':
+ return []
+
+ words = result.split(',')
+ for word in words:
+ assert word != '', 'empty entry in {}'.format(result)
+ assert to_digits(word).startswith(prefix), \
+ '{} does not match prefix {}'.format(word, prefix)
+ return words
+
+
+# Ordinary predictive entry is unaffected.
+assert matching(to_digits('abandon'), 5) == ['abandon']
+assert 'cat' in matching(to_digits('cat'), 10)
+# Shorter words sort first, so an exact match leads the list it shares with longer ones.
+assert matching(to_digits('zoo'), 10)[0] == 'zoo'
+
+# A prefix no word can match yields an empty list.
+assert matching('999999999', 10) == []
+
+# A broad prefix has far more matches than fit, so the result is truncated.
+for prefix in ('2', '7', '22'):
+ truncated = matching(prefix, 2048)
+ # Truncation happens at the end of the buffer rather than well before it: the
+ # match that did not fit needs at most MAX_WORD_LEN + 1 bytes.
+ assert len(truncated) >= 25, \
+ 'only {} matches for prefix {}'.format(len(truncated), prefix)
+ assert MAX_RESULT_LEN - len(','.join(truncated)) <= MAX_WORD_LEN
+
+# max_matches still caps the result, and zero now means zero rather than unlimited.
+assert len(matching('2', 10)) == 10
+assert len(matching('2', 1)) == 1
+assert matching('2', 0) == []
+
+# A negative count would otherwise wrap to an effectively unlimited unsigned one.
+must_reject(lambda: bip39.get_words_matching_prefix('2', -1, 'bip39'))
+
+# Bytewords share the implementation and the same buffer.
+assert len(matching(to_digits('acid'), 10, word_list='bytewords')) >= 1
+assert len(matching('2', 256, word_list='bytewords')) >= 25
+
+# An unrecognised word list selects no table at all.
+assert bip39.get_words_matching_prefix('2', 10, 'not-a-word-list') is None
+
+return_value.write(b'OK')Why this scored 60/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.