diff --git a/app/src/apdu_pubkey.c b/app/src/apdu_pubkey.c index d6383119f..1f0ce0d9f 100644 --- a/app/src/apdu_pubkey.c +++ b/app/src/apdu_pubkey.c @@ -34,7 +34,7 @@ static void provide_pubkey(void); static void prompt_address(void); -static void format_pkh(char *); +static void format_pkh(char *, size_t); static void stream_cb(tz_ui_cb_type_t); static void @@ -64,11 +64,11 @@ provide_pubkey(void) } static void -format_pkh(char *buffer) +format_pkh(char *buffer, size_t len) { cx_ecfp_public_key_t pubkey = {0}; uint8_t hash[21]; - TZ_PREAMBLE(("buffer=%p", buffer)); + TZ_PREAMBLE(("buffer=%p, len=%u", buffer, len)); TZ_CHECK(generate_public_key(&pubkey, global.path_with_curve.derivation_type, @@ -83,9 +83,11 @@ format_pkh(char *buffer) case DERIVATION_TYPE_BIP32_ED25519: hash[0] = 0; break; default: CX_CHECK(EXC_WRONG_PARAM); break; } - TZ_CHECK(tz_format_pkh(hash, 21, buffer)); // clang-format on + if (tz_format_pkh(hash, 21, buffer, len)) + TZ_FAIL(EXC_UNKNOWN); + TZ_POSTAMBLE; } @@ -114,7 +116,7 @@ prompt_address(void) global.step = ST_PROMPT; tz_ui_stream_init(stream_cb); - TZ_CHECK(format_pkh(buf)); + TZ_CHECK(format_pkh(buf, sizeof(buf))); tz_ui_stream_push_all(TZ_UI_STREAM_CB_NOCB, "Provide Key", buf, TZ_UI_ICON_NONE); tz_ui_stream_push_accept_reject(); @@ -157,7 +159,7 @@ verify_address(void) char buf[TZ_BASE58CHECK_BUFFER_SIZE(20, 3)]; TZ_PREAMBLE(("void")); - TZ_CHECK(format_pkh(buf)); + TZ_CHECK(format_pkh(buf, sizeof(buf))); nbgl_useCaseAddressConfirmation(buf, confirmation_callback); TZ_POSTAMBLE; } diff --git a/app/src/apdu_sign.c b/app/src/apdu_sign.c index e3c6d2b50..c44b96aed 100644 --- a/app/src/apdu_sign.c +++ b/app/src/apdu_sign.c @@ -453,7 +453,8 @@ handle_data_apdu_blind(packet_t *pkt) global.apdu.sign.received_last_msg = true; global.apdu.sign.step = SIGN_ST_WAIT_USER_INPUT; - tz_format_base58(FINAL_HASH, sizeof(FINAL_HASH), obuf); + if (tz_format_base58(FINAL_HASH, sizeof(FINAL_HASH), obuf, sizeof(obuf))) + TZ_FAIL(EXC_UNKNOWN); // clang-format off switch(global.apdu.sign.u.blind.tag) { diff --git a/app/src/parser/formatting.c b/app/src/parser/formatting.c index 530f4ccc0..59ecfcfcb 100644 --- a/app/src/parser/formatting.c +++ b/app/src/parser/formatting.c @@ -202,12 +202,18 @@ tz_michelson_op_name(uint8_t op_code) static const char tz_b58digits_ordered[] = "123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz"; -void -tz_format_base58(const uint8_t *n, size_t l, char *obuf) +int +tz_format_base58(const uint8_t *n, size_t l, char *obuf, size_t olen) { int carry; size_t i, j, high, zcount = 0, obuf_len = TZ_BASE58_BUFFER_SIZE(l); + if (olen < obuf_len) { + PRINTF("[DEBUG] tz_format_base58() called with %u obuf need %u\n", + olen, obuf_len); + return 1; + } + memset(obuf, 0, obuf_len); while (zcount < l && !n[zcount]) @@ -230,14 +236,21 @@ tz_format_base58(const uint8_t *n, size_t l, char *obuf) for (i = 0; j < obuf_len; ++i, ++j) obuf[i] = tz_b58digits_ordered[(unsigned)obuf[j]]; obuf[i] = '\0'; + return 0; } -void -tz_format_decimal(const uint8_t *n, size_t l, char *obuf) +int +tz_format_decimal(const uint8_t *n, size_t l, char *obuf, size_t olen) { int carry; size_t i, j, high, zcount = 0, obuf_len = TZ_DECIMAL_BUFFER_SIZE(l); + if (olen < obuf_len) { + PRINTF("[DEBUG] tz_format_base58() called with %u obuf need %u\n", + olen, obuf_len); + return 1; + } + memset(obuf, 0, obuf_len); while (zcount < l && !n[l - zcount - 1]) @@ -245,7 +258,7 @@ tz_format_decimal(const uint8_t *n, size_t l, char *obuf) if (zcount == l) { obuf[0] = '0'; - return; + return 0; } for (i = zcount, high = obuf_len - 1; i < l; ++i, high = j) { @@ -262,6 +275,7 @@ tz_format_decimal(const uint8_t *n, size_t l, char *obuf) for (i = 0; j < obuf_len; ++i, ++j) obuf[i] = '0' + obuf[j]; obuf[i] = '\0'; + return 0; } #ifndef ACTUALLY_ON_LEDGER @@ -365,7 +379,7 @@ find_prefix(const char *s, const uint8_t **p, size_t *pl, size_t dl) int tz_format_base58check(const char *sprefix, const uint8_t *data, size_t size, - char *obuf) + char *obuf, size_t olen) { const uint8_t *prefix = NULL; size_t prefix_len; @@ -375,8 +389,13 @@ tz_format_base58check(const char *sprefix, const uint8_t *data, size_t size, /* In order to avoid vla, we have a maximum buffer size of 64 */ uint8_t prepared[64]; - if (prefix_len + size + 4 > 64) + if (prefix_len + size + 4 > sizeof(prepared)) { + PRINTF( + "[WARNING] tz_format_base58check() failed: fixed size " + "array is too small need: %u\n", + prefix_len + size + 4); return 1; + } memcpy(prepared, prefix, prefix_len); memcpy(prepared + prefix_len, data, size); @@ -384,12 +403,11 @@ tz_format_base58check(const char *sprefix, const uint8_t *data, size_t size, cx_hash_sha256(prepared, size + prefix_len, tmp, 32); cx_hash_sha256(tmp, 32, tmp, 32); memcpy(prepared + size + prefix_len, tmp, 4); - tz_format_base58(prepared, prefix_len + size + 4, obuf); - return 0; + return tz_format_base58(prepared, prefix_len + size + 4, obuf, olen); } int -tz_format_pkh(const uint8_t *data, size_t size, char *obuf) +tz_format_pkh(const uint8_t *data, size_t size, char *obuf, size_t olen) { const char *prefix; @@ -405,11 +423,11 @@ tz_format_pkh(const uint8_t *data, size_t size, char *obuf) } // clang-format on - return tz_format_base58check(prefix, data + 1, size - 1, obuf); + return tz_format_base58check(prefix, data + 1, size - 1, obuf, olen); } int -tz_format_pk(const uint8_t *data, size_t size, char *obuf) +tz_format_pk(const uint8_t *data, size_t size, char *obuf, size_t olen) { const char *prefix; @@ -425,25 +443,25 @@ tz_format_pk(const uint8_t *data, size_t size, char *obuf) } // clang-format on - return tz_format_base58check(prefix, data + 1, size - 1, obuf); + return tz_format_base58check(prefix, data + 1, size - 1, obuf, olen); } /* Deprecated, use tz_format_base58check("o", ...) instead */ int -tz_format_oph(const uint8_t *data, size_t size, char *obuf) +tz_format_oph(const uint8_t *data, size_t size, char *obuf, size_t olen) { - return tz_format_base58check("o", data, size, obuf); + return tz_format_base58check("o", data, size, obuf, olen); } /* Deprecated, use tz_format_base58check("B", ...) instead */ int -tz_format_bh(const uint8_t *data, size_t size, char *obuf) +tz_format_bh(const uint8_t *data, size_t size, char *obuf, size_t olen) { - return tz_format_base58check("B", data, size, obuf); + return tz_format_base58check("B", data, size, obuf, olen); } int -tz_format_address(const uint8_t *data, size_t size, char *obuf) +tz_format_address(const uint8_t *data, size_t size, char *obuf, size_t olen) { const char *prefix; @@ -456,10 +474,10 @@ tz_format_address(const uint8_t *data, size_t size, char *obuf) case 3: prefix = "scr1"; break; case 4: prefix = "zkr1"; break; - case 0: return tz_format_pkh (data+1, size-1, obuf); + case 0: return tz_format_pkh(data+1, size-1, obuf, olen); default: return 1; } // clang-format on - return tz_format_base58check(prefix, data + 1, size - 2, obuf); + return tz_format_base58check(prefix, data + 1, size - 2, obuf, olen); } diff --git a/app/src/parser/formatting.h b/app/src/parser/formatting.h index 80162da73..478209064 100644 --- a/app/src/parser/formatting.h +++ b/app/src/parser/formatting.h @@ -192,13 +192,13 @@ typedef enum { // stored in little-endian order in the first `l` bytes of `n`. The // output buffer `obuf` must be at least `DECIMAL_BUFFER_SIZE(l)` // (caller responsibility). -void tz_format_decimal(const uint8_t *, size_t, char *); +int tz_format_decimal(const uint8_t *, size_t, char *, size_t); #define TZ_DECIMAL_BUFFER_SIZE(_l) ((_l)*241 / 100 + 1) // Formats a data `n` of size `l` in base58 using Tezos' alphabet // order (same as Bitcoin). The output buffer `obuf` must be at least // `BASE58_BUFFER_SIZE(l)` (caller responsibility). -void tz_format_base58(const uint8_t *, size_t, char *); +int tz_format_base58(const uint8_t *, size_t, char *, size_t); #define TZ_BASE58_BUFFER_SIZE(_l) ((_l)*138 / 100 + 1) // Looks up the prefix from the provided string (arg1), e.g. "B", @@ -209,7 +209,8 @@ void tz_format_base58(const uint8_t *, size_t, char *); // double-sha256 of this concatenation, and call `format_base58`. The // output buffer `obuf` must be at least `BASE58CHECK_BUFFER_SIZE(l, // prefix_len)` (caller responsibility). -int tz_format_base58check(const char *, const uint8_t *, size_t, char *); +int tz_format_base58check(const char *, const uint8_t *, size_t, char *, + size_t); #define TZ_BASE58CHECK_BUFFER_SIZE(_l, _p) \ TZ_BASE58_BUFFER_SIZE((_p) + (_l) + 4) @@ -224,13 +225,13 @@ int tz_format_base58check(const char *, const uint8_t *, size_t, char *); // tag 1: tz2(36) // tag 2: tz3(36) // tag 3: tz4(36) -int tz_format_pkh(const uint8_t *, size_t, char *); +int tz_format_pkh(const uint8_t *, size_t, char *, size_t); // size 32, o(51) -int tz_format_oph(const uint8_t *, size_t, char *); +int tz_format_oph(const uint8_t *, size_t, char *, size_t); // size 32, B(51) -int tz_format_bh(const uint8_t *, size_t, char *); +int tz_format_bh(const uint8_t *, size_t, char *, size_t); // size 22: tag(1) + data(21) // tag 0: tag(1) + pkh(20) (tz1, tz2, tz3, tz4, see format_pkh) @@ -238,11 +239,11 @@ int tz_format_bh(const uint8_t *, size_t, char *); // tag 2: txrolluph(20) + padding(1), txr1(36) // tag 3: rolluph(20) + padding(1), scr1(36) // tag 4: zkrolluph(20) + padding(1), zkr1(36) -int tz_format_address(const uint8_t *, size_t, char *); +int tz_format_address(const uint8_t *, size_t, char *, size_t); // size 33/34/49: tag(1) + data(32/33/48) // tag 0: pk(32), edpk(54) // tag 1: pk(33), sppk(55) // tag 2: pk(33), p2pk(55) // tag 3: pk(48), BLpk(76) -int tz_format_pk(const uint8_t *, size_t, char *); +int tz_format_pk(const uint8_t *, size_t, char *, size_t); diff --git a/app/src/parser/num_parser.c b/app/src/parser/num_parser.c index b3cf6427a..071d65656 100644 --- a/app/src/parser/num_parser.c +++ b/app/src/parser/num_parser.c @@ -57,7 +57,7 @@ tz_parse_num_step(tz_num_parser_buffer *buffers, tz_num_parser_regs *regs, if (!cont) { regs->stop = true; tz_format_decimal(buffers->bytes, (regs->size + 7) / 8, - buffers->decimal); + buffers->decimal, sizeof(buffers->decimal)); } return TZ_CONTINUE; } diff --git a/app/src/parser/operation_parser.c b/app/src/parser/operation_parser.c index 6751594c5..626fc852e 100644 --- a/app/src/parser/operation_parser.c +++ b/app/src/parser/operation_parser.c @@ -303,6 +303,18 @@ tz_operation_parser_init(tz_parser_state *state, uint16_t size, } } +/* + * We use a macro for CAPTURE rather than defining a ptr like: + * + * uint8_t *capture = state->buffers.capture + * + * because sizeof(*capture) == 1, whereas sizeof(CAPTURE) is + * the size of the buffer. This allows us to more idiomatically + * check the size of buffers. + */ + +#define CAPTURE (state->buffers.capture) + static tz_parser_result tz_print_string(tz_parser_state *state) { @@ -313,7 +325,7 @@ tz_print_string(tz_parser_state *state) tz_continue; } op->frame->step = TZ_OPERATION_STEP_PRINT; - op->frame->step_print.str = (char *)state->buffers.capture; + op->frame->step_print.str = (char *)CAPTURE; tz_continue; } @@ -322,7 +334,6 @@ tz_operation_parser_step(tz_parser_state *state) { tz_operation_state *op = &state->operation; tz_parser_regs *regs = &state->regs; - uint8_t *capture = state->buffers.capture; // cannot restart after error if (TZ_IS_ERR(state->errno)) @@ -495,14 +506,14 @@ tz_operation_parser_step(tz_parser_state *state) tz_must(tz_parser_read(state, &b)); value = value << 8 | b; } - snprintf((char *)capture, 30, "%d", value); + snprintf((char *)CAPTURE, sizeof(CAPTURE), "%d", value); tz_must(tz_print_string(state)); break; } case TZ_OPERATION_STEP_READ_BYTES: { if (op->frame->step_read_bytes.ofs < op->frame->step_read_bytes.len) { uint8_t *c; - c = &capture[op->frame->step_read_bytes.ofs]; + c = &CAPTURE[op->frame->step_read_bytes.ofs]; tz_must(tz_parser_read(state, c)); op->frame->step_read_bytes.ofs++; } else { @@ -512,50 +523,56 @@ tz_operation_parser_step(tz_parser_state *state) } switch (op->frame->step_read_bytes.kind) { case TZ_OPERATION_FIELD_SOURCE: - memcpy(op->source, capture, 22); + memcpy(op->source, CAPTURE, 22); __attribute__((fallthrough)); case TZ_OPERATION_FIELD_PKH: - if (tz_format_pkh(capture, 21, (char *)capture)) + if (tz_format_pkh(CAPTURE, 21, (char *)CAPTURE, + sizeof(CAPTURE))) tz_raise(INVALID_TAG); break; case TZ_OPERATION_FIELD_PK: - if (tz_format_pk(capture, op->frame->step_read_bytes.len, - (char *)capture)) + if (tz_format_pk(CAPTURE, op->frame->step_read_bytes.len, + (char *)CAPTURE, sizeof(CAPTURE))) tz_raise(INVALID_TAG); break; case TZ_OPERATION_FIELD_SR: - if (tz_format_base58check("sr1", capture, 20, - (char *)capture)) + if (tz_format_base58check("sr1", CAPTURE, 20, (char *)CAPTURE, + sizeof(CAPTURE))) tz_raise(INVALID_TAG); break; case TZ_OPERATION_FIELD_SRC: - if (tz_format_base58check("src1", capture, 32, - (char *)capture)) + if (tz_format_base58check("src1", CAPTURE, 32, + (char *)CAPTURE, + sizeof(CAPTURE))) tz_raise(INVALID_TAG); break; case TZ_OPERATION_FIELD_PROTO: - if (tz_format_base58check("proto", capture, 32, - (char *)capture)) + if (tz_format_base58check("proto", CAPTURE, 32, + (char *)CAPTURE, + sizeof(CAPTURE))) tz_raise(INVALID_TAG); break; case TZ_OPERATION_FIELD_DESTINATION: - memcpy(op->destination, capture, 22); - if (tz_format_address(capture, 22, (char *)capture)) + memcpy(op->destination, CAPTURE, 22); + if (tz_format_address(CAPTURE, 22, (char *)CAPTURE, + sizeof(CAPTURE))) tz_raise(INVALID_TAG); break; case TZ_OPERATION_FIELD_OPH: - if (tz_format_oph(capture, 32, (char *)capture)) + if (tz_format_oph(CAPTURE, 32, (char *)CAPTURE, + sizeof(CAPTURE))) tz_raise(INVALID_TAG); break; case TZ_OPERATION_FIELD_BH: - if (tz_format_bh(capture, 32, (char *)capture)) + if (tz_format_bh(CAPTURE, 32, (char *)CAPTURE, + sizeof(CAPTURE))) tz_raise(INVALID_TAG); break; default: tz_raise(INVALID_STATE); } op->frame->step = TZ_OPERATION_STEP_PRINT; - op->frame->step_print.str = (char *)capture; + op->frame->step_print.str = (char *)CAPTURE; } break; } @@ -577,32 +594,32 @@ tz_operation_parser_step(tz_parser_state *state) } case TZ_OPERATION_STEP_READ_STRING: { if (state->ofs == op->frame->stop) { - capture[op->frame->step_read_string.ofs] = 0; + CAPTURE[op->frame->step_read_string.ofs] = 0; tz_must(tz_print_string(state)); } else { uint8_t b; tz_must(tz_parser_read(state, &b)); - capture[op->frame->step_read_string.ofs] = b; + CAPTURE[op->frame->step_read_string.ofs] = b; op->frame->step_read_string.ofs++; } break; } case TZ_OPERATION_STEP_READ_BINARY: { if (state->ofs == op->frame->stop) { - capture[op->frame->step_read_string.ofs] = 0; + CAPTURE[op->frame->step_read_string.ofs] = 0; tz_must(tz_print_string(state)); } else if (op->frame->step_read_string.ofs + 2 >= TZ_CAPTURE_BUFFER_SIZE) { - capture[op->frame->step_read_string.ofs] = 0; + CAPTURE[op->frame->step_read_string.ofs] = 0; op->frame->step_read_string.ofs = 0; if (!op->frame->step_read_string.skip) { tz_must(push_frame(state, TZ_OPERATION_STEP_PARTIAL_PRINT)); - op->frame->step_print.str = (char *)capture; + op->frame->step_print.str = (char *)CAPTURE; } } else { uint8_t b; tz_must(tz_parser_read(state, &b)); - char *buf = (char *)capture + op->frame->step_read_string.ofs; + char *buf = (char *)CAPTURE + op->frame->step_read_string.ofs; snprintf(buf, 4, "%02x", b); op->frame->step_read_string.ofs += 2; } @@ -613,27 +630,27 @@ tz_operation_parser_step(tz_parser_state *state) tz_must(tz_parser_read(state, &b)); switch (b) { case 0: - strcpy((char *)capture, "default"); + strcpy((char *)CAPTURE, "default"); tz_must(tz_print_string(state)); break; case 1: - strcpy((char *)capture, "root"); + strcpy((char *)CAPTURE, "root"); tz_must(tz_print_string(state)); break; case 2: - strcpy((char *)capture, "do"); + strcpy((char *)CAPTURE, "do"); tz_must(tz_print_string(state)); break; case 3: - strcpy((char *)capture, "set_delegate"); + strcpy((char *)CAPTURE, "set_delegate"); tz_must(tz_print_string(state)); break; case 4: - strcpy((char *)capture, "remove_delegate"); + strcpy((char *)CAPTURE, "remove_delegate"); tz_must(tz_print_string(state)); break; case 5: - strcpy((char *)capture, "deposit"); + strcpy((char *)CAPTURE, "deposit"); tz_must(tz_print_string(state)); break; case 0xFF: @@ -881,10 +898,10 @@ tz_operation_parser_step(tz_parser_state *state) tz_must(tz_parser_read(state, &b)); switch (b) { case 0: - strcpy((char *)capture, "arith"); + strcpy((char *)CAPTURE, "arith"); break; case 1: - strcpy((char *)capture, "wasm_2_0_0"); + strcpy((char *)CAPTURE, "wasm_2_0_0"); break; default: tz_raise(INVALID_TAG); @@ -897,13 +914,13 @@ tz_operation_parser_step(tz_parser_state *state) tz_must(tz_parser_read(state, &b)); switch (b) { case 0: - strcpy((char *)capture, "yay"); + strcpy((char *)CAPTURE, "yay"); break; case 1: - strcpy((char *)capture, "nay"); + strcpy((char *)CAPTURE, "nay"); break; case 2: - strcpy((char *)capture, "pass"); + strcpy((char *)CAPTURE, "pass"); break; default: tz_raise(INVALID_TAG);