From 57d11bbbb38d8c447606f55fdab79bb665e96f3b Mon Sep 17 00:00:00 2001 From: Aido Date: Fri, 10 Nov 2023 23:49:06 +0000 Subject: [PATCH] Fixed issue with overflowing variable length array Set the SSKR word buffer to a sensible size --- CHANGELOG.md | 11 +++++++++ Makefile | 2 +- TODO.md | 3 ++- src/bc-sskr/sskr.c | 4 ++-- src/nano/ux_nano.h | 9 ++++++- src/nano/ux_nano_sskr.c | 36 +++++++++++++++------------- src/ux_common/onboarding_seed_sskr.c | 3 ++- 7 files changed, 45 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d77994ee..9b2fd99e 100755 --- 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 index ec9dd032..764030a3 100755 --- 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 index 2b2cc319..c6e3f1de 100755 --- 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 index 34acca31..db6fc620 100644 --- 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 index ba06bdd6..15acfea5 100644 --- 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 index 01b68af5..b7ef14e1 100644 --- 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 index 9249d9c4..334aad7b 100644 --- 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);