From 3712724eae1ad1056e63465341d16080f49af0a7 Mon Sep 17 00:00:00 2001 From: Ajinkya Rajandekar Date: Mon, 12 Feb 2024 16:19:07 +0000 Subject: [PATCH 1/5] Remove unnecessary comments --- app/src/apdu.c | 2 -- app/src/apdu.h | 2 -- app/src/apdu_sign.c | 5 +---- app/src/globals.h | 2 -- app/src/keys.c | 2 -- app/src/keys.h | 2 -- app/src/ui_stream.h | 4 +--- app/src/ui_strings.c | 3 --- tests/integration/test_runtime.sh | 1 - 9 files changed, 2 insertions(+), 21 deletions(-) diff --git a/app/src/apdu.c b/app/src/apdu.c index 9b97fc4a..4715ede4 100644 --- a/app/src/apdu.c +++ b/app/src/apdu.c @@ -1,7 +1,5 @@ /* Tezos Ledger application - Some common primitives and some command handlers - TODO: split this file (apdu primitives and apdu handlers) - Copyright 2023 Nomadic Labs Copyright 2023 Trilitech diff --git a/app/src/apdu.h b/app/src/apdu.h index 5d6c8584..467567f7 100644 --- a/app/src/apdu.h +++ b/app/src/apdu.h @@ -1,7 +1,5 @@ /* Tezos Ledger application - Some common primitives and some command handlers - TODO: split this file (apdu primitives and apdu handlers) - Copyright 2023 Nomadic Labs With code excerpts from: diff --git a/app/src/apdu_sign.c b/app/src/apdu_sign.c index 7a4ebbe2..be7738ff 100644 --- a/app/src/apdu_sign.c +++ b/app/src/apdu_sign.c @@ -115,11 +115,10 @@ sign_packet(void) &global.path_with_curve.bip32_path, bufs[0].ptr, bufs[0].size, sig, &bufs[1].size)); - /* If we aren't returning the hash, zero its buffer... */ + /* If we aren't returning the hash, zero its buffer. */ if (!global.keys.apdu.sign.return_hash) { bufs[0].size = 0; } - io_send_response_buffers(bufs, 2, SW_OK); global.step = ST_IDLE; @@ -724,8 +723,6 @@ handle_data_apdu_blind(void) STRLCPY(hash, obuf); continue_blindsign_cb(); #endif - - /* XXXrcd: the logic here need analysis. */ TZ_POSTAMBLE; } #undef FINAL_HASH diff --git a/app/src/globals.h b/app/src/globals.h index b0510da8..dc38e1e6 100644 --- a/app/src/globals.h +++ b/app/src/globals.h @@ -118,5 +118,3 @@ extern const settings_t N_settings_real; extern unsigned int app_stack_canary; // From SDK extern unsigned char G_io_seproxyhal_spi_buffer[IO_SEPROXYHAL_BUFFER_SIZE_B]; - -// extern const uint32_t mainnet_chain_id = 0x7A06A770 // NetXdQprcVkpaWU diff --git a/app/src/keys.c b/app/src/keys.c index d5a7d507..6991a3b3 100644 --- a/app/src/keys.c +++ b/app/src/keys.c @@ -1,7 +1,5 @@ /* Tezos Ledger application - Signature primitives - TODO: cleanup/refactor - Copyright 2023 Nomadic Labs Copyright 2023 Functori diff --git a/app/src/keys.h b/app/src/keys.h index 84c8a27e..e5e587d6 100644 --- a/app/src/keys.h +++ b/app/src/keys.h @@ -1,7 +1,5 @@ /* Tezos Ledger application - Signature primitives - TODO: cleanup/refactor - Copyright 2023 Nomadic Labs With code excerpts from: diff --git a/app/src/ui_stream.h b/app/src/ui_stream.h index c320c26e..564a47f4 100644 --- a/app/src/ui_stream.h +++ b/app/src/ui_stream.h @@ -151,9 +151,7 @@ typedef struct { int16_t total; int16_t last; bool full; - // FIXME: workaround for issue with non-local control flow. Remove once - // fixed see !66 - bool pressed_right; + bool pressed_right; #ifdef HAVE_NBGL tz_ui_stream_display_t current_screen; char verify_address[TZ_BASE58CHECK_BUFFER_SIZE(20, 3)]; diff --git a/app/src/ui_strings.c b/app/src/ui_strings.c index 2ced007d..00bad70f 100644 --- a/app/src/ui_strings.c +++ b/app/src/ui_strings.c @@ -136,9 +136,6 @@ ui_strings_can_fit(size_t len, bool *can_fit) /* @param in: ptr to char[] to copy into the buffer @param in_len: number of of chars to copy. in_len <= strlen(in) @param out: will be set to the start of the char[] in the buffer - - // TODO: for future, when appending is a possibility - @param out_len: strlen(out) 0 < out_len <= in_len */ void ui_strings_push(const char *in, size_t len, char **out) diff --git a/tests/integration/test_runtime.sh b/tests/integration/test_runtime.sh index 81d2da67..025d2bf8 100644 --- a/tests/integration/test_runtime.sh +++ b/tests/integration/test_runtime.sh @@ -65,7 +65,6 @@ compare_strings() { STR2="$2" if [ "nanox" = "$TARGET" ]; then - # TODO: raise issue on speculos? STR1="$(echo $1 | sed 's/Parsing errorERR//g')" STR2="$(echo $2 | sed 's/Parsing errorERR//g')" fi From 3ba5d1a8538346173c664d7a389d13db1d50cb54 Mon Sep 17 00:00:00 2001 From: Ajinkya Rajandekar Date: Fri, 9 Feb 2024 16:29:18 +0000 Subject: [PATCH 2/5] Reset entire buffer not just its size --- app/src/apdu_sign.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/src/apdu_sign.c b/app/src/apdu_sign.c index be7738ff..4dabf90b 100644 --- a/app/src/apdu_sign.c +++ b/app/src/apdu_sign.c @@ -117,8 +117,10 @@ sign_packet(void) /* If we aren't returning the hash, zero its buffer. */ if (!global.keys.apdu.sign.return_hash) { + memset((void *)bufs[0].ptr, 0, bufs[0].size); bufs[0].size = 0; } + io_send_response_buffers(bufs, 2, SW_OK); global.step = ST_IDLE; From f3b8ef489c6e850c832e01d48ae299f3765ee635 Mon Sep 17 00:00:00 2001 From: Ajinkya Rajandekar Date: Mon, 12 Feb 2024 09:10:41 +0000 Subject: [PATCH 3/5] - Initialize pressed_right to 0. --- app/src/ui_stream.c | 11 ++++++----- app/src/ui_stream_nbgl.c | 14 ++++++-------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/app/src/ui_stream.c b/app/src/ui_stream.c index 00d7f5d2..f3a0f958 100644 --- a/app/src/ui_stream.c +++ b/app/src/ui_stream.c @@ -50,11 +50,12 @@ tz_ui_stream_init(void (*cb)(tz_ui_cb_type_t cb_type)) FUNC_ENTER(("cb=%p", cb)); memset(s, 0x0, sizeof(*s)); - s->cb = cb; - s->full = false; - s->current = 0; - s->total = -1; - s->last = 0; + s->cb = cb; + s->full = false; + s->current = 0; + s->pressed_right = false; + s->total = -1; + s->last = 0; ui_strings_init(); diff --git a/app/src/ui_stream_nbgl.c b/app/src/ui_stream_nbgl.c index 56015b81..6788a609 100644 --- a/app/src/ui_stream_nbgl.c +++ b/app/src/ui_stream_nbgl.c @@ -273,11 +273,12 @@ tz_ui_stream_init(void (*cb)(tz_ui_cb_type_t cb_type)) FUNC_ENTER(("cb=%p", cb)); memset(s, 0x0, sizeof(*s)); - s->cb = cb; - s->full = false; - s->last = 0; - s->current = -1; - s->total = -1; + s->cb = cb; + s->full = false; + s->last = 0; + s->current = -1; + s->total = -1; + s->pressed_right = false; ui_strings_init(); @@ -347,9 +348,6 @@ tz_ui_nav_cb(uint8_t page, nbgl_pageContent_t *content) s->pressed_right, s->current, s->total, s->full, global.step); if (global.step == ST_ERROR) { - // TODO: this is handled by change_screen_right except we disable it - // to use it here. We should make ui_stream fully compatible with - // exception.h global.step = ST_IDLE; ui_home_init(); result = false; From aa79b0670a359ade138aeb1f0fe41479b7625fb5 Mon Sep 17 00:00:00 2001 From: Ajinkya Rajandekar Date: Mon, 12 Feb 2024 13:28:21 +0000 Subject: [PATCH 4/5] Remove redundant global.step check --- app/src/apdu_sign.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/app/src/apdu_sign.c b/app/src/apdu_sign.c index 4dabf90b..501c1f21 100644 --- a/app/src/apdu_sign.c +++ b/app/src/apdu_sign.c @@ -488,15 +488,14 @@ handle_first_apdu_clear(__attribute__((unused)) command_t *cmd) #endif tz_ui_stream_init(stream_cb); global.step = ST_CLEAR_SIGN; - if (global.step == ST_CLEAR_SIGN) { + #ifdef TARGET_NANOS - tz_ui_stream_push(TZ_UI_STREAM_CB_NOCB, "Review operation", "", - TZ_UI_LAYOUT_HOME_PB, TZ_UI_ICON_EYE); -#elif defined(HAVE_BAGL) - tz_ui_stream_push(TZ_UI_STREAM_CB_NOCB, "Review", "operation", + tz_ui_stream_push(TZ_UI_STREAM_CB_NOCB, "Review operation", "", TZ_UI_LAYOUT_HOME_PB, TZ_UI_ICON_EYE); +#elif defined(HAVE_BAGL) + tz_ui_stream_push(TZ_UI_STREAM_CB_NOCB, "Review", "operation", + TZ_UI_LAYOUT_HOME_PB, TZ_UI_ICON_EYE); #endif - } #ifdef HAVE_SWAP } else { PRINTF("[DEBUG] If called from SWAP : global.step =%d\n", From 4da7ac47e81849e95e709b7f91a2cc9df9ab7ab2 Mon Sep 17 00:00:00 2001 From: Ajinkya Rajandekar Date: Mon, 12 Feb 2024 16:17:32 +0000 Subject: [PATCH 5/5] Fix unsigned int treated as int. --- app/src/parser/operation_parser.c | 4 ++-- app/src/parser/operation_state.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/src/parser/operation_parser.c b/app/src/parser/operation_parser.c index 8fbf2355..40bf2420 100644 --- a/app/src/parser/operation_parser.c +++ b/app/src/parser/operation_parser.c @@ -419,7 +419,7 @@ tz_step_size(tz_parser_state *state) } op->frame->step_size.size = (op->frame->step_size.size << 8) | b; op->frame->step_size.size_len--; - if (op->frame->step_size.size_len <= 0) { + if (op->frame->step_size.size_len == 0) { op->frame[-1].stop = state->ofs + op->frame->step_size.size; tz_must(pop_frame(state)); } @@ -590,7 +590,7 @@ tz_step_read_int32(tz_parser_state *state) ASSERT_STEP(state, READ_INT32); tz_operation_state *op = &state->operation; uint8_t b; - uint32_t *value = &op->frame->step_read_int32.value; + int32_t *value = &op->frame->step_read_int32.value; if (op->frame->step_read_int32.ofs < 4) { tz_must(tz_parser_read(state, &b)); *value = (*value << 8) | b; diff --git a/app/src/parser/operation_state.h b/app/src/parser/operation_state.h index a9ec5efa..eae58b6d 100644 --- a/app/src/parser/operation_state.h +++ b/app/src/parser/operation_state.h @@ -147,8 +147,8 @@ typedef struct { uint8_t skip : 1, natural : 1; } step_read_num; struct { - uint32_t value; - uint8_t skip : 1, ofs : 3; + int32_t value; + uint8_t skip : 1, ofs : 3; } step_read_int32; struct { uint16_t ofs;