Fixed issue with overflowing variable length array

Set the SSKR word buffer to a sensible size
aido · Nov 11, 2023 · 57d11bb · 57d11bb
1 parent 3a28df7
commit 57d11bb
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,16 @@
 # Change log
 
+## [1.5.2] - 2023-11-10
+### Added
+-
+
+### Changed
+-
+
+### Fixed
+- Save memory by setting the SSKR word buffer to a sensible size
+  - There is just enough memory available on Nano S to hold the phrases for 10 shares. Other devices can hold the full 16 shares.
+
 ## [1.5.1] - 2023-11-09
 ### Added
 - Added unit tests for shamir

diff --git a/Makefile b/Makefile
@@ -28,7 +28,7 @@ all: default
 APPNAME = "Seed Tool"
 APPVERSION_M = 1
 APPVERSION_N = 5
-APPVERSION_P = 1
+APPVERSION_P = 2
 APPVERSION   = "$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)"
 
 APP_LOAD_PARAMS = --appFlags 0x10 $(COMMON_LOAD_PARAMS) --curve secp256k1 --path ""

diff --git a/TODO.md b/TODO.md
@@ -3,7 +3,7 @@
 ### Todo
 
 - [ ] Update automated function tests to test on nanox and nanosp
-- [ ] Save memory by setting the SSKR word buffer (G_bolos_ux_context.sskr_words_buffer) to a sensible size. Maybe just store SSKR Bytewords as shorter two letter minimal Bytewords rather than a 4 letter Byteword plus spaace for each share. Convert minimal ByteWords back to four letter Bytewords just prior to display.
+- [ ] There is just enough memory available on Nano S to hold the phrases for 10 shares. Maybe just store SSKR Bytewords as shorter two letter minimal Bytewords rather than a 4 letter Byteword plus space for each share. Convert minimal ByteWords back to four letter Bytewords just prior to display.
 - [ ] If/when the `cx_bn_gf2_n_mul()` syscall is available on Ledger Nano S change all Galois Field functionality to use syscalls.
   - See [gf_syscalls](https://github.com/aido/app-seed-tool/tree/gf_syscalls) branch of repo.
 
@@ -18,6 +18,7 @@
 
 ### Done ✓
 
+- [x] Save memory by setting the SSKR word buffer (G_bolos_ux_context.sskr_words_buffer) to a sensible size
 - [x] Add unit tests
 - [x] Add code coverage to GitHub actions
 - [x] Add option to generate BIP39 mnemonics from SSKR shares even if shares do not validate against seed on device

diff --git a/src/bc-sskr/sskr.c b/src/bc-sskr/sskr.c
@@ -230,7 +230,7 @@ int sskr_generate(uint8_t group_threshold,
     }
 
     // Figure out how many shards we are dealing with
-    uint8_t total_shards = sskr_count_shards(group_threshold, groups, groups_len);
+    int total_shards = sskr_count_shards(group_threshold, groups, groups_len);
     if (total_shards < 0) {
         return total_shards;
     }
@@ -254,7 +254,7 @@ int sskr_generate(uint8_t group_threshold,
                                    master_secret,
                                    master_secret_len,
                                    shards,
-                                   total_shards,
+                                   (uint16_t) total_shards,
                                    random_generator);
 
     if (total_shards < 0) {

diff --git a/src/nano/ux_nano.h b/src/nano/ux_nano.h
@@ -85,11 +85,18 @@ typedef struct bolos_ux_context {
     // for CheckSeed app only
     uint8_t processing;
 
+#if defined(TARGET_NANOS)
+    // 10 shares * 229 chars per share (46 SSKR Bytewords)
+#define SSKR_WORDS_BUFFER_MAX_SIZE_B 2290
+#else
+    // 16 shares * 229 chars per share (46 SSKR Bytewords)
+#define SSKR_WORDS_BUFFER_MAX_SIZE_B 3664
+#endif
     uint8_t sskr_share_count;
     uint8_t sskr_share_index;
     unsigned int sskr_group_descriptor[1][2];
     unsigned int sskr_words_buffer_length;
-    char sskr_words_buffer[];
+    char sskr_words_buffer[SSKR_WORDS_BUFFER_MAX_SIZE_B];
 } bolos_ux_context_t;
 
 extern bolos_ux_context_t G_bolos_ux_context;

diff --git a/src/nano/ux_nano_sskr.c b/src/nano/ux_nano_sskr.c
@@ -131,6 +131,7 @@ void generate_sskr(void) {
                                    &G_bolos_ux_context.sskr_share_count,
                                    (unsigned char*) G_bolos_ux_context.sskr_words_buffer,
                                    &G_bolos_ux_context.sskr_words_buffer_length);
+
 #if defined(TARGET_NANOS)
     G_bolos_ux_context.processing = 0;
 #endif
@@ -152,23 +153,24 @@ void generate_sskr(void) {
     ux_flow_init(0, dynamic_flow, NULL);
 }
 
-const char* const sskr_descriptor_values[] = {
-    "1",
-    "2",
-    "3",
-    "4",
-    "5",
-    "6",
-    "7",
-    "8",
-    "9",
-    "10",
-    "11",
-    "12",
-    "13",
-    "14",
-    "15",
-    "16",
+const char* const sskr_descriptor_values[] = {"1",
+                                              "2",
+                                              "3",
+                                              "4",
+                                              "5",
+                                              "6",
+                                              "7",
+                                              "8",
+                                              "9",
+                                              "10",
+#ifndef TARGET_NANOS
+                                              "11",
+                                              "12",
+                                              "13",
+                                              "14",
+                                              "15",
+                                              "16"
+#endif
 };
 
 const char* sskr_threshold_getter(unsigned int idx) {

diff --git a/src/ux_common/onboarding_seed_sskr.c b/src/ux_common/onboarding_seed_sskr.c
@@ -51,7 +51,8 @@ unsigned int bolos_ux_sskr_hex_decode(unsigned char *mnemonic_hex,
             mnemonic_hex + (i * mnemonic_length / sskr_shares_count) + 4 + (sskr_share_len > 23);
     }
 
-    int output_len = sskr_combine(ptr_sskr_shares, sskr_share_len, sskr_shares_count, output, 32);
+    int output_len =
+        sskr_combine(ptr_sskr_shares, sskr_share_len, (uint8_t) sskr_shares_count, output, 32);
 
     if (output_len < 1) {
         memzero(mnemonic_hex, mnemonic_length);