Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/address ledger audit vuln #21

Merged
merged 14 commits into from
Nov 7, 2024
7 changes: 5 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ PATH_APP_LOAD_PARAMS = "44'/0'" "44'/1'" "48'/0'" "48'/1'" "49'/0'" "49'/1'" "84
# Application version
APPVERSION_M = 1
APPVERSION_N = 1
APPVERSION_P = 1
APPVERSION_P = 2
APPVERSION_SUFFIX = # if not empty, appended at the end. Do not add a dash.

ifeq ($(APPVERSION_SUFFIX),)
Expand Down Expand Up @@ -159,7 +159,7 @@ DEFINES += HAVE_BOLOS_APP_STACK_CANARY
DEFINES += IO_SEPROXYHAL_BUFFER_SIZE_B=300

# debugging helper functions and macros
CFLAGS += -g -include debug-helpers/debug.h
CFLAGS += -include debug-helpers/debug.h
keiff3r marked this conversation as resolved.
Show resolved Hide resolved

# DEFINES += HAVE_PRINT_STACK_POINTER

Expand All @@ -169,6 +169,9 @@ ifeq ($(DEBUG),10)
DEFINES += HAVE_PRINTF HAVE_SEMIHOSTED_PRINTF PRINTF=semihosted_printf
endif

ifeq ($(DEBUG),1)
CFLAGS += -g
endif
# Needed to be able to include the definition of G_cx
INCLUDES_PATH += $(BOLOS_SDK)/lib_cxng/src

Expand Down
17 changes: 14 additions & 3 deletions src/handler/lib/get_merkle_preimage.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,11 @@ int call_get_merkle_preimage(dispatcher_context_t *dispatcher_context,
cx_sha256_init(&hash_context);

// update hash
crypto_hash_update(&hash_context.header, data_ptr, partial_data_len);
int ret = cx_hash_no_throw(&hash_context.header, 0, data_ptr, partial_data_len, NULL, 0);
if (ret != 0) {
PRINTF("Error updating hash\n");
return -11;
}

buffer_t out_buffer = buffer_create(out_ptr, out_ptr_len);

Expand Down Expand Up @@ -98,10 +102,17 @@ int call_get_merkle_preimage(dispatcher_context_t *dispatcher_context,
}

// update hash
crypto_hash_update(
ret = cx_hash_no_throw(
&hash_context.header,
0,
dispatcher_context->read_buffer.ptr + dispatcher_context->read_buffer.offset,
n_bytes);
n_bytes,
NULL,
0);
if (ret != 0) {
PRINTF("Error updating hash\n");
return -12;
}

// write bytes to output
buffer_write_bytes(&out_buffer, data_ptr, n_bytes);
Expand Down
9 changes: 7 additions & 2 deletions src/handler/sign_erc4361_message.c
Original file line number Diff line number Diff line change
Expand Up @@ -311,9 +311,14 @@ void handler_sign_erc4361_message(dispatcher_context_t *dc, uint8_t protocol_ver
}
// # Format signature into standard bitcoin format
int r_length = sig[3];
int s_length = sig[4 + r_length + 1];
if (r_length < 0 || r_length > 33) {
SAFE_SEND_SW(dc, SW_BAD_STATE); // can never happen
ui_post_processing_confirm_message(dc, false);
return;
}

if (r_length > 33 || s_length > 33) {
int s_length = sig[4 + r_length + 1];
if (s_length < 0 || s_length > 33) {
SAFE_SEND_SW(dc, SW_BAD_STATE); // can never happen
ui_post_processing_confirm_message(dc, false);
return;
Expand Down
14 changes: 12 additions & 2 deletions src/handler/withdraw.c
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,18 @@ static bool display_data_content_and_confirm(dispatcher_context_t* dc,
snprintf(value_with_ticker, sizeof(value_with_ticker), "stBTC %s", value);

// Trim the value of trailing zeros in a char of size of value
int i = sizeof(value_with_ticker) - 1;
int value_with_ticker_len = sizeof(value_with_ticker) - 1;
int i = value_with_ticker_len;
while (value_with_ticker[i] == '0' || value_with_ticker[i] == '\0' ||
value_with_ticker[i] == '.') {
if (i == 0) {
break;
}
i--;
}
value_with_ticker[i + 1] = '\0';
if (i < value_with_ticker_len) {
value_with_ticker[i + 1] = '\0';
}
// Get the second chunk that contains the data to display
call_get_merkle_leaf_element(dc,
data_merkle_root,
Expand Down Expand Up @@ -264,6 +270,10 @@ void add_leading_zeroes(uint8_t* dest_buffer,
PRINTF("Error: Null buffer\n");
return;
}
if (dest_size < src_size) {
PRINTF("Error: Destination buffer is too small\n");
return;
}
// Clear the destination buffer
memset(dest_buffer, 0, dest_size);

Expand Down
Binary file modified tests/snapshots/flex/test_dashboard/00001.png
keiff3r marked this conversation as resolved.
Show resolved Hide resolved
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/snapshots/nanosp/test_dashboard/00001.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/snapshots/nanox/test_dashboard/00001.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/snapshots/stax/test_dashboard/00001.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading