Skip to content

Commit

Permalink
Fix more heap overflows
Browse files Browse the repository at this point in the history
  • Loading branch information
@0xPxt
0xPxt committed Aug 23, 2024
1 parent bd85e73 commit a58c5e3
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 21 deletions.
39 changes: 28 additions & 11 deletions app/src/items.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ static items_error_t items_validateSigners() {
item_t *item = &item_array.items[item_array.numOfItems];
item_t *ofKey_item = &item_array.items[item_array.numOfItems - 1];
uint16_t token_index = 0;
jsmntok_t *token;

PARSER_TO_ITEMS_ERROR(object_get_value(json_all, *curr_token_idx, JSON_SIGNERS, curr_token_idx));

Expand All @@ -173,9 +174,17 @@ static items_error_t items_validateSigners() {
for (uint8_t i = 0; i < (uint8_t)clist_element_count; i++) {
if (array_get_nth_element(json_all, clist_token_index, i, &token_index) == parser_ok) {
uint16_t name_token_index = 0;

if (object_get_value(json_all, token_index, JSON_NAME, &name_token_index) == parser_ok) {
if (MEMCMP("coin.TRANSFER", json_all->buffer + json_all->tokens[name_token_index].start,
sizeof("coin.TRANSFER") - 1) == 0) {
uint16_t len = 0;

token = &(json_all->tokens[name_token_index]);

len = token->end - token->start;

if (len == 0 || strlen("coin.TRANSFER") != len) continue;

if (MEMCMP("coin.TRANSFER", json_all->buffer + token->start, len) == 0) {
if (parser_findPubKeyInClist(ofKey_item->json_token_index) == parser_ok) {
*curr_token_idx = 0;
return items_ok;
Expand Down Expand Up @@ -260,23 +269,31 @@ static items_error_t items_storeAllTransfers() {
if (array_get_nth_element(json_all, clist_token_index, i, &token_index) == parser_ok) {
uint16_t name_token_index = 0;
if (object_get_value(json_all, token_index, JSON_NAME, &name_token_index) == parser_ok) {
uint16_t len = 0;

token = &(json_all->tokens[name_token_index]);
if (MEMCMP("coin.TRANSFER_XCHAIN", json_all->buffer + token->start,
sizeof("coin.TRANSFER_XCHAIN") - 1) == 0) {

len = token->end - token->start;

if (len == 0) continue;

if (strlen("coin.TRANSFER") == len &&
MEMCMP("coin.TRANSFER", json_all->buffer + token->start, len) == 0) {
*curr_token_idx = token_index;
items_storeTxCrossItem(json_all, token_index, &num_of_transfers);
items_storeTxItem(json_all, token_index, &num_of_transfers);
item_array.numOfItems++;
} else if (MEMCMP("coin.TRANSFER", json_all->buffer + token->start,
sizeof("coin.TRANSFER") - 1) == 0) {
} else if (strlen("coin.TRANSFER_XCHAIN") == len &&
MEMCMP("coin.TRANSFER_XCHAIN", json_all->buffer + token->start, len) == 0) {
*curr_token_idx = token_index;
items_storeTxItem(json_all, token_index, &num_of_transfers);
items_storeTxCrossItem(json_all, token_index, &num_of_transfers);
item_array.numOfItems++;
} else if (MEMCMP("coin.ROTATE", json_all->buffer + token->start, sizeof("coin.ROTATE") - 1) ==
0) {
} else if (strlen("coin.ROTATE") == len &&
MEMCMP("coin.ROTATE", json_all->buffer + token->start, len) == 0) {
*curr_token_idx = token_index;
items_storeTxRotateItem(json_all, token_index);
item_array.numOfItems++;
} else if (MEMCMP("coin.GAS", json_all->buffer + token->start, sizeof("coin.GAS") - 1) != 0) {
} else if (strlen("coin.GAS") != len ||
MEMCMP("coin.GAS", json_all->buffer + token->start, len) != 0) {
// Any other case that's not coin.GAS
*curr_token_idx = token_index;
items_storeUnknownItem(json_all, token_index);
Expand Down
3 changes: 2 additions & 1 deletion app/src/items_defs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "zxtypes.h"

#define MAX_NUMBER_OF_ITEMS 22
#define MAX_KEY_LENGTH 23 // Max length of key (screen title on display)

#define PARSER_TO_ITEMS_ERROR(__CALL) \
{ \
Expand All @@ -36,7 +37,7 @@
}

typedef struct {
char key[23];
char key[MAX_KEY_LENGTH];
uint16_t json_token_index;
bool_t can_display;
} item_t;
Expand Down
15 changes: 12 additions & 3 deletions app/src/items_format.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,17 +204,26 @@ items_error_t items_hashToDisplayString(__Z_UNUSED item_t item, char *outVal, ui
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;
uint8_t outVal_idx = 0;
parsed_json_t *json_all = &(parser_getParserTxObj()->json);
uint16_t item_token_index = item.json_token_index;
jsmntok_t *token;
uint16_t len = 0;

PARSER_TO_ITEMS_ERROR(object_get_value(json_all, item_token_index, "name", &token_index));
token = &(json_all->tokens[token_index]);

len = token->end - token->start + sizeof("name: ");
snprintf(outVal, len, "name: %s", json_all->buffer + token->start);
len = token->end - token->start;

if (len == 0) return items_length_zero;

len += sizeof("name: ");

if (len >= outValLen) {
return items_data_too_large;
}

snprintf(outVal, outValLen, "name: %.*s", len, json_all->buffer + token->start);
outVal_idx += len;

// Remove null terminator
Expand Down
5 changes: 1 addition & 4 deletions app/src/json/json_parser.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,8 @@ parser_error_t array_get_nth_element(const parsed_json_t *json, uint16_t array_t

uint16_t element_count = 0;
uint16_t prev_element_end = array_token.start;
while (true) {
while (*token_index < json->numberOfTokens) {
(*token_index)++;
if (*token_index >= json->numberOfTokens) {
break;
}
jsmntok_t current_token = json->tokens[*token_index];
if (current_token.start > array_token.end) {
break;
Expand Down
4 changes: 2 additions & 2 deletions app/src/parser_impl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ parser_error_t parser_findPubKeyInClist(uint16_t key_token_index) {
return parser_no_data;
}

for (uint16_t i = 0; i < (uint8_t)clist_element_count; i++) {
for (uint16_t i = 0; i < clist_element_count; i++) {

Check warning

Code scanning / CodeQL

Comparison result is always the same Warning

Comparison is always false because i >= 0 and 0 >= clist_element_count.
CHECK_ERROR(array_get_nth_element(json_all, clist_token_index, i, &args_token_index));
CHECK_ERROR(object_get_value(json_all, args_token_index, JSON_ARGS, &args_token_index));
CHECK_ERROR(array_get_element_count(json_all, args_token_index, &number_of_args));
Expand Down Expand Up @@ -121,7 +121,7 @@ parser_error_t parser_validateMetaField() {
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;
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';
Expand Down

1 comment on commit a58c5e3

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cpp-Linter Report ⚠️

Some files did not pass the configured checks!

clang-format reports: 2 file(s) not formatted
  • app/src/items_format.c
  • app/src/items_defs.h

Have any feedback or feature suggestions? Share it here.

Please sign in to comment.