From 991d66a603554dcc6257c8cec4420d67900936ee Mon Sep 17 00:00:00 2001 From: 0xPxt Date: Thu, 22 Aug 2024 21:10:13 +0200 Subject: [PATCH] Fixing edge cases shown when fuzzing --- app/src/items.c | 10 ++-- app/src/items.h | 4 +- app/src/items_defs.h | 4 +- app/src/items_format.c | 108 +++++++++++++++++++++++++---------------- app/src/items_format.h | 28 +++++------ app/src/parser.c | 9 ++-- app/src/parser_impl.c | 9 +++- 7 files changed, 103 insertions(+), 69 deletions(-) diff --git a/app/src/items.c b/app/src/items.c index 4d16db8..46e81af 100644 --- a/app/src/items.c +++ b/app/src/items.c @@ -48,9 +48,9 @@ item_array_t item_array; uint8_t hash[BLAKE2B_HASH_SIZE] = {0}; -char base64_hash[44]; +char base64_hash[45]; -void items_initItems() { +items_error_t items_initItems() { MEMZERO(&item_array, sizeof(item_array_t)); item_array.numOfUnknownCapabitilies = 1; @@ -58,11 +58,13 @@ void items_initItems() { for (uint8_t i = 0; i < sizeof(item_array.items) / sizeof(item_array.items[0]); i++) { item_array.items[i].can_display = bool_true; } + + return items_ok; } item_array_t *items_getItemArray() { return &item_array; } -void items_storeItems() { +items_error_t items_storeItems() { items_storeSigningTransaction(); items_storeNetwork(); @@ -90,6 +92,8 @@ void items_storeItems() { items_storeHash(); items_storeSignForAddr(); + + return items_ok; } uint16_t items_getTotalItems() { return item_array.numOfItems; } diff --git a/app/src/items.h b/app/src/items.h index afb1474..3c921ce 100644 --- a/app/src/items.h +++ b/app/src/items.h @@ -22,7 +22,7 @@ #include "parser_common.h" #include "zxtypes.h" -void items_initItems(); -void items_storeItems(); +items_error_t items_initItems(); +items_error_t items_storeItems(); uint16_t items_getTotalItems(); item_array_t *items_getItemArray(); \ No newline at end of file diff --git a/app/src/items_defs.h b/app/src/items_defs.h index 7016d87..8d14cfc 100644 --- a/app/src/items_defs.h +++ b/app/src/items_defs.h @@ -43,6 +43,8 @@ typedef struct { typedef enum { items_ok, + items_length_zero, + items_data_too_large, items_error, } items_error_t; @@ -50,5 +52,5 @@ typedef struct { item_t items[MAX_NUMBER_OF_ITEMS]; uint8_t numOfItems; uint8_t numOfUnknownCapabitilies; - items_error_t (*toString[MAX_NUMBER_OF_ITEMS])(item_t item, char *outVal, uint16_t *outValLen); + items_error_t (*toString[MAX_NUMBER_OF_ITEMS])(item_t item, char *outVal, uint16_t outValLen); } item_array_t; diff --git a/app/src/items_format.c b/app/src/items_format.c index d9eca5a..2755c99 100644 --- a/app/src/items_format.c +++ b/app/src/items_format.c @@ -21,56 +21,80 @@ #include "crypto.h" #include "parser.h" -extern char base64_hash[44]; +extern char base64_hash[45]; -items_error_t items_stdToDisplayString(item_t item, char *outVal, uint16_t *outValLen) { +items_error_t items_stdToDisplayString(item_t item, char *outVal, uint16_t outValLen) { parsed_json_t *json_all = &(parser_getParserTxObj()->json); jsmntok_t *token = &(json_all->tokens[item.json_token_index]); + uint16_t len = token->end - token->start + 1; - *outValLen = token->end - token->start + 1; - snprintf(outVal, *outValLen, "%s", json_all->buffer + token->start); + if (len == 1) return items_length_zero; + + if (len > outValLen) { + return items_data_too_large; + } + + snprintf(outVal, len, "%s", json_all->buffer + token->start); return items_ok; } -items_error_t items_nothingToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t *outValLen) { +items_error_t items_nothingToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t outValLen) { char nothing[] = " "; - *outValLen = 2; - snprintf(outVal, *outValLen, "%s", nothing); + uint16_t len = 2; + + if (len > outValLen) return items_data_too_large; + + snprintf(outVal, len, "%s", nothing); return items_ok; } -items_error_t items_warningToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t *outValLen) { - *outValLen = sizeof(WARNING_TEXT); - snprintf(outVal, *outValLen, WARNING_TEXT); +items_error_t items_warningToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t outValLen) { + uint16_t len = sizeof(WARNING_TEXT); + + if (len > outValLen) return items_data_too_large; + + snprintf(outVal, len, WARNING_TEXT); return items_ok; } -items_error_t items_cautionToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t *outValLen) { - *outValLen = sizeof(CAUTION_TEXT); - snprintf(outVal, *outValLen, CAUTION_TEXT); +items_error_t items_cautionToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t outValLen) { + uint16_t len = sizeof(CAUTION_TEXT); + + if (len > outValLen) return items_data_too_large; + + snprintf(outVal, len, CAUTION_TEXT); return items_ok; } -items_error_t items_txTooLargeToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t *outValLen) { - *outValLen = sizeof(TX_TOO_LARGE_TEXT); - snprintf(outVal, *outValLen, TX_TOO_LARGE_TEXT); +items_error_t items_txTooLargeToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t outValLen) { + uint16_t len = sizeof(TX_TOO_LARGE_TEXT); + + if (len > outValLen) return items_data_too_large; + + snprintf(outVal, len, TX_TOO_LARGE_TEXT); return items_ok; } -items_error_t items_signingToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t *outValLen) { - *outValLen = sizeof("Transaction"); - snprintf(outVal, *outValLen, "Transaction"); +items_error_t items_signingToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t outValLen) { + uint16_t len = sizeof("Transaction"); + + if (len > outValLen) return items_data_too_large; + + snprintf(outVal, len, "Transaction"); return items_ok; } -items_error_t items_requiringToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t *outValLen) { - *outValLen = sizeof("Capabilities"); - snprintf(outVal, *outValLen, "Capabilities"); +items_error_t items_requiringToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t outValLen) { + uint16_t len = sizeof("Capabilities"); + + if (len > outValLen) return items_data_too_large; + + snprintf(outVal, len, "Capabilities"); return items_ok; } -items_error_t items_transferToDisplayString(item_t item, char *outVal, uint16_t *outValLen) { +items_error_t items_transferToDisplayString(item_t item, char *outVal, uint16_t outValLen) { char amount[50]; uint8_t amount_len = 0; char to[65]; @@ -89,13 +113,13 @@ items_error_t items_transferToDisplayString(item_t item, char *outVal, uint16_t PARSER_TO_ITEMS_ERROR(parser_arrayElementToString(token_index, 2, amount, &amount_len)); - *outValLen = amount_len + from_len + to_len + sizeof(" from ") + sizeof(" to ") + 4 * sizeof("\""); - snprintf(outVal, *outValLen, "%s from \"%s\" to \"%s\"", amount, from, to); + outValLen = amount_len + from_len + to_len + sizeof(" from ") + sizeof(" to ") + 4 * sizeof("\""); + snprintf(outVal, outValLen, "%s from \"%s\" to \"%s\"", amount, from, to); return items_ok; } -items_error_t items_crossTransferToDisplayString(item_t item, char *outVal, uint16_t *outValLen) { +items_error_t items_crossTransferToDisplayString(item_t item, char *outVal, uint16_t outValLen) { char amount[50]; uint8_t amount_len = 0; char to[65]; @@ -118,14 +142,14 @@ items_error_t items_crossTransferToDisplayString(item_t item, char *outVal, uint PARSER_TO_ITEMS_ERROR(parser_arrayElementToString(token_index, 3, chain, &chain_len)); - *outValLen = amount_len + from_len + to_len + chain_len + sizeof("Cross-chain ") + sizeof(" from ") + sizeof(" to ") + + outValLen = amount_len + from_len + to_len + chain_len + sizeof("Cross-chain ") + sizeof(" from ") + sizeof(" to ") + 6 * sizeof("\"") + sizeof(" to chain "); - snprintf(outVal, *outValLen, "Cross-chain %s from \"%s\" to \"%s\" to chain \"%s\"", amount, from, to, chain); + snprintf(outVal, outValLen, "Cross-chain %s from \"%s\" to \"%s\" to chain \"%s\"", amount, from, to, chain); return items_ok; } -items_error_t items_rotateToDisplayString(item_t item, char *outVal, uint16_t *outValLen) { +items_error_t items_rotateToDisplayString(item_t item, char *outVal, uint16_t outValLen) { uint16_t token_index = 0; uint16_t item_token_index = item.json_token_index; parsed_json_t *json_all = &(parser_getParserTxObj()->json); @@ -135,13 +159,13 @@ items_error_t items_rotateToDisplayString(item_t item, char *outVal, uint16_t *o PARSER_TO_ITEMS_ERROR(array_get_nth_element(json_all, token_index, 0, &token_index)); token = &(json_all->tokens[token_index]); - *outValLen = token->end - token->start + sizeof("\"\""); - snprintf(outVal, *outValLen, "\"%s\"", json_all->buffer + token->start); + outValLen = token->end - token->start + sizeof("\"\""); + snprintf(outVal, outValLen, "\"%s\"", json_all->buffer + token->start); return items_ok; } -items_error_t items_gasToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t *outValLen) { +items_error_t items_gasToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t outValLen) { char gasLimit[10]; uint8_t gasLimit_len = 0; char gasPrice[64]; @@ -165,19 +189,19 @@ items_error_t items_gasToDisplayString(__Z_UNUSED item_t item, char *outVal, uin gasPrice_len = token->end - token->start + 1; snprintf(gasPrice, gasPrice_len, "%s", json_all->buffer + token->start); - *outValLen = gasLimit_len + gasPrice_len + sizeof("at most ") + sizeof(" at price "); - snprintf(outVal, *outValLen, "at most %s at price %s", gasLimit, gasPrice); + outValLen = gasLimit_len + gasPrice_len + sizeof("at most ") + sizeof(" at price "); + snprintf(outVal, outValLen, "at most %s at price %s", gasLimit, gasPrice); return items_ok; } -items_error_t items_hashToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t *outValLen) { - *outValLen = sizeof(base64_hash); - snprintf(outVal, *outValLen, "%s", base64_hash); +items_error_t items_hashToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t outValLen) { + outValLen = sizeof(base64_hash) - 1; + snprintf(outVal, outValLen, "%s", base64_hash); return items_ok; } -items_error_t items_unknownCapabilityToDisplayString(item_t item, char *outVal, uint16_t *outValLen) { +items_error_t items_unknownCapabilityToDisplayString(item_t item, char *outVal, uint16_t outValLen) { uint16_t token_index = 0; uint16_t args_count = 0; uint8_t len = 0; @@ -241,13 +265,13 @@ items_error_t items_unknownCapabilityToDisplayString(item_t item, char *outVal, outVal_idx += len; } - *outValLen = outVal_idx; + outValLen = outVal_idx; return items_ok; } #if defined(TARGET_NANOS) || defined(TARGET_NANOX) || defined(TARGET_NANOS2) || defined(TARGET_STAX) || defined(TARGET_FLEX) -items_error_t items_signForAddrToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t *outValLen) { +items_error_t items_signForAddrToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t outValLen) { uint8_t address[65]; uint16_t address_len; @@ -255,8 +279,8 @@ items_error_t items_signForAddrToDisplayString(__Z_UNUSED item_t item, char *out return items_error; } - *outValLen = sizeof(address); - array_to_hexstr(outVal, *outValLen, address, PUB_KEY_LENGTH); + outValLen = sizeof(address); + array_to_hexstr(outVal, outValLen, address, PUB_KEY_LENGTH); return items_ok; } diff --git a/app/src/items_format.h b/app/src/items_format.h index 4f85b2b..738d759 100644 --- a/app/src/items_format.h +++ b/app/src/items_format.h @@ -27,19 +27,19 @@ #define TX_TOO_LARGE_TEXT \ "Transaction too large for Ledger to display. PROCEED WITH GREAT CAUTION. Do you want to continue?" -items_error_t items_stdToDisplayString(item_t item, char *outVal, uint16_t *outValLen); -items_error_t items_nothingToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t *outValLen); -items_error_t items_warningToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t *outValLen); -items_error_t items_cautionToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t *outValLen); -items_error_t items_txTooLargeToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t *outValLen); -items_error_t items_signingToDisplayString(item_t item, char *outVal, uint16_t *outValLen); -items_error_t items_requiringToDisplayString(item_t item, char *outVal, uint16_t *outValLen); -items_error_t items_transferToDisplayString(item_t item, char *outVal, uint16_t *outValLen); -items_error_t items_crossTransferToDisplayString(item_t item, char *outVal, uint16_t *outValLen); -items_error_t items_rotateToDisplayString(item_t item, char *outVal, uint16_t *outValLen); -items_error_t items_gasToDisplayString(item_t item, char *outVal, uint16_t *outValLen); -items_error_t items_hashToDisplayString(item_t item, char *outVal, uint16_t *outValLen); -items_error_t items_unknownCapabilityToDisplayString(item_t item, char *outVal, uint16_t *outValLen); +items_error_t items_stdToDisplayString(item_t item, char *outVal, uint16_t outValLen); +items_error_t items_nothingToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t outValLen); +items_error_t items_warningToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t outValLen); +items_error_t items_cautionToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t outValLen); +items_error_t items_txTooLargeToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t outValLen); +items_error_t items_signingToDisplayString(item_t item, char *outVal, uint16_t outValLen); +items_error_t items_requiringToDisplayString(item_t item, char *outVal, uint16_t outValLen); +items_error_t items_transferToDisplayString(item_t item, char *outVal, uint16_t outValLen); +items_error_t items_crossTransferToDisplayString(item_t item, char *outVal, uint16_t outValLen); +items_error_t items_rotateToDisplayString(item_t item, char *outVal, uint16_t outValLen); +items_error_t items_gasToDisplayString(item_t item, char *outVal, uint16_t outValLen); +items_error_t items_hashToDisplayString(item_t item, char *outVal, uint16_t outValLen); +items_error_t items_unknownCapabilityToDisplayString(item_t item, char *outVal, uint16_t outValLen); #if defined(TARGET_NANOS) || defined(TARGET_NANOX) || defined(TARGET_NANOS2) || defined(TARGET_STAX) || defined(TARGET_FLEX) -items_error_t items_signForAddrToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t *outValLen); +items_error_t items_signForAddrToDisplayString(__Z_UNUSED item_t item, char *outVal, uint16_t outValLen); #endif diff --git a/app/src/parser.c b/app/src/parser.c index c606065..83b0d97 100644 --- a/app/src/parser.c +++ b/app/src/parser.c @@ -50,8 +50,8 @@ parser_error_t parser_parse(parser_context_t *ctx, const uint8_t *data, size_t d CHECK_ERROR(_read_json_tx(ctx, tx_obj)); - items_initItems(); - items_storeItems(); + ITEMS_TO_PARSER_ERROR(items_initItems()); + ITEMS_TO_PARSER_ERROR(items_storeItems()); return parser_ok; } @@ -100,8 +100,7 @@ parser_error_t parser_getItem(const parser_context_t *ctx, uint8_t displayIdx, c *pageCount = 1; uint8_t numItems = 0; item_array_t *item_array = items_getItemArray(); - char tempVal[255] = {0}; - uint16_t tempValLen = 0; + char tempVal[300] = {0}; CHECK_ERROR(parser_getNumItems(ctx, &numItems)) CHECK_APP_CANARY() @@ -109,7 +108,7 @@ parser_error_t parser_getItem(const parser_context_t *ctx, uint8_t displayIdx, c cleanOutput(outKey, outKeyLen, outVal, outValLen); snprintf(outKey, outKeyLen, "%s", item_array->items[displayIdx].key); - ITEMS_TO_PARSER_ERROR(item_array->toString[displayIdx](item_array->items[displayIdx], tempVal, &tempValLen)); + ITEMS_TO_PARSER_ERROR(item_array->toString[displayIdx](item_array->items[displayIdx], tempVal, sizeof(tempVal))); pageString(outVal, outValLen, tempVal, pageIdx, pageCount); return parser_ok; diff --git a/app/src/parser_impl.c b/app/src/parser_impl.c index 4d0f671..7208b54 100644 --- a/app/src/parser_impl.c +++ b/app/src/parser_impl.c @@ -104,14 +104,14 @@ parser_error_t parser_arrayElementToString(uint16_t json_token_index, uint16_t e parser_error_t parser_validateMetaField() { const char *keywords[20] = {JSON_CREATION_TIME, JSON_TTL, JSON_GAS_LIMIT, JSON_CHAIN_ID, JSON_GAS_PRICE, JSON_SENDER}; - char meta_curr_key[20]; + char meta_curr_key[40]; uint16_t meta_token_index = 0; uint16_t meta_num_elements = 0; uint16_t key_token_idx = 0; parsed_json_t *json_all = &(parser_tx_obj->json); jsmntok_t *token; - object_get_value(json_all, 0, JSON_META, &meta_token_index); + CHECK_ERROR(object_get_value(json_all, 0, JSON_META, &meta_token_index)); if (!items_isNullField(meta_token_index)) { object_get_element_count(json_all, meta_token_index, &meta_num_elements); @@ -120,6 +120,9 @@ parser_error_t parser_validateMetaField() { object_get_nth_key(json_all, meta_token_index, i, &key_token_idx); token = &(json_all->tokens[key_token_idx]); + // Prevent buffer overflow in case of big key-value pair in meta field. + if (token->end - token->start > sizeof(meta_curr_key)) return parser_invalid_meta_field; + MEMCPY(meta_curr_key, json_all->buffer + token->start, token->end - token->start); meta_curr_key[token->end - token->start] = '\0'; @@ -177,5 +180,7 @@ bool items_isNullField(uint16_t json_token_index) { parsed_json_t *json_all = &(parser_getParserTxObj()->json); jsmntok_t *token = &(json_all->tokens[json_token_index]); + if (token->end - token->start != sizeof("null") - 1) return false; + return (MEMCMP("null", json_all->buffer + token->start, token->end - token->start) == 0); }