ui/trinary_input_string: directly fetch bip39 words
What changed, and why it matters
This is a routine internal refactoring of how the on-screen keyboard looks up BIP39 seed words. It changes the code from carrying around actual word strings to carrying around word numbers and asking a helper function for the word when needed. There is no security bug being fixed here and no new attack path introduced by the change itself.
No security action required; review as normal refactoring if desired.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors trinary_input_string so that its wordlist member stores uint16_t BIP39 indices instead of const char* pointers. A new C helper, keystore_get_bip39_word_stack(), copies a BIP39 word into a caller-provided stack buffer and immediately frees the libwally-allocated string. The Rust Bip39Wordlist struct now holds Vec
Changed components
src/keystore.csrc/keystore.hsrc/rust/bitbox02/src/keystore.rssrc/ui/components/trinary_input_string.csrc/ui/components/trinary_input_string.hInspect captured patch +53 / −33
diff --git a/src/keystore.c b/src/keystore.c
index 83d206c..b170536 100644
--- a/src/keystore.c
+++ b/src/keystore.c
@@ -522,6 +522,17 @@ bool keystore_bip39_mnemonic_to_seed(const char* mnemonic, uint8_t* seed_out, si
return bip39_mnemonic_to_bytes(NULL, mnemonic, seed_out, 32, seed_len_out) == WALLY_OK;
}
+bool keystore_get_bip39_word_stack(uint16_t idx, char* word_out, size_t word_out_size)
+{
+ char* word_ptr;
+ if (bip39_get_word(NULL, idx, &word_ptr) != WALLY_OK) {
+ return false;
+ }
+ int snprintf_result = snprintf(word_out, word_out_size, "%s", word_ptr);
+ wally_free_string(word_ptr);
+ return snprintf_result >= 0 && snprintf_result < (int)word_out_size;
+}
+
bool keystore_get_bip39_word(uint16_t idx, char** word_out)
{
return bip39_get_word(NULL, idx, word_out) == WALLY_OK;
diff --git a/src/keystore.h b/src/keystore.h
index 2683389..f699116 100644
--- a/src/keystore.h
+++ b/src/keystore.h
@@ -153,6 +153,11 @@ USE_RESULT bool keystore_bip39_mnemonic_to_seed(
*/
USE_RESULT bool keystore_get_bip39_word(uint16_t idx, char** word_out);
+/**
+ * Retrieves the BIP39 word by index. `word_out` should be of at least 9 bytes long.
+ */
+USE_RESULT bool keystore_get_bip39_word_stack(uint16_t idx, char* word_out, size_t word_out_size);
+
/**
* Get a commitment to the original nonce before tweaking it with the host nonce. This is part of
* the ECDSA Anti-Klepto Protocol. For more details, check the docs of
diff --git a/src/rust/bitbox02/src/keystore.rs b/src/rust/bitbox02/src/keystore.rs
index 5acd561..02d8281 100644
--- a/src/rust/bitbox02/src/keystore.rs
+++ b/src/rust/bitbox02/src/keystore.rs
@@ -169,10 +169,10 @@ pub fn get_bip39_word(idx: u16) -> Result<zeroize::Zeroizing<String>, ()> {
}
/// An opaque C type which gives access to all BIP39 words.
-pub struct Bip39Wordlist(Vec<*const u8>);
+pub struct Bip39Wordlist(Vec<u16>);
impl Bip39Wordlist {
- pub fn as_ptr(&self) -> *const *const u8 {
+ pub fn as_ptr(&self) -> *const u16 {
self.0.as_ptr()
}
@@ -181,16 +181,6 @@ impl Bip39Wordlist {
}
}
-impl Drop for Bip39Wordlist {
- fn drop(&mut self) {
- for ptr in self.0.iter() {
- unsafe {
- bitbox02_sys::wally_free_string(*ptr as _);
- }
- }
- }
-}
-
/// If indices is None, all BIP39 English words are returned, otherwise only the words of the given
/// indices in the BIP39 English wordlist.
pub fn get_bip39_wordlist(indices: Option<&[u16]>) -> Bip39Wordlist {
@@ -198,18 +188,7 @@ pub fn get_bip39_wordlist(indices: Option<&[u16]>) -> Bip39Wordlist {
Some(indices) => indices.to_vec(),
None => (0..BIP39_WORDLIST_LEN).collect(),
};
- Bip39Wordlist(
- indices
- .into_iter()
- .map(|i| {
- let mut word_ptr: *mut u8 = core::ptr::null_mut();
- match unsafe { bitbox02_sys::keystore_get_bip39_word(i, &mut word_ptr) } {
- false => panic!("get_bip39_wordlist"),
- true => word_ptr as _,
- }
- })
- .collect(),
- )
+ Bip39Wordlist(indices)
}
pub struct SignResult {
diff --git a/src/ui/components/trinary_input_string.c b/src/ui/components/trinary_input_string.c
index c0a8117..d221610 100644
--- a/src/ui/components/trinary_input_string.c
+++ b/src/ui/components/trinary_input_string.c
@@ -21,6 +21,7 @@
#include "trinary_input_char.h"
#include <hardfault.h>
+#include <keystore.h>
#include <screen.h>
#include <touch/gestures.h>
#include <ui/event.h>
@@ -62,7 +63,7 @@ static const UG_FONT* _font = &font_password_11X12;
typedef struct {
// Can be NULL.
- const char* const* wordlist;
+ const uint16_t* wordlist;
size_t wordlist_size;
bool number_input;
// Only applies if wordlist != NULL: determines if a word from the wordlist was entered.
@@ -257,7 +258,10 @@ static void _maybe_autocomplete(component_t* trinary_input_string)
// initial value means no word was found yet.
size_t found_word_idx = data->wordlist_size;
for (size_t word_idx = 0; word_idx < data->wordlist_size; word_idx++) {
- const char* word = data->wordlist[word_idx];
+ char word[10];
+ if (!keystore_get_bip39_word_stack(data->wordlist[word_idx], word, sizeof(word))) {
+ Abort("keystore_get_bip39_word_stack");
+ }
bool is_prefix = strncmp(data->string, word, data->string_index) == 0;
if (is_prefix) {
if (found_word_idx != data->wordlist_size) {
@@ -266,9 +270,16 @@ static void _maybe_autocomplete(component_t* trinary_input_string)
}
found_word_idx = word_idx;
}
+
+ util_zero(word, sizeof(word));
+ }
+ char word[10];
+ if (!keystore_get_bip39_word_stack(data->wordlist[found_word_idx], word, sizeof(word))) {
+ Abort("keystore_get_bip39_word_stack");
}
- data->string_index =
- snprintf(data->string, sizeof(data->string), "%s", data->wordlist[found_word_idx]);
+
+ data->string_index = snprintf(data->string, sizeof(data->string), "%s", word);
+ util_zero(word, sizeof(word));
}
static void _set_alphabet(component_t* trinary_input_string)
@@ -282,7 +293,10 @@ static void _set_alphabet(component_t* trinary_input_string)
// The wordlist is assumed to be sorted and only have 'a-z' characters.
char charset[27] = {0};
for (size_t word_idx = 0; word_idx < data->wordlist_size; word_idx++) {
- const char* word = data->wordlist[word_idx];
+ char word[10];
+ if (!keystore_get_bip39_word_stack(data->wordlist[word_idx], word, sizeof(word))) {
+ Abort("keystore_get_bip39_word_stack");
+ }
bool is_prefix = strncmp(data->string, word, data->string_index) == 0;
if (is_prefix) {
if (strlen(word) > data->string_index) {
@@ -329,7 +343,12 @@ static void _set_can_confirm(component_t* trinary_input_string)
data->can_confirm = false;
// Can only confirm if the entered word matches a word in the wordlist.
for (size_t i = 0; i < data->wordlist_size; i++) {
- if (STREQ(data->wordlist[i], data->string)) {
+ char word[10];
+ if (!keystore_get_bip39_word_stack(data->wordlist[i], word, sizeof(word))) {
+ Abort("keystore_get_bip39_word_stack");
+ }
+
+ if (STREQ(word, data->string)) {
data->can_confirm = true;
return;
}
@@ -515,7 +534,12 @@ void trinary_input_string_set_input(component_t* trinary_input_string, const cha
return;
}
for (size_t i = 0; i < data->wordlist_size; i++) {
- if (STREQ(data->wordlist[i], word)) {
+ char bip39_word[10];
+ if (!keystore_get_bip39_word_stack(data->wordlist[i], bip39_word, sizeof(bip39_word))) {
+ Abort("keystore_get_bip39_word_stack");
+ }
+
+ if (STREQ(bip39_word, word)) {
data->string_index = snprintf(data->string, sizeof(data->string), "%s", word);
_set_alphabet(trinary_input_string);
_set_can_confirm(trinary_input_string);
diff --git a/src/ui/components/trinary_input_string.h b/src/ui/components/trinary_input_string.h
index c7bd7cf..64c2ba7 100644
--- a/src/ui/components/trinary_input_string.h
+++ b/src/ui/components/trinary_input_string.h
@@ -24,8 +24,9 @@
typedef struct {
const char* title;
- // Restrict and autocomplete to this list of words. Set to NULL to allow arbitrary input.
- const char* const* wordlist;
+ // Restrict and autocomplete to this list of BIP39 words. The elements are indices into the
+ // BIP39 English wordlist. Set to NULL to allow arbitrary input.
+ uint16_t const* wordlist;
// If true, the user can enter numbers only.
bool number_input;
// Set to 0 if wordlist is NULL.
Why this scored 15/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.