mnemonic: gracefully handle invalid bcur-bip39 mnemonics, improve tests

diff --git a/main/bcur.c b/main/bcur.c
index 2235be4..434bfeb 100644
--- a/main/bcur.c
+++ b/main/bcur.c
@@ -3,6 +3,7 @@
 #include "jade_assert.h"
 #include "keychain.h"
 #include "qrcode.h"
+#include "qrmode.h"
 #include "qrscan.h"
 #include "ui.h"
 #include "utils/malloc_ext.h"
@@ -70,10 +71,8 @@ static const uint32_t QR_SCALE_FACTOR[] = { 0, 6, 5, 4, 4, 3, 3, 2, 2, 2, 2, 2,
 bool bcur_parse_bip39(
     const uint8_t* cbor, const size_t cbor_len, char* mnemonic, const size_t mnemonic_len, size_t* written)
 {
-    JADE_ASSERT(cbor);
-    JADE_ASSERT(cbor_len);
-    JADE_ASSERT(mnemonic);
-    JADE_ASSERT(mnemonic_len);
+    JADE_ASSERT(cbor && cbor_len);
+    JADE_ASSERT(mnemonic && mnemonic_len == MNEMONIC_BUFLEN);
     JADE_INIT_OUT_SIZE(written);
 
     // Parse cbor
@@ -99,9 +98,9 @@ bool bcur_parse_bip39(
     if (cberr != CborNoError || !cbor_value_is_valid(&mapItem) || !cbor_value_is_array(&mapItem)) {
         return false;
     }
-    size_t number_of_words = 0;
-    cberr = cbor_value_get_array_length(&mapItem, &number_of_words);
-    if (cberr != CborNoError || !number_of_words || !cbor_value_is_container(&mapItem)) {
+    size_t num_words = 0;
+    cberr = cbor_value_get_array_length(&mapItem, &num_words);
+    if (cberr != CborNoError || (num_words != 12 && num_words != 24) || !cbor_value_is_container(&mapItem)) {
         return false;
     }
     CborValue arrayItem;
@@ -110,23 +109,31 @@ bool bcur_parse_bip39(
         return false;
     }
     size_t write_pos = 0;
-    for (size_t i = 0; i < number_of_words; ++i) {
+    for (size_t i = 0; i < num_words; ++i) {
+        JADE_ASSERT(write_pos < MNEMONIC_BUFLEN - MNEMONIC_MAX_WORD_LEN - 1);
         if (write_pos) {
             // Add space separator
             mnemonic[write_pos++] = ' ';
         }
 
+        if (!cbor_value_is_text_string(&arrayItem)) {
+            return false; // Non-string in array
+        }
+
+        // Copy the next word
         CborValue next;
-        size_t tmp_len = mnemonic_len - write_pos;
+        size_t tmp_len = MNEMONIC_MAX_WORD_LEN + 1;
         cberr = cbor_value_copy_text_string(&arrayItem, mnemonic + write_pos, &tmp_len, &next);
-        JADE_ASSERT(cberr == CborNoError);
+        if (cberr != CborNoError || !tmp_len) {
+            return false;
+        }
         write_pos += tmp_len;
         arrayItem = next;
     }
-    JADE_ASSERT(cbor_value_at_end(&arrayItem));
+    if (!cbor_value_at_end(&arrayItem)) {
+        return false;
+    }
     cberr = cbor_value_leave_container(&mapItem, &arrayItem);
-    JADE_ASSERT(cberr == CborNoError);
-
     if (cberr != CborNoError || !cbor_value_is_valid(&mapItem) || !cbor_value_is_integer(&mapItem)) {
         return false;
     }
@@ -147,14 +154,17 @@ bool bcur_parse_bip39(
         return false;
     }
     cberr = cbor_value_advance(&mapItem);
-    JADE_ASSERT(cberr == CborNoError && cbor_value_at_end(&mapItem));
+    if (cberr != CborNoError || !cbor_value_at_end(&mapItem)) {
+        return false;
+    }
 
     cberr = cbor_value_leave_container(&value, &mapItem);
-    JADE_ASSERT(cberr == CborNoError);
+    if (cberr != CborNoError) {
+        return false;
+    }
 
     mnemonic[write_pos++] = '\0';
     *written = write_pos;
-
     return true;
 }
 
diff --git a/main/process/mnemonic.c b/main/process/mnemonic.c
index 8048c2e..cf3cef7 100644
--- a/main/process/mnemonic.c
+++ b/main/process/mnemonic.c
@@ -22,12 +22,6 @@
 #include <cdecoder.h>
 #include <ctype.h>
 
-// NOTE: Jade only supports the bip39 English wordlist
-
-// Should be large enough for all 12 and 24 word mnemonics
-#define MNEMONIC_MAXWORDS 24
-#define MNEMONIC_BUFLEN 256
-
 #define MAX_NUM_FINAL_WORDS 128
 #define NUM_WORDS_SELECT 10
 
@@ -989,8 +983,7 @@ static bool import_bcur_bip39(
     const uint8_t* bytes, const size_t bytes_len, char* buf, const size_t buf_len, size_t* written)
 {
     JADE_ASSERT(bytes);
-    JADE_ASSERT(buf);
-    JADE_ASSERT(buf_len);
+    JADE_ASSERT(buf && buf_len);
     JADE_INIT_OUT_SIZE(written);
 
     JADE_ASSERT(bytes[bytes_len] == '\0');
@@ -1099,7 +1092,7 @@ static bool import_compactseedqr(
 bool import_mnemonic(const uint8_t* bytes, const size_t bytes_len, char* buf, const size_t buf_len, size_t* written)
 {
     JADE_ASSERT(bytes);
-    JADE_ASSERT(buf);
+    JADE_ASSERT(buf && buf_len >= MNEMONIC_BUFLEN);
     JADE_INIT_OUT_SIZE(written);
 
     JADE_ASSERT(bytes[bytes_len] == '\0');
@@ -1110,7 +1103,7 @@ bool import_mnemonic(const uint8_t* bytes, const size_t bytes_len, char* buf, co
     // 4. Try to read word prefixes or whole words (space separated)
     return import_compactseedqr(bytes, bytes_len, buf, buf_len, written)
         || import_seedqr(bytes, bytes_len, buf, buf_len, written)
-        || import_bcur_bip39(bytes, bytes_len, buf, buf_len, written)
+        || import_bcur_bip39(bytes, bytes_len, buf, MNEMONIC_BUFLEN, written)
         || expand_words(bytes, bytes_len, buf, buf_len, written);
 }
 
@@ -1123,33 +1116,32 @@ bool import_and_validate_mnemonic(qr_data_t* qr_data)
     JADE_ASSERT(qr_data->len < sizeof(qr_data->data));
     JADE_ASSERT(qr_data->data[qr_data->len] == '\0');
 
-    char buf[sizeof(qr_data->data)];
-    SENSITIVE_PUSH(buf, sizeof(buf));
+    char mnemonic[sizeof(qr_data->data)];
+    SENSITIVE_PUSH(mnemonic, sizeof(mnemonic));
 
     // Try to import mnemonic, validate, and if all good copy over into the qr_data
     size_t written = 0;
-    if (import_mnemonic(qr_data->data, qr_data->len, buf, sizeof(buf), &written)
-        && bip39_mnemonic_validate(NULL, buf) == WALLY_OK) {
+    bool ret;
+    if (import_mnemonic(qr_data->data, qr_data->len, mnemonic, sizeof(mnemonic), &written)
+        && bip39_mnemonic_validate(NULL, mnemonic) == WALLY_OK) {
         JADE_ASSERT(written);
-        JADE_ASSERT(written <= sizeof(buf));
-        JADE_ASSERT(buf[written - 1] == '\0');
+        JADE_ASSERT(written <= sizeof(mnemonic));
+        JADE_ASSERT(mnemonic[written - 1] == '\0');
 
-        memcpy(qr_data->data, buf, written);
+        memcpy(qr_data->data, mnemonic, written);
         qr_data->len = written - 1; // Do not include nul-terminator
+        ret = true;
+    } else {
+        // Show the user that a valid qr was scanned, but the string data
+        // did not constitute (or expand to) a valid bip39 mnemonic string.
 
-        SENSITIVE_POP(buf);
-        return true;
+        const char* message[] = { "Invalid recovery phrase" };
+        await_error_activity(message, 1);
+        qr_data->len = 0;
+        ret = false;
     }
-
-    // Show the user that a valid qr was scanned, but the string data
-    // did not constitute (or expand to) a valid bip39 mnemonic string.
-    SENSITIVE_POP(buf);
-
-    const char* message[] = { "Invalid recovery phrase" };
-    await_error_activity(message, 1);
-    qr_data->len = 0;
-
-    return false;
+    SENSITIVE_POP(mnemonic);
+    return ret;
 }
 
 static bool mnemonic_qr(char* mnemonic, const size_t mnemonic_len)
diff --git a/main/qrmode.c b/main/qrmode.c
index e54de41..3d1ab37 100644
--- a/main/qrmode.c
+++ b/main/qrmode.c
@@ -28,8 +28,6 @@
 #include <string.h>
 #include <time.h>
 
-#define MNEMONIC_BUFLEN 256
-
 #define MAX_QR_V2_DATA_LEN 32
 #define MAX_QR_V4_DATA_LEN 78
 #define MAX_QR_V6_DATA_LEN 134
@@ -1265,17 +1263,16 @@ static bool handle_bip39_qr(const uint8_t* cbor, const size_t cbor_len)
     char mnemonic[MNEMONIC_BUFLEN];
     SENSITIVE_PUSH(mnemonic, sizeof(mnemonic));
     size_t written = 0;
+    bool ret = true;
     if (!bcur_parse_bip39(cbor, cbor_len, mnemonic, sizeof(mnemonic), &written) || written >= sizeof(mnemonic)
         || !handle_mnemonic_qr(mnemonic)) {
-        SENSITIVE_POP(mnemonic);
         JADE_LOGE("Processing scanned mnemonic data failed");
         const char* message[] = { "Failed loading wallet" };
         await_error_activity(message, 1);
-        return false;
+        ret = false;
     }
-
     SENSITIVE_POP(mnemonic);
-    return true;
+    return ret;
 }
 
 // Handle scanning a QR - supports addresses and PSBTs
diff --git a/main/qrmode.h b/main/qrmode.h
index 5ba15db..ead29a8 100644
--- a/main/qrmode.h
+++ b/main/qrmode.h
@@ -7,6 +7,17 @@
 
 #include "otpauth.h"
 
+// NOTE: Jade only supports the bip39 English wordlist,
+// with a 12 or 24 word mnemonic phrase.
+#define MNEMONIC_MAXWORDS 24
+
+// The longest valid words in the English wordlist are 8 characters.
+#define MNEMONIC_MAX_WORD_LEN 8
+
+// Size of a buffer for holding a mnemonic phrase.
+// 24 8-character words + 23 spaces + NUL = 216 bytes
+#define MNEMONIC_BUFLEN 216
+
 // Display singlesig xpub qr code
 void display_xpub_qr(void);
 
diff --git a/test_jade.py b/test_jade.py
index 7af6791..32356a8 100644
--- a/test_jade.py
+++ b/test_jade.py
@@ -219,6 +219,18 @@ kthskoihieiajpihktihiyjzhsjnihihiojzjlkoihaoidihjtrkkndede'
 TEST_MNEMONIC_BCUR_BIP39_UPPER = 'UR:CRYPTO-BIP39/OEADLKIYJKISINIHJZIEIHIOJPJL\
 KPJOIHIHJPJLIEIHIHHSKTHSJEIHIEJZJLIAJEIOJKHSKPJKHSIOIHIEIAHSJKISIHIOJZHSJPIHIE\
 KTHSKOIHIEIAJPIHKTIHIYJZHSJNIHIHIOJZJLKOIHAOIDIHJTRKKNDEDE'
+# bcur-bip39 mnemonic that has too many (32) words
+TEST_MNEMONIC_BCUR_BIP39_TOO_MANY = 'ur:crypto-bip39/oeadmkcxis\
+jyjljpjyjlinjkihis' * 31 + 'jyjljpjyjlinjkihaoidihjtsgfpvooe'
+# bcur-bip39 mnemonic that has too few (11) words
+TEST_MNEMONIC_BCUR_BIP39_TOO_FEW = 'ur:crypto-account/1-5/lpadahcshecygmzcbdca\
+guoeadluiohsidhsjtiejljtiohsidhsjtiejljtzsmeoelb'
+# bcur-bip39 mnemonic, the last word is abandoned which is 9 chars
+TEST_MNEMONIC_BCUR_BIP39_LONG_WORD = 'ur:crypto-account/1-5/lpadahcsincyrkgosr\
+cegooeadlkiohsidhsjtiejljtiohsidhsjtiejljtiohsintbwscw'
+TEST_MNEMONIC_BCUR_BIP39_EMPTY_WORD = 'ur:crypto-account/1-5/lpadahcshncysavwc\
+sdighoeadlkiohsidhsjtiejljtiohsidhsjtiejljtiocpcxtnnd'
+
 
 TEST_MNEMONIC_BCUR_BIP39_STRING = 'shield group erode awake lock sausage \
 cash glare wave crew flame glove'
@@ -2242,22 +2254,33 @@ def test_mnemonic_import(jade):
 
 
 def test_mnemonic_import_bad(jade):
-    # Check that mnemonic-prefixes are rejected if the prefixes match multiple words
-    # (but none of them exactly/full-match).  ie. prefix is ambiguous.  met -> metal, method
-    for i, bad_mnemonic in enumerate([TEST_MNEMONIC_PREFIXES_AMBIGUOUS,
-                                      TEST_MNEMONIC_SEEDSIGNER[:-1],  # bad length
-                                      TEST_MNEMONIC_SEEDSIGNER + '1234',  # bad length
-                                      TEST_MNEMONIC_SEEDSIGNER[:-4] + '2048',  # out of range
-                                      TEST_MNEMONIC_SEEDSIGNER[:-4] + '0000',  # invalid mnemonic
-                                      TEST_MNEMONIC_SEEDSIGNER_COMPACT[:-1],  # bad length
-                                      ]):
+    # Check importing invalid mnemonics
+    bad_mnemonics = [
+        # mnemonic phrase
+        TEST_MNEMONIC_PREFIXES_AMBIGUOUS,        # ambiguous prefixes
+        # seedsigner
+        TEST_MNEMONIC_SEEDSIGNER[:-1],           # bad length (too short)
+        TEST_MNEMONIC_SEEDSIGNER + '1234',       # bad length (too long)
+        TEST_MNEMONIC_SEEDSIGNER[:-4] + '2048',  # out of range
+        TEST_MNEMONIC_SEEDSIGNER[:-4] + '0000',  # invalid checksum word
+        TEST_MNEMONIC_SEEDSIGNER_COMPACT[:-1],   # bad length (compact case)
+        # bcur-bip39
+        TEST_MNEMONIC_BCUR_BIP39_TOO_MANY,       # too many words
+        TEST_MNEMONIC_BCUR_BIP39_TOO_FEW,        # too few words
+        TEST_MNEMONIC_BCUR_BIP39_LONG_WORD,      # word too long
+        TEST_MNEMONIC_BCUR_BIP39_EMPTY_WORD,     # empty word
+    ]
+    for i, bad_mnemonic in enumerate(bad_mnemonics):
         request = jade.build_request('badmnemonic_' + str(i), 'debug_set_mnemonic',
                                      {'mnemonic': bad_mnemonic})
         reply = jade.make_rpc_call(request)
         assert reply['id'] == request['id']
         assert 'result' not in reply
         assert reply['error']['code'] == JadeError.BAD_PARAMETERS
-        assert reply['error']['message'].startswith('Failed to expand mnemonic prefixes')
+        message = reply['error']['message']
+        expected = ['Failed to expand mnemonic prefixes',
+                    'Failed to extract mnemonic prefixes']
+        assert any(m in message for m in expected), message
 
 
 def test_passphrase(jade):