diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 2d24609a..ab447d10 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -92,7 +92,6 @@ | 0x6C66 | `SW_HASH_SIGNING_MODE_NOT_ENABLED` | Hash signing model not enabled | | 0x6D00 | `SW_INS_NOT_SUPPORTED` | No command exists with `INS` | | 0x6E00 | `SW_CLA_NOT_SUPPORTED` | Bad `CLA` used for this application | -| 0xB000 | `SW_WRONG_RESPONSE_LENGTH` | Wrong response length (buffer too small or too big) | | 0xB002 | `SW_DISPLAY_ADDRESS_FAIL` | Failed to display address | | 0xB003 | `SW_DISPLAY_TRANSACTION_HASH_FAIL` | Failed to display transaction hash | | 0xB004 | `SW_WRONG_TX_LENGTH` | Wrong raw transaction length | diff --git a/fuzz/CMakeLists.txt b/fuzz/CMakeLists.txt index 668328b7..83d9c7ac 100644 --- a/fuzz/CMakeLists.txt +++ b/fuzz/CMakeLists.txt @@ -34,6 +34,8 @@ if (${CMAKE_SOURCE_DIR} STREQUAL ${CMAKE_BINARY_DIR}) endif () add_compile_definitions(TEST) +add_compile_definitions(PRINTF=printf) + add_definitions("-DIO_SEPROXYHAL_BUFFER_SIZE_B=128") # cmake -DIO_SEPROXYHAL_BUFFER_SIZE_B=128 add_definitions("-DTARGET_NANOS=1") diff --git a/libstellar/formatter.c b/libstellar/formatter.c index 6acbc4cd..3a44b224 100644 --- a/libstellar/formatter.c +++ b/libstellar/formatter.c @@ -2,6 +2,7 @@ #include // bool #ifdef TEST +#include #define PIC(x) x #else #include "os.h" @@ -61,7 +62,7 @@ static uint8_t plugin_data_pair_count; static bool format_sub_invocation_start(formatter_data_t *fdata); static bool push_to_formatter_stack(format_function_t formatter) { if (formatter_index >= MAX_FORMATTERS_PER_OPERATION) { - // PRINTF("Formatter stack overflow\n"); + PRINTF("Formatter stack overflow\n"); return false; } @@ -784,7 +785,6 @@ static bool format_change_trust_limit(formatter_data_t *fdata) { static bool format_change_trust_detail_liquidity_pool_fee(formatter_data_t *fdata) { STRLCPY(fdata->caption, "Pool Fee Rate", fdata->caption_len); - // TODO: add a function to print a uint32_t uint8_t fee[4] = {0}; for (int i = 0; i < 4; i++) { fee[i] = fdata->envelope->tx_details.tx.op_details.change_trust_op.line.liquidity_pool @@ -1952,7 +1952,7 @@ static bool format_sub_invocation_invoke_host_function_func_name(formatter_data_ push_to_formatter_stack(&format_sub_invocation_invoke_host_function_args)) } } else { - // PRINTF("we should move control to plugin\n"); + PRINTF("we should move control to plugin\n"); parameters_index = 0; FORMATTER_CHECK( push_to_formatter_stack(&format_sub_invocation_invoke_host_function_args_with_plugin)) @@ -1979,11 +1979,11 @@ static bool format_sub_invocation_invoke_host_function_contract_id(formatter_dat } static bool format_sub_invocation_auth_function(formatter_data_t *fdata) { - // PRINTF("function_type: %d\n", fdata->envelope->soroban_authorization.auth_function_type); soroban_authorization_function_type_t auth_function_type = fdata->envelope->type == ENVELOPE_TYPE_SOROBAN_AUTHORIZATION ? fdata->envelope->soroban_authorization.auth_function_type : fdata->envelope->tx_details.tx.op_details.invoke_host_function_op.auth_function_type; + PRINTF("auth_function_type: %d\n", auth_function_type); switch (auth_function_type) { case SOROBAN_AUTHORIZED_FUNCTION_TYPE_CONTRACT_FN: STRLCPY(fdata->caption, "Soroban", fdata->caption_len); @@ -2211,7 +2211,7 @@ static bool format_invoke_host_function_func_name(formatter_data_t *fdata) { FORMATTER_CHECK(push_to_formatter_stack(&format_invoke_host_function_args)) } } else { - // PRINTF("we should move control to plugin\n"); + PRINTF("we should move control to plugin\n"); parameters_index = 0; FORMATTER_CHECK(push_to_formatter_stack(&format_invoke_host_function_args_with_plugin)) } diff --git a/libstellar/include/stellar/types.h b/libstellar/include/stellar/types.h index 8ad5757f..0c295c9e 100644 --- a/libstellar/include/stellar/types.h +++ b/libstellar/include/stellar/types.h @@ -30,9 +30,10 @@ #define VERSION_BYTE_ED25519_SIGNED_PAYLOAD 15 << 3 #define VERSION_BYTE_CONTRACT 2 << 3 -#define ASSET_CODE_MAX_LENGTH 13 -#define CLAIMANTS_MAX_LENGTH 10 -#define PATH_PAYMENT_MAX_PATH_LENGTH 5 +#define ASSET_CODE_MAX_LENGTH 13 +#define CLAIMANTS_MAX_LENGTH 10 +#define PATH_PAYMENT_MAX_PATH_LENGTH 5 +#define HOST_FUNCTION_ARGS_MAX_LENGTH 10 /* For sure not more than 35 operations will fit in that */ #define MAX_OPS 35 diff --git a/libstellar/parser.c b/libstellar/parser.c index d641a1af..52f32227 100644 --- a/libstellar/parser.c +++ b/libstellar/parser.c @@ -5,6 +5,10 @@ #include "stellar/types.h" #include "stellar/network.h" +#ifdef TEST +#include +#endif + #define PARSER_CHECK(x) \ { \ if (!(x)) return false; \ @@ -1049,10 +1053,10 @@ static bool parse_invoke_contract_args(buffer_t *buffer, invoke_contract_args_t args->parameters_length = args_len; args->parameters_position = buffer->offset; - // if (args_len > 10) { - // // TODO: We dont support more than 10 arguments - // return false; - // } + if (args_len > HOST_FUNCTION_ARGS_MAX_LENGTH) { + // We dont support more than 10 arguments + return false; + } // PRINTF("function_name.text_size=%d, function_name.text=%s, args->parameters_length=%d\n", // args->function.name_size, @@ -1125,7 +1129,7 @@ static bool parse_invoke_host_function(buffer_t *buffer, invoke_host_function_op uint32_t type; PARSER_CHECK(buffer_read32(buffer, &type)); op->host_function_type = type; - // PRINTF("host_func_type=%d\n", &op->host_function_type); + PRINTF("host_function_type=%d\n", type); switch (op->host_function_type) { case HOST_FUNCTION_TYPE_INVOKE_CONTRACT: PARSER_CHECK(parse_invoke_contract_args(buffer, &op->invoke_contract_args)) @@ -1174,7 +1178,7 @@ static bool parse_invoke_host_function(buffer_t *buffer, invoke_host_function_op } op->sub_invocations_count = sub_invocations_count; - // PRINTF("sub_invocations_count=%d\n", sub_invocations_count); + PRINTF("sub_invocations_count=%d\n", sub_invocations_count); // for (uint8_t i = 0; i < 16; i++) { // PRINTF("sub_invocation_positions[%d]=%d\n", i, op->sub_invocation_positions[i]); // } @@ -1188,7 +1192,7 @@ static bool parse_extend_footprint_ttl(buffer_t *buffer, extend_footprint_ttl_op } static bool parse_operation(buffer_t *buffer, operation_t *operation) { - // PRINTF("parse_operation: offset=%d\n", buffer->offset); + PRINTF("parse_operation: offset=%ld\n", buffer->offset); explicit_bzero(operation, sizeof(operation_t)); uint32_t op_type; @@ -1318,16 +1322,14 @@ static bool parse_transaction_operation_len(buffer_t *buffer, uint8_t *operation return true; } -// static bool check_operations(buffer_t *buffer, uint8_t op_count) -// { -// // PRINTF("check_operations: offset=%d\n", buffer->offset); -// operation_t op; -// for (uint8_t i = 0; i < op_count; i++) -// { -// PARSER_CHECK(parse_operation(buffer, &op)) -// } -// return true; -// } +static bool check_operations(buffer_t *buffer, uint8_t op_count) { + PRINTF("check_operations: op_count=%d, offset=%ld\n", op_count, buffer->offset); + operation_t op; + for (uint8_t i = 0; i < op_count; i++) { + PARSER_CHECK(parse_operation(buffer, &op)) + } + return true; +} static bool parse_transaction_details(buffer_t *buffer, transaction_details_t *transaction) { // account used to run the (inner)transaction @@ -1394,7 +1396,7 @@ static bool parse_network(buffer_t *buffer, uint8_t *network) { } bool parse_transaction_envelope(const uint8_t *data, size_t data_len, envelope_t *envelope) { - // PRINTF("parse_transaction_envelope: offset=%d\n", buffer->offset); + PRINTF("parse_transaction_envelope\n"); buffer_t buffer = { .ptr = data, .size = data_len, @@ -1425,6 +1427,9 @@ bool parse_transaction_envelope(const uint8_t *data, size_t data_len, envelope_t } envelope->tx_details.tx.operation_position = buffer.offset; + + // check all operations are valid + PARSER_CHECK(check_operations(&buffer, envelope->tx_details.tx.operations_count)); return true; } @@ -1470,7 +1475,7 @@ bool parse_auth_function(buffer_t *buffer, bool parse_soroban_authorization_envelope(const uint8_t *data, size_t data_len, envelope_t *envelope) { - // PRINTF("parse_transaction_envelope: offset=%d\n", buffer->offset); + PRINTF("parse_soroban_authorization_envelope\n"); buffer_t buffer = { .ptr = data, .size = data_len, @@ -1506,7 +1511,7 @@ bool parse_soroban_authorization_envelope(const uint8_t *data, envelope->soroban_authorization.sub_invocation_positions)); } envelope->soroban_authorization.sub_invocations_count = sub_invocations_count; - // PRINTF("sub_invocations_count=%d\n", sub_invocations_count); + PRINTF("sub_invocations_count=%d\n", sub_invocations_count); // for (uint8_t i = 0; i < 16; i++) { // PRINTF("sub_invocation_positions[%d]=%d\n", // i, diff --git a/libstellar/printer.c b/libstellar/printer.c index 98bd02a0..e152b1fa 100644 --- a/libstellar/printer.c +++ b/libstellar/printer.c @@ -235,7 +235,6 @@ bool print_ed25519_signed_payload(const ed25519_signed_payload_t *signed_payload size_t out_len, uint8_t num_chars_l, uint8_t num_chars_r) { - // TODO: calculate the exact length char tmp[ED25519_SIGNED_PAYLOAD_MAX_LENGTH]; if (!encode_ed25519_signed_payload(signed_payload, tmp, sizeof(tmp))) { return false; diff --git a/src/sw.h b/src/sw.h index 17bec006..5c895b55 100644 --- a/src/sw.h +++ b/src/sw.h @@ -4,6 +4,10 @@ * Status word for fail of transaction formatting. */ #define SW_FORMATTING_FAIL 0x6125 +/** + * Status word for too many pages to display. (40 pages max on Stax) + */ +#define SW_TOO_MANY_PAGES 0x6126 /** * Status word for denied by user. */ @@ -28,14 +32,6 @@ * Status word for instruction class is different than CLA. */ #define SW_CLA_NOT_SUPPORTED 0x6E00 -/** - * Status word for wrong response length (buffer too small or too big). - */ -#define SW_WRONG_RESPONSE_LENGTH 0xB000 -/** - * Status word for fail to display BIP32 path. - */ -#define SW_DISPLAY_BIP32_PATH_FAIL 0xB001 /** * Status word for fail to display address. */ @@ -46,6 +42,7 @@ #define SW_DISPLAY_TRANSACTION_HASH_FAIL 0xB003 /** * Status word for wrong transaction length. + * When the data requested by the user is too large, this exception will be thrown. */ #define SW_WRONG_TX_LENGTH 0xB004 /** diff --git a/src/ui/nbgl_transaction.c b/src/ui/nbgl_transaction.c index 273c9a83..af30a949 100644 --- a/src/ui/nbgl_transaction.c +++ b/src/ui/nbgl_transaction.c @@ -79,8 +79,7 @@ static void reject_auth_choice(void); static inline void INCR_AND_CHECK_PAGE_NB(void) { nb_pages++; if (nb_pages >= MAX_NUMBER_OF_PAGES) { - // TODO - THROW(SW_BAD_STATE); + THROW(SW_TOO_MANY_PAGES); } } diff --git a/tests_unit/CMakeLists.txt b/tests_unit/CMakeLists.txt index ef4a688e..e7354e42 100644 --- a/tests_unit/CMakeLists.txt +++ b/tests_unit/CMakeLists.txt @@ -34,6 +34,7 @@ if(${CMAKE_SOURCE_DIR} STREQUAL ${CMAKE_BINARY_DIR}) endif() add_compile_definitions(TEST) +add_compile_definitions(PRINTF=printf) include_directories(../libstellar/include) include_directories($ENV{BOLOS_SDK}/lib_standard_app)