Code cleanup. (#22)

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                            |

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")
 

libstellar/formatter.c

@@ -2,6 +2,7 @@
 #include <stdbool.h>  // bool
 
 #ifdef TEST
+#include <stdio.h>
 #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))
     }

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

libstellar/parser.c

@@ -5,6 +5,10 @@
 #include "stellar/types.h"
 #include "stellar/network.h"
 
+#ifdef TEST
+#include <stdio.h>
+#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,

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;

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
 /**

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);
     }
 }
 

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)