Skip to content

Commit

Permalink
Add an output length parameter to most tz_format_*() functions
Browse files Browse the repository at this point in the history 
We just return an error if we don't fit.  This shouldn't happen unless
there is a mistake in the code, but failing gracefully is vastly superior
to having a buffer overflow.
  • Loading branch information
@elric1
elric1 committed Sep 24, 2023
1 parent 870282f commit 60c911a
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 72 deletions.
14 changes: 8 additions & 6 deletions app/src/apdu_pubkey.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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;
}

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}
Expand Down
3 changes: 2 additions & 1 deletion app/src/apdu_sign.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
58 changes: 38 additions & 20 deletions app/src/parser/formatting.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand All @@ -230,22 +236,29 @@ 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])
++zcount;

if (zcount == l) {
obuf[0] = '0';
return;
return 0;
}

for (i = zcount, high = obuf_len - 1; i < l; ++i, high = j) {
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -375,21 +389,25 @@ 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);
uint8_t tmp[32];
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;

Expand All @@ -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;

Expand All @@ -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;

Expand All @@ -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);
}
17 changes: 9 additions & 8 deletions app/src/parser/formatting.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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)

Expand All @@ -224,25 +225,25 @@ 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)
// tag 1: txrolluph(20) + padding(1), KT1(36)
// 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);
2 changes: 1 addition & 1 deletion app/src/parser/num_parser.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Loading

0 comments on commit 60c911a

Please sign in to comment.