diff --git a/src_features/signMessage/cmd_signMessage.c b/src_features/signMessage/cmd_signMessage.c index c344f92..3940c70 100644 --- a/src_features/signMessage/cmd_signMessage.c +++ b/src_features/signMessage/cmd_signMessage.c @@ -12,7 +12,7 @@ static const char SIGN_MAGIC[] = * Check if a given character is a "special" displayable ASCII character * * @param[in] c character we're checking - * @return wether the character is special or not + * @return whether the character is special or not */ static inline bool is_char_special(char c) { return ((c >= '\b') && (c <= '\r')); @@ -23,7 +23,7 @@ static inline bool is_char_special(char c) { * * @param[in] data the input data * @param[in] the length of the input data - * @return wether the data is fully ASCII or not + * @return whether the data is fully ASCII or not */ static bool is_data_ascii(const uint8_t *const data, size_t length) { for (uint8_t idx = 0; idx < length; ++idx) { @@ -59,7 +59,7 @@ static bool is_value_str_ascii() { * * @param[in] data the input data * @param[in] length the data length - * @param[in] is_ascii wether the data is ASCII or not + * @param[in] is_ascii whether the data is ASCII or not */ static void feed_value_str(const uint8_t *const data, size_t length, bool is_ascii) { uint16_t value_strlen = strlen(strings.tmp.tmp); diff --git a/src_features/signMessageEIP712/commands_712.c b/src_features/signMessageEIP712/commands_712.c index 5681767..008c4bc 100644 --- a/src_features/signMessageEIP712/commands_712.c +++ b/src_features/signMessageEIP712/commands_712.c @@ -14,6 +14,14 @@ #include "filtering.h" #include "common_712.h" +/** + * Send the response to the previous APDU command + * + * In case of an error it uses the global variable to retrieve the error code and resets + * the app context + * + * @param[in] success whether the command was successful + */ void handle_eip712_return_code(bool success) { if (success) @@ -31,6 +39,12 @@ void handle_eip712_return_code(bool success) } } +/** + * Process the EIP712 struct definition command + * + * @param[in] apdu_buf the APDU payload + * @return whether the command was successful or not + */ bool handle_eip712_struct_def(const uint8_t *const apdu_buf) { bool ret = true; @@ -61,6 +75,12 @@ bool handle_eip712_struct_def(const uint8_t *const apdu_buf) return ret; } +/** + * Process the EIP712 struct implementation command + * + * @param[in] apdu_buf the APDU payload + * @return whether the command was successful or not + */ bool handle_eip712_struct_impl(const uint8_t *const apdu_buf) { bool ret = false; @@ -112,10 +132,17 @@ bool handle_eip712_struct_impl(const uint8_t *const apdu_buf) return ret; } +/** + * Process the EIP712 filtering command + * + * @param[in] apdu_buf the APDU payload + * @return whether the command was successful or not + */ bool handle_eip712_filtering(const uint8_t *const apdu_buf) { bool ret = true; bool reply_apdu = true; + e_filtering_type type; if (eip712_context == NULL) { @@ -135,11 +162,14 @@ bool handle_eip712_filtering(const uint8_t *const apdu_buf) break; case P1_CONTRACT_NAME: case P1_FIELD_NAME: + type = (apdu_buf[OFFSET_P1] == P1_CONTRACT_NAME) + ? FILTERING_CONTRACT_NAME + : FILTERING_STRUCT_FIELD; if (ui_712_get_filtering_mode() == EIP712_FILTERING_FULL) { ret = provide_filtering_info(&apdu_buf[OFFSET_CDATA], apdu_buf[OFFSET_LC], - apdu_buf[OFFSET_P1]); + type); if ((apdu_buf[OFFSET_P1] == P1_CONTRACT_NAME) && ret) { reply_apdu = false; @@ -161,6 +191,12 @@ bool handle_eip712_filtering(const uint8_t *const apdu_buf) return ret; } +/** + * Process the EIP712 sign command + * + * @param[in] apdu_buf the APDU payload + * @return whether the command was successful or not + */ bool handle_eip712_sign(const uint8_t *const apdu_buf) { bool ret = false; diff --git a/src_features/signMessageEIP712/context.c b/src_features/signMessageEIP712/context.c index 0318f39..b03c554 100644 --- a/src_features/signMessageEIP712/context.c +++ b/src_features/signMessageEIP712/context.c @@ -17,6 +17,7 @@ s_eip712_context *eip712_context = NULL; /** + * Initialize the EIP712 context * * @return a boolean indicating if the initialization was successful or not */ @@ -59,6 +60,9 @@ bool eip712_context_init(void) return true; } +/** + * De-initialize the EIP712 context + */ void eip712_context_deinit(void) { typed_data_deinit(); diff --git a/src_features/signMessageEIP712/field_hash.c b/src_features/signMessageEIP712/field_hash.c index 4b915e5..1cec18e 100644 --- a/src_features/signMessageEIP712/field_hash.c +++ b/src_features/signMessageEIP712/field_hash.c @@ -18,6 +18,11 @@ static s_field_hashing *fh = NULL; +/** + * Initialize the field hash context + * + * @return whether the initialization was successful or not + */ bool field_hash_init(void) { if (fh == NULL) @@ -32,14 +37,25 @@ bool field_hash_init(void) return true; } +/** + * Deinitialize the field hash context + */ void field_hash_deinit(void) { fh = NULL; } +/** + * Special handling of the first chunk received from a field value + * + * @param[in] field_ptr pointer to the struct field definition + * @param[in] data the field value + * @param[in,out] data_length the value length + * @return the data pointer + */ static const uint8_t *field_hash_prepare(const void *const field_ptr, - const uint8_t *data, - uint8_t *data_length) + const uint8_t *data, + uint8_t *data_length) { e_type field_type; @@ -56,6 +72,16 @@ static const uint8_t *field_hash_prepare(const void *const field_ptr, return data; } +/** + * Finalize static field hash + * + * Encode the field data depending on its type + * + * @param[in] field_ptr pointer to the struct field definition + * @param[in] data the field value + * @param[in] data_length the value length + * @return pointer to the encoded value + */ static const uint8_t *field_hash_finalize_static(const void *const field_ptr, const uint8_t *const data, uint8_t data_length) @@ -85,7 +111,6 @@ static const uint8_t *field_hash_finalize_static(const void *const field_ptr, default: apdu_response_code = APDU_RESPONSE_INVALID_DATA; PRINTF("Unknown solidity type!\n"); - return NULL; } if (value == NULL) @@ -96,6 +121,13 @@ static const uint8_t *field_hash_finalize_static(const void *const field_ptr, return value; } +/** + * Finalize dynamic field hash + * + * Allocate and hash the data + * + * @return pointer to the hash, \ref NULL if it failed + */ static uint8_t *field_hash_finalize_dynamic(void) { uint8_t *value; @@ -115,7 +147,13 @@ static uint8_t *field_hash_finalize_dynamic(void) return value; } -static void field_hash_feed_parent(e_type field_type, const uint8_t *const value) +/** + * Feed the newly created field hash into the parent struct's progressive hash + * + * @param[in] field_type the struct field's type + * @param[in] hash the field hash + */ +static void field_hash_feed_parent(e_type field_type, const uint8_t *const hash) { uint8_t len; @@ -130,13 +168,23 @@ static void field_hash_feed_parent(e_type field_type, const uint8_t *const value // last thing in mem is the hash of the previous field // and just before it is the current hash context - cx_sha3_t *hash_ctx = (cx_sha3_t*)(value - sizeof(cx_sha3_t)); - // start the progressive hash on it - hash_nbytes(value, len, (cx_hash_t*)hash_ctx); + cx_sha3_t *hash_ctx = (cx_sha3_t*)(hash - sizeof(cx_sha3_t)); + // continue the progressive hash on it + hash_nbytes(hash, len, (cx_hash_t*)hash_ctx); // deallocate it mem_dealloc(len); } +/** + * Special domain fields handling + * + * Do something special for certain EIP712Domain fields + * + * @param[in] field_ptr pointer to the struct field definition + * @param[in] data the field value + * @param[in] data_length the value length + * @return whether an error occured or not + */ static bool field_hash_domain_special_fields(const void *const field_ptr, const uint8_t *const data, uint8_t data_length) @@ -174,6 +222,14 @@ static bool field_hash_domain_special_fields(const void *const field_ptr, return true; } +/** + * Finalize the data hashing + * + * @param[in] field_ptr pointer to the struct field definition + * @param[in] data the field value + * @param[in] data_length the value length + * @return whether an error occured or not + */ static bool field_hash_finalize(const void *const field_ptr, const uint8_t *const data, uint8_t data_length) @@ -214,6 +270,14 @@ static bool field_hash_finalize(const void *const field_ptr, return true; } +/** + * Hash a field value + * + * @param[in] data the field value + * @param[in] data_length the value length + * @param[in] partial whether there is more of that data coming later or not + * @return whether the data hashing was successful or not + */ bool field_hash(const uint8_t *data, uint8_t data_length, bool partial) diff --git a/src_features/signMessageEIP712/filtering.c b/src_features/signMessageEIP712/filtering.c index 5050e31..ac283e1 100644 --- a/src_features/signMessageEIP712/filtering.c +++ b/src_features/signMessageEIP712/filtering.c @@ -22,15 +22,62 @@ static const uint8_t EIP712_FEEDER_PUBLIC_KEY[] = { #endif // HAVE_EIP712_TESTING_KEY -static bool verify_filtering_signature(uint8_t dname_length, - const char *const dname, - uint8_t sig_length, - const uint8_t *const sig, - uint8_t p1) +/** + * Reconstruct the field path and hash it + * + * @param[in] hash_ctx the hashing context + */ +static void hash_filtering_path(cx_hash_t *const hash_ctx) { const void *field_ptr; const char *key; uint8_t key_len; + + for (uint8_t i = 0; i < path_get_depth_count(); ++i) + { + if (i > 0) + { + hash_byte('.', hash_ctx); + } + if ((field_ptr = path_get_nth_field(i + 1)) != NULL) + { + if ((key = get_struct_field_keyname(field_ptr, &key_len)) != NULL) + { + // field name + hash_nbytes((uint8_t*)key, key_len, hash_ctx); + + // array levels + if (struct_field_is_array(field_ptr)) + { + uint8_t lvl_count; + + get_struct_field_array_lvls_array(field_ptr, &lvl_count); + for (int j = 0; j < lvl_count; ++j) + { + hash_nbytes((uint8_t*)".[]", 3, hash_ctx); + } + } + } + } + } +} + +/** + * Verify the provided signature + * + * @param[in] dname_length length of provided substitution name + * @param[in] dname provided substitution name + * @param[in] sig_length provided signature length + * @param[in] sig pointer to the provided signature + * @param[in] type the type of filtering + * @return whether the signature verification worked or not + */ +static bool verify_filtering_signature(uint8_t dname_length, + const char *const dname, + uint8_t sig_length, + const uint8_t *const sig, + e_filtering_type type) +{ uint8_t hash[INT256_LENGTH]; cx_ecfp_public_key_t verifying_key; cx_sha256_t hash_ctx; @@ -52,37 +99,9 @@ static bool verify_filtering_signature(uint8_t dname_length, sizeof(eip712_context->schema_hash), (cx_hash_t*)&hash_ctx); - if (p1 == P1_FIELD_NAME) + if (type == FILTERING_STRUCT_FIELD) { - uint8_t depth_count = path_get_depth_count(); - - for (uint8_t i = 0; i < depth_count; ++i) - { - if (i > 0) - { - hash_byte('.', (cx_hash_t*)&hash_ctx); - } - if ((field_ptr = path_get_nth_field(i + 1)) != NULL) - { - if ((key = get_struct_field_keyname(field_ptr, &key_len)) != NULL) - { - // field name - hash_nbytes((uint8_t*)key, key_len, (cx_hash_t*)&hash_ctx); - - // array levels - if (struct_field_is_array(field_ptr)) - { - uint8_t lvl_count; - - get_struct_field_array_lvls_array(field_ptr, &lvl_count); - for (int j = 0; j < lvl_count; ++j) - { - hash_nbytes((uint8_t*)".[]", 3, (cx_hash_t*)&hash_ctx); - } - } - } - } - } + hash_filtering_path((cx_hash_t*)&hash_ctx); } // Display name @@ -123,7 +142,17 @@ static bool verify_filtering_signature(uint8_t dname_length, return true; } -bool provide_filtering_info(const uint8_t *const payload, uint8_t length, uint8_t p1) +/** + * Provide filtering information about upcoming struct field + * + * @param[in] payload the raw data received + * @param[in] length payload length + * @param[in] type the type of filtering + * @return if everything went well or not + */ +bool provide_filtering_info(const uint8_t *const payload, + uint8_t length, + e_filtering_type type) { bool ret = false; uint8_t dname_len; @@ -131,7 +160,7 @@ bool provide_filtering_info(const uint8_t *const payload, uint8_t length, uin uint8_t sig_len; const uint8_t *sig; - if (p1 == P1_CONTRACT_NAME) + if (type == FILTERING_CONTRACT_NAME) { if (path_get_root_type() != ROOT_DOMAIN) { @@ -139,7 +168,7 @@ bool provide_filtering_info(const uint8_t *const payload, uint8_t length, uin return false; } } - else // P1_FIELD_NAME + else // FILTERING_STRUCT_FIELD { if (path_get_root_type() != ROOT_MESSAGE) { @@ -157,9 +186,9 @@ bool provide_filtering_info(const uint8_t *const payload, uint8_t length, uin sig = &payload[1 + dname_len + 1]; if ((sig_len > 0) && ((1 + dname_len + 1 + sig_len) == length)) { - if ((ret = verify_filtering_signature(dname_len, dname, sig_len, sig, p1))) + if ((ret = verify_filtering_signature(dname_len, dname, sig_len, sig, type))) { - if (p1 == P1_CONTRACT_NAME) + if (type == FILTERING_CONTRACT_NAME) { if (!N_storage.verbose_eip712) { @@ -168,7 +197,7 @@ bool provide_filtering_info(const uint8_t *const payload, uint8_t length, uin ui_712_redraw_generic_step(); } } - else // P1_FIELD_NAME + else // FILTERING_STRUCT_FIELD { if (dname_len > 0) // don't substitute for an empty name { diff --git a/src_features/signMessageEIP712/filtering.h b/src_features/signMessageEIP712/filtering.h index e04c6a1..c914dd7 100644 --- a/src_features/signMessageEIP712/filtering.h +++ b/src_features/signMessageEIP712/filtering.h @@ -6,7 +6,15 @@ #include #include -bool provide_filtering_info(const uint8_t *const payload, uint8_t length, uint8_t p1); +typedef enum +{ + FILTERING_CONTRACT_NAME, + FILTERING_STRUCT_FIELD +} e_filtering_type; + +bool provide_filtering_info(const uint8_t *const payload, + uint8_t length, + e_filtering_type type); #endif // HAVE_EIP712_FULL_SUPPORT diff --git a/src_features/signMessageEIP712/hash_bytes.c b/src_features/signMessageEIP712/hash_bytes.c index 0a5ee6a..8b69c94 100644 --- a/src_features/signMessageEIP712/hash_bytes.c +++ b/src_features/signMessageEIP712/hash_bytes.c @@ -9,7 +9,7 @@ * @param[in] n number of bytes to hash * @param[in] hash_ctx pointer to the hashing context */ -void hash_nbytes(const uint8_t *const bytes_ptr, uint8_t n, cx_hash_t *hash_ctx) +void hash_nbytes(const uint8_t *const bytes_ptr, uint8_t n, cx_hash_t *const hash_ctx) { cx_hash(hash_ctx, 0, @@ -25,7 +25,7 @@ void hash_nbytes(const uint8_t *const bytes_ptr, uint8_t n, cx_hash_t *hash_ctx) * @param[in] byte byte to hash * @param[in] hash_ctx pointer to the hashing context */ -void hash_byte(uint8_t byte, cx_hash_t *hash_ctx) +void hash_byte(uint8_t byte, cx_hash_t *const hash_ctx) { hash_nbytes(&byte, 1, hash_ctx); } diff --git a/src_features/signMessageEIP712/path.c b/src_features/signMessageEIP712/path.c index fae422b..746dda0 100644 --- a/src_features/signMessageEIP712/path.c +++ b/src_features/signMessageEIP712/path.c @@ -71,7 +71,7 @@ static const void *get_nth_field(uint8_t *const fields_count_ptr, } /** - * Get the element the path is pointing to. (internal) + * Get the element the path is pointing to. * * @param[out] the number of fields in the depth of the returned field * @return the field which the path points to @@ -81,11 +81,23 @@ static inline const void *get_field(uint8_t *const fields_count) return get_nth_field(fields_count, path_struct->depth_count); } +/** + * Get Nth struct field from path + * + * @param[in] n nth depth requested + * @return pointer to the matching field, \ref NULL otherwise + */ const void *path_get_nth_field(uint8_t n) { return get_nth_field(NULL, n); } +/** + * Get Nth to last struct field from path + * + * @param[in] n nth to last depth requested + * @return pointer to the matching field, \ref NULL otherwise + */ const void *path_get_nth_field_to_last(uint8_t n) { const char *typename; @@ -103,7 +115,7 @@ const void *path_get_nth_field_to_last(uint8_t n) } /** - * Get the element the path is pointing to. (public facing) + * Get the element the path is pointing to * * @return the field which the path points to */ @@ -132,11 +144,21 @@ static bool path_depth_list_push(void) return true; } +/** + * Get the last hashing context (corresponding to the current path depth) + * + * @return pointer to the hashing context + */ static cx_sha3_t *get_last_hash_ctx(void) { return (cx_sha3_t*)mem_alloc(0) - 1; } +/** + * Finalize the last hashing context + * + * @param[out] hash pointer to buffer where the hash will be stored + */ static void finalize_hash_depth(uint8_t *hash) { const cx_sha3_t *hash_ctx; @@ -152,7 +174,12 @@ static void finalize_hash_depth(uint8_t *hash) mem_dealloc(sizeof(*hash_ctx)); // remove hash context } -static void feed_last_hash_depth(uint8_t *hash) +/** + * Continue last progressive hashing context with given hash + * + * @param[in] hash pointer to given hash + */ +static void feed_last_hash_depth(const uint8_t *const hash) { const cx_sha3_t *hash_ctx; @@ -166,6 +193,12 @@ static void feed_last_hash_depth(uint8_t *hash) 0); } +/** + * Create a new hashing context depth in memory + * + * @param[in] init if the hashing context should be initialized + * @return whether the memory allocation of the hashing context was successful + */ static bool push_new_hash_depth(bool init) { cx_sha3_t *hash_ctx; @@ -450,6 +483,7 @@ static bool check_and_add_array_depth(const void *depth, /** * Add a new array depth with a given size (number of elements). * + * @param[in] size number of elements * @return whether the add was successful or not */ bool path_new_array_depth(uint8_t size) @@ -609,6 +643,11 @@ bool path_advance(void) return true; } +/** + * Get root structure type from path (domain or message) + * + * @return enum representing root type + */ e_root_type path_get_root_type(void) { if (path_struct == NULL) @@ -618,6 +657,11 @@ e_root_type path_get_root_type(void) return path_struct->root_type; } +/** + * Get root structure from path + * + * @return pointer to the root structure definition + */ const void *path_get_root(void) { if (path_struct == NULL) @@ -627,6 +671,11 @@ const void *path_get_root(void) return path_struct->root_struct; } +/** + * Get the current amount of depth + * + * @return depth count + */ uint8_t path_get_depth_count(void) { if (path_struct == NULL) @@ -637,7 +686,7 @@ uint8_t path_get_depth_count(void) } /** - * Allocates the path indexes in memory and sets it with a depth of 0. + * Initialize the path context with its indexes in memory and sets it with a depth of 0. * * @return whether the memory allocation were successful. */ @@ -645,12 +694,21 @@ bool path_init(void) { if (path_struct == NULL) { - apdu_response_code = APDU_RESPONSE_INSUFFICIENT_MEMORY; - path_struct = MEM_ALLOC_AND_ALIGN_TYPE(*path_struct); + if ((path_struct = MEM_ALLOC_AND_ALIGN_TYPE(*path_struct)) == NULL) + { + apdu_response_code = APDU_RESPONSE_INSUFFICIENT_MEMORY; + } + else + { + path_struct->depth_count = 0; + } } return path_struct != NULL; } +/** + * De-initialize the path context + */ void path_deinit(void) { path_struct = NULL; diff --git a/src_features/signMessageEIP712/schema_hash.c b/src_features/signMessageEIP712/schema_hash.c index 004b464..a6cd3c0 100644 --- a/src_features/signMessageEIP712/schema_hash.c +++ b/src_features/signMessageEIP712/schema_hash.c @@ -6,6 +6,19 @@ #include "format_hash_field_type.h" #include "context.h" +// the SDK does not define a SHA-224 type, define it here so it's easier +// to understand in the code +typedef cx_sha256_t cx_sha224_t; + +/** + * Compute the schema hash + * + * The schema hash is the value of the root field "types" in the JSON data, + * stripped of all its spaces and newlines. This function reconstructs the JSON syntax + * from the stored typed data. + * + * @return whether the schema hash was successful or not + */ bool compute_schema_hash(void) { const void *struct_ptr; @@ -14,7 +27,7 @@ bool compute_schema_hash(void) uint8_t fields_count; const char *name; uint8_t name_length; - cx_sha256_t hash_ctx; // sha224 + cx_sha224_t hash_ctx; cx_sha224_init(&hash_ctx); diff --git a/src_features/signMessageEIP712/sol_typenames.c b/src_features/signMessageEIP712/sol_typenames.c index ec9fa53..44d1687 100644 --- a/src_features/signMessageEIP712/sol_typenames.c +++ b/src_features/signMessageEIP712/sol_typenames.c @@ -21,14 +21,21 @@ enum IDX_COUNT }; -static bool find_enum_matches(const uint8_t enum_to_idx[TYPES_COUNT - 1][IDX_COUNT], uint8_t s_idx) +/** + * Find a match between a typename index and all the type enums associated to it + * + * @param[in] enum_to_idx the type enum to typename index table + * @param[in] t_idx typename index + * @return whether at least one match was found + */ +static bool find_enum_matches(const uint8_t enum_to_idx[TYPES_COUNT - 1][IDX_COUNT], uint8_t t_idx) { uint8_t *enum_match = NULL; // loop over enum/typename pairs for (uint8_t e_idx = 0; e_idx < (TYPES_COUNT - 1); ++e_idx) { - if (s_idx == enum_to_idx[e_idx][IDX_STR_IDX]) // match + if (t_idx == enum_to_idx[e_idx][IDX_STR_IDX]) // match { if (enum_match != NULL) // in case of a previous match, mark it { @@ -45,6 +52,11 @@ static bool find_enum_matches(const uint8_t enum_to_idx[TYPES_COUNT - 1][IDX_COU return (enum_match != NULL); } +/** + * Initialize solidity typenames in memory + * + * @return whether the initialization went well or not + */ bool sol_typenames_init(void) { const char *const typenames[] = { @@ -74,10 +86,10 @@ bool sol_typenames_init(void) } *(sol_typenames) = 0; // loop over typenames - for (uint8_t s_idx = 0; s_idx < ARRAY_SIZE(typenames); ++s_idx) + for (uint8_t t_idx = 0; t_idx < ARRAY_SIZE(typenames); ++t_idx) { // if at least one match was found - if (find_enum_matches(enum_to_idx, s_idx)) + if (find_enum_matches(enum_to_idx, t_idx)) { if ((typename_len_ptr = mem_alloc(sizeof(uint8_t))) == NULL) { @@ -85,7 +97,7 @@ bool sol_typenames_init(void) return false; } // get pointer to the allocated space just above - *typename_len_ptr = strlen(PIC(typenames[s_idx])); + *typename_len_ptr = strlen(PIC(typenames[t_idx])); if ((typename_ptr = mem_alloc(sizeof(char) * *typename_len_ptr)) == NULL) { @@ -93,7 +105,7 @@ bool sol_typenames_init(void) return false; } // copy typename - memcpy(typename_ptr, PIC(typenames[s_idx]), *typename_len_ptr); + memcpy(typename_ptr, PIC(typenames[t_idx]), *typename_len_ptr); } // increment array size *(sol_typenames) += 1; @@ -102,6 +114,7 @@ bool sol_typenames_init(void) } /** + * Get typename from a given field * * @param[in] field_ptr pointer to a struct field * @param[out] length length of the returned typename diff --git a/src_features/signMessageEIP712/type_hash.c b/src_features/signMessageEIP712/type_hash.c index b0f32d6..67e54b8 100644 --- a/src_features/signMessageEIP712/type_hash.c +++ b/src_features/signMessageEIP712/type_hash.c @@ -14,6 +14,7 @@ #include "typed_data.h" /** + * Encode & hash the given structure field * * @param[in] field_ptr pointer to the struct field * @return \ref true it finished correctly, \ref false if it didn't (memory allocation) @@ -37,6 +38,7 @@ static bool encode_and_hash_field(const void *const field_ptr) } /** + * Encode & hash the a given structure type * * @param[in] struct_ptr pointer to the structure we want the typestring of * @param[in] str_length length of the formatted string in memory @@ -79,7 +81,7 @@ static bool encode_and_hash_type(const void *const struct_ptr) } /** - * + * Sort the given structs based by alphabetical order * * @param[in] deps_count count of how many struct dependencies pointers * @param[in,out] deps pointer to the first dependency pointer @@ -116,7 +118,7 @@ static void sort_dependencies(uint8_t deps_count, } /** - * + * Find all the dependencies from a given structure * * @param[out] deps_count count of how many struct dependencie pointers * @param[in] first_dep pointer to the first dependency pointer @@ -178,7 +180,7 @@ static const void **get_struct_dependencies(uint8_t *const deps_count, } /** - * + * Encode the structure's type and hash it * * @param[in] struct_name name of the given struct * @param[in] struct_name_length length of the name of the given struct diff --git a/src_features/signMessageEIP712/typed_data.c b/src_features/signMessageEIP712/typed_data.c index d9f52fd..54c4838 100644 --- a/src_features/signMessageEIP712/typed_data.c +++ b/src_features/signMessageEIP712/typed_data.c @@ -11,7 +11,11 @@ static s_typed_data *typed_data = NULL; - +/** + * Initialize the typed data context + * + * @return whether the memory allocation was successful or not + */ bool typed_data_init(void) { if (typed_data == NULL) @@ -39,14 +43,29 @@ void typed_data_deinit(void) typed_data = NULL; } -// lib functions -static const uint8_t *field_skip_typedesc(const uint8_t *field_ptr, const uint8_t *ptr) +/** + * Skip TypeDesc from a structure field + * + * @param[in] field_ptr pointer to the beginning of the struct field + * @param[in] ptr pointer to the current location within the struct field + * @return pointer to the data right after + */ +static const uint8_t *field_skip_typedesc(const uint8_t *field_ptr, + const uint8_t *ptr) { (void)ptr; return field_ptr + sizeof(typedesc_t); } -static const uint8_t *field_skip_typename(const uint8_t *field_ptr, const uint8_t *ptr) +/** + * Skip the type name from a structure field + * + * @param[in] field_ptr pointer to the beginning of the struct field + * @param[in] ptr pointer to the current location within the struct field + * @return pointer to the data right after + */ +static const uint8_t *field_skip_typename(const uint8_t *field_ptr, + const uint8_t *ptr) { uint8_t size; @@ -58,7 +77,15 @@ static const uint8_t *field_skip_typename(const uint8_t *field_ptr, const uint8_ return ptr; } -static const uint8_t *field_skip_typesize(const uint8_t *field_ptr, const uint8_t *ptr) +/** + * Skip the type size from a structure field + * + * @param[in] field_ptr pointer to the beginning of the struct field + * @param[in] ptr pointer to the current location within the struct field + * @return pointer to the data right after + */ +static const uint8_t *field_skip_typesize(const uint8_t *field_ptr, + const uint8_t *ptr) { if (struct_field_has_typesize(field_ptr)) { @@ -67,7 +94,15 @@ static const uint8_t *field_skip_typesize(const uint8_t *field_ptr, const uint8_ return ptr; } -static const uint8_t *field_skip_array_levels(const uint8_t *field_ptr, const uint8_t *ptr) +/** + * Skip the array levels from a structure field + * + * @param[in] field_ptr pointer to the beginning of the struct field + * @param[in] ptr pointer to the current location within the struct field + * @return pointer to the data right after + */ +static const uint8_t *field_skip_array_levels(const uint8_t *field_ptr, + const uint8_t *ptr) { uint8_t size; @@ -82,7 +117,15 @@ static const uint8_t *field_skip_array_levels(const uint8_t *field_ptr, const ui return ptr; } -static const uint8_t *field_skip_keyname(const uint8_t *field_ptr, const uint8_t *ptr) +/** + * Skip the key name from a structure field + * + * @param[in] field_ptr pointer to the beginning of the struct field + * @param[in] ptr pointer to the current location within the struct field + * @return pointer to the data right after + */ +static const uint8_t *field_skip_keyname(const uint8_t *field_ptr, + const uint8_t *ptr) { uint8_t size; @@ -91,6 +134,13 @@ static const uint8_t *field_skip_keyname(const uint8_t *field_ptr, const uint8_t return ptr + size; } +/** + * Get data pointer & array size from a given pointer + * + * @param[in] ptr given pointer + * @param[out] array_size pointer to array size + * @return pointer to data + */ const void *get_array_in_mem(const void *ptr, uint8_t *const array_size) { if (ptr == NULL) @@ -101,53 +151,91 @@ const void *get_array_in_mem(const void *ptr, uint8_t *const array_size) { *array_size = *(uint8_t*)ptr; } - return (ptr + 1); + return (ptr + sizeof(*array_size)); } +/** + * Get pointer to beginning of string & its length from a given pointer + * + * @param[in] ptr given pointer + * @param[out] string_length pointer to string length + * @return pointer to beginning of the string + */ const char *get_string_in_mem(const uint8_t *ptr, uint8_t *const string_length) { return (char*)get_array_in_mem(ptr, string_length); } -// ptr must point to the beginning of a struct field -static inline uint8_t get_struct_field_typedesc(const uint8_t *ptr) +/** + * Get the TypeDesc from a given struct field pointer + * + * @param[in] field_ptr struct field pointer + * @return TypeDesc + */ +static inline typedesc_t get_struct_field_typedesc(const uint8_t *const field_ptr) { - if (ptr == NULL) + if (field_ptr == NULL) { return 0; } - return *ptr; + return *field_ptr; } -// ptr must point to the beginning of a struct field -bool struct_field_is_array(const uint8_t *ptr) +/** + * Check whether a struct field is an array + * + * @param[in] field_ptr struct field pointer + * @return bool whether it is the case or not + */ +bool struct_field_is_array(const uint8_t *const field_ptr) { - return (get_struct_field_typedesc(ptr) & ARRAY_MASK); + return (get_struct_field_typedesc(field_ptr) & ARRAY_MASK); } -// ptr must point to the beginning of a struct field -bool struct_field_has_typesize(const uint8_t *ptr) +/** + * Check whether a struct field has a type size associated to it + * + * @param[in] field_ptr struct field pointer + * @return bool whether it is the case or not + */ +bool struct_field_has_typesize(const uint8_t *const field_ptr) { - return (get_struct_field_typedesc(ptr) & TYPESIZE_MASK); + return (get_struct_field_typedesc(field_ptr) & TYPESIZE_MASK); } -// ptr must point to the beginning of a struct field -e_type struct_field_type(const uint8_t *ptr) +/** + * Get type from a struct field + * + * @param[in] field_ptr struct field pointer + * @return its type enum + */ +e_type struct_field_type(const uint8_t *const field_ptr) { - return (get_struct_field_typedesc(ptr) & TYPE_MASK); + return (get_struct_field_typedesc(field_ptr) & TYPE_MASK); } -// ptr must point to the beginning of a struct field -uint8_t get_struct_field_typesize(const uint8_t *ptr) +/** + * Get type size from a struct field + * + * @param[in] field_ptr struct field pointer + * @return its type size + */ +uint8_t get_struct_field_typesize(const uint8_t *const field_ptr) { - if (ptr == NULL) + if (field_ptr == NULL) { return 0; } - return *(ptr + 1); + return *field_skip_typedesc(field_ptr, NULL); } -// ptr must point to the beginning of a struct field +/** + * Get custom type name from a struct field + * + * @param[in] field_ptr struct field pointer + * @param[out] length the type name length + * @return type name pointer + */ const char *get_struct_field_custom_typename(const uint8_t *field_ptr, uint8_t *const length) { @@ -161,7 +249,13 @@ const char *get_struct_field_custom_typename(const uint8_t *field_ptr, return get_string_in_mem(ptr, length); } -// ptr must point to the beginning of a struct field +/** + * Get type name from a struct field + * + * @param[in] field_ptr struct field pointer + * @param[out] length the type name length + * @return type name pointer + */ const char *get_struct_field_typename(const uint8_t *field_ptr, uint8_t *const length) { @@ -176,34 +270,51 @@ const char *get_struct_field_typename(const uint8_t *field_ptr, return get_struct_field_sol_typename(field_ptr, length); } -// ptr must point to the beginning of a depth level -e_array_type struct_field_array_depth(const uint8_t *ptr, +/** + * Get array type of a given struct field's array depth + * + * @param[in] array_depth_ptr given array depth + * @param[out] array_size pointer to array size + * @return array type of that depth + */ +e_array_type struct_field_array_depth(const uint8_t *array_depth_ptr, uint8_t *const array_size) { - if (ptr == NULL) + if (array_depth_ptr == NULL) { return 0; } - if (*ptr == ARRAY_FIXED_SIZE) + if (*array_depth_ptr == ARRAY_FIXED_SIZE) { - *array_size = *(ptr + 1); + if (array_size != NULL) + { + *array_size = *(array_depth_ptr + sizeof(uint8_t)); + } } - return *ptr; + return *array_depth_ptr; } -// ptr must point to the beginning of a struct field level -const uint8_t *get_next_struct_field_array_lvl(const uint8_t *ptr) +/** + * Get next array depth form a given struct field's array depth + * + * @param[in] array_depth_ptr given array depth + * @return next array depth + */ +const uint8_t *get_next_struct_field_array_lvl(const uint8_t *const array_depth_ptr) { - if (ptr == NULL) + const uint8_t *ptr; + + if (array_depth_ptr == NULL) { return NULL; } - switch (*ptr) + switch (*array_depth_ptr) { case ARRAY_DYNAMIC: + ptr = array_depth_ptr; break; case ARRAY_FIXED_SIZE: - ptr += 1; + ptr = array_depth_ptr + 1; break; default: // should not be in here :^) @@ -213,8 +324,14 @@ const uint8_t *get_next_struct_field_array_lvl(const uint8_t *ptr) return ptr + 1; } -// ptr must point to the beginning of a struct field -const uint8_t *get_struct_field_array_lvls_array(const uint8_t *field_ptr, +/** + * Get the array levels from a given struct field + * + * @param[in] field_ptr given struct field + * @param[out] length number of array levels + * @return pointer to the first array level + */ +const uint8_t *get_struct_field_array_lvls_array(const uint8_t *const field_ptr, uint8_t *const length) { const uint8_t *ptr; @@ -230,7 +347,13 @@ const uint8_t *get_struct_field_array_lvls_array(const uint8_t *field_ptr, return get_array_in_mem(ptr, length); } -// ptr must point to the beginning of a struct field +/** + * Get key name from a given struct field + * + * @param[in] field_ptr given struct field + * @param[out] length name length + * @return key name + */ const char *get_struct_field_keyname(const uint8_t *field_ptr, uint8_t *const length) { @@ -248,8 +371,13 @@ const char *get_struct_field_keyname(const uint8_t *field_ptr, return get_string_in_mem(ptr, length); } -// ptr must point to the beginning of a struct field -const uint8_t *get_next_struct_field(const void *field_ptr) +/** + * Get next struct field from a given field + * + * @param[in] field_ptr given struct field + * @return pointer to the next field + */ +const uint8_t *get_next_struct_field(const void *const field_ptr) { const void *ptr; @@ -265,8 +393,14 @@ const uint8_t *get_next_struct_field(const void *field_ptr) return field_skip_keyname(field_ptr, ptr); } -// ptr must point to the beginning of a struct -const char *get_struct_name(const uint8_t *struct_ptr, uint8_t *const length) +/** + * Get name from a given struct + * + * @param[in] struct_ptr given struct + * @param[out] length name length + * @return struct name + */ +const char *get_struct_name(const uint8_t *const struct_ptr, uint8_t *const length) { if (struct_ptr == NULL) { @@ -276,8 +410,14 @@ const char *get_struct_name(const uint8_t *struct_ptr, uint8_t *const length) return (char*)get_string_in_mem(struct_ptr, length); } -// ptr must point to the beginning of a struct -const uint8_t *get_struct_fields_array(const uint8_t *struct_ptr, +/** + * Get struct fields from a given struct + * + * @param[in] struct_ptr given struct + * @param[out] length name length + * @return struct name + */ +const uint8_t *get_struct_fields_array(const uint8_t *const struct_ptr, uint8_t *const length) { const void *ptr; @@ -294,8 +434,13 @@ const uint8_t *get_struct_fields_array(const uint8_t *struct_ptr, return get_array_in_mem(ptr, length); } -// ptr must point to the beginning of a struct -const uint8_t *get_next_struct(const uint8_t *struct_ptr) +/** + * Get next struct from a given struct + * + * @param[in] struct_ptr given struct + * @return pointer to next struct + */ +const uint8_t *get_next_struct(const uint8_t *const struct_ptr) { uint8_t fields_count; const void *ptr; @@ -313,22 +458,33 @@ const uint8_t *get_next_struct(const uint8_t *struct_ptr) return ptr; } -// ptr must point to the size of the structs array +/** + * Get structs array + * + * @param[out] length number of structs + * @return pointer to the first struct + */ const uint8_t *get_structs_array(uint8_t *const length) { return get_array_in_mem(typed_data->structs_array, length); } -// Finds struct with a given name -const uint8_t *get_structn(const char *const name_ptr, - const uint8_t name_length) +/** + * Find struct with a given name + * + * @param[in] name struct name + * @param[in] length name length + * @return pointer to struct + */ +const uint8_t *get_structn(const char *const name, + const uint8_t length) { uint8_t structs_count; const uint8_t *struct_ptr; - const char *name; - uint8_t length; + const char *struct_name; + uint8_t name_length; - if (name_ptr == NULL) + if (name == NULL) { apdu_response_code = APDU_RESPONSE_CONDITION_NOT_SATISFIED; return NULL; @@ -336,8 +492,8 @@ const uint8_t *get_structn(const char *const name_ptr, struct_ptr = get_structs_array(&structs_count); while (structs_count-- > 0) { - name = get_struct_name(struct_ptr, &length); - if ((name_length == length) && (memcmp(name, name_ptr, length) == 0)) + struct_name = get_struct_name(struct_ptr, &name_length); + if ((length == name_length) && (memcmp(name, struct_name, length) == 0)) { return struct_ptr; } @@ -346,8 +502,14 @@ const uint8_t *get_structn(const char *const name_ptr, apdu_response_code = APDU_RESPONSE_CONDITION_NOT_SATISFIED; return NULL; } -// +/** + * Set struct name + * + * @param[in] length name length + * @param[in] name name + * @return whether it was successful or not + */ bool set_struct_name(uint8_t length, const uint8_t *const name) { uint8_t *length_ptr; @@ -387,7 +549,15 @@ bool set_struct_name(uint8_t length, const uint8_t *const name) return true; } -static bool set_struct_field_custom(const uint8_t *const data, uint8_t *data_idx) +/** + * Set struct field custom typename + * + * @param[in] data the field data + * @param[in] data_idx the data index + * @return whether it was successful or not + */ +static bool set_struct_field_custom_typename(const uint8_t *const data, + uint8_t *data_idx) { uint8_t *typename_len_ptr; char *typename; @@ -411,6 +581,13 @@ static bool set_struct_field_custom(const uint8_t *const data, uint8_t *data_idx return true; } +/** + * Set struct field's array levels + * + * @param[in] data the field data + * @param[in] data_idx the data index + * @return whether it was successful or not + */ static bool set_struct_field_array(const uint8_t *const data, uint8_t *data_idx) { uint8_t *array_levels_count; @@ -452,6 +629,13 @@ static bool set_struct_field_array(const uint8_t *const data, uint8_t *data_idx) return true; } +/** + * Set struct field's type size + * + * @param[in] data the field data + * @param[in,out] data_idx the data index + * @return whether it was successful or not + */ static bool set_struct_field_typesize(const uint8_t *const data, uint8_t *data_idx) { uint8_t *type_size_ptr; @@ -466,6 +650,12 @@ static bool set_struct_field_typesize(const uint8_t *const data, uint8_t *data_i return true; } +/** + * Set struct field + * + * @param[in] data the field data + * @return whether it was successful or not + */ bool set_struct_field(const uint8_t *const data) { uint8_t data_idx = OFFSET_CDATA; @@ -499,7 +689,7 @@ bool set_struct_field(const uint8_t *const data) } else if ((*type_desc_ptr & TYPE_MASK) == TYPE_CUSTOM) { - if (set_struct_field_custom(data, &data_idx) == false) + if (set_struct_field_custom_typename(data, &data_idx) == false) { return false; } diff --git a/src_features/signMessageEIP712/typed_data.h b/src_features/signMessageEIP712/typed_data.h index d8eb1ab..06bc7d1 100644 --- a/src_features/signMessageEIP712/typed_data.h +++ b/src_features/signMessageEIP712/typed_data.h @@ -55,9 +55,9 @@ const char *get_struct_field_typename(const uint8_t *ptr, uint8_t *const length); e_array_type struct_field_array_depth(const uint8_t *ptr, uint8_t *const array_size); -const uint8_t *get_next_struct_field_array_lvl(const uint8_t *ptr); +const uint8_t *get_next_struct_field_array_lvl(const uint8_t *const ptr); const uint8_t *struct_field_half_skip(const uint8_t *ptr); -const uint8_t *get_struct_field_array_lvls_array(const uint8_t *ptr, +const uint8_t *get_struct_field_array_lvls_array(const uint8_t *const ptr, uint8_t *const length); const char *get_struct_field_keyname(const uint8_t *ptr, uint8_t *const length); diff --git a/src_features/signMessageEIP712/ui_logic.c b/src_features/signMessageEIP712/ui_logic.c index f5b19b7..54f3c2e 100644 --- a/src_features/signMessageEIP712/ui_logic.c +++ b/src_features/signMessageEIP712/ui_logic.c @@ -49,6 +49,15 @@ static bool ui_712_field_shown(void) return ret; } +/** + * Set UI buffer + * + * @param[in] src source buffer + * @param[in] src_length source buffer size + * @param[in] dst destination buffer + * @param[in] dst_length destination buffer length + * @param[in] explicit_trunc if truncation should be explicitely shown + */ static void ui_712_set_buf(const char *const src, size_t src_length, char *const dst, @@ -73,6 +82,9 @@ static void ui_712_set_buf(const char *const src, } } +/** + * Skip the field if needed and reset its UI flags + */ void ui_712_finalize_field(void) { if (!ui_712_field_shown()) @@ -104,6 +116,9 @@ void ui_712_set_value(const char *const str, uint8_t length) ui_712_set_buf(str, length, strings.tmp.tmp, sizeof(strings.tmp.tmp), true); } +/** + * Redraw the dynamic UI step that shows EIP712 information + */ void ui_712_redraw_generic_step(void) { if (!ui_ctx->shown) // Initialize if it is not already @@ -163,7 +178,7 @@ void ui_712_next_field(void) /** * Used to notify of a new struct to review * - * @param[in] struct_ptr pointer to the structure + * @param[in] struct_ptr pointer to the structure to be shown */ void ui_712_review_struct(const void *const struct_ptr) { @@ -184,6 +199,9 @@ void ui_712_review_struct(const void *const struct_ptr) ui_712_redraw_generic_step(); } +/** + * Show the hash of the message on the generic UI step + */ void ui_712_message_hash(void) { const char *const title = "Message hash"; @@ -197,6 +215,13 @@ void ui_712_message_hash(void) ui_712_redraw_generic_step(); } +/** + * Format given data as a string representation of a boolean + * + * @param[in] data the data that needs formatting + * @param[in] length its length + * @return if the formatting was successful + */ static bool ui_712_format_bool(const uint8_t *const data, uint8_t length) { if (length != 1) @@ -215,6 +240,12 @@ static bool ui_712_format_bool(const uint8_t *const data, uint8_t length) return true; } +/** + * Format given data as a string representation of bytes + * + * @param[in] data the data that needs formatting + * @param[in] length its length + */ static void ui_712_format_bytes(const uint8_t *const data, uint8_t length) { snprintf(strings.tmp.tmp, @@ -231,6 +262,13 @@ static void ui_712_format_bytes(const uint8_t *const data, uint8_t length) } } +/** + * Format given data as a string representation of an integer + * + * @param[in] data the data that needs formatting + * @param[in] length its length + * @return if the formatting was successful + */ static bool ui_712_format_int(const uint8_t *const data, uint8_t length, const void *const field_ptr) @@ -290,6 +328,12 @@ static bool ui_712_format_int(const uint8_t *const data, return true; } +/** + * Format given data as a string representation of an unsigned integer + * + * @param[in] data the data that needs formatting + * @param[in] length its length + */ static void ui_712_format_uint(const uint8_t *const data, uint8_t length) { uint256_t value256; @@ -435,6 +479,7 @@ unsigned int ui_712_approve(const bagl_element_t *e) /** * Reject button handling, calls the common handler function then * deinitializes the EIP712 context altogether. + * @param[in] e unused here, just needed to match the UI function signature * @return unused here, just needed to match the UI function signature */ @@ -445,6 +490,12 @@ unsigned int ui_712_reject(const bagl_element_t *e) return 0; } +/** + * Set a structure field's UI flags + * + * @param[in] show if this field should be shown on the device + * @param[in] name_provided if a substitution name has been provided + */ void ui_712_flag_field(bool show, bool name_provided) { if (show) @@ -457,21 +508,39 @@ void ui_712_flag_field(bool show, bool name_provided) } } +/** + * Set the UI filtering mode + * + * @param[in] the new filtering mode + */ void ui_712_set_filtering_mode(e_eip712_filtering_mode mode) { ui_ctx->filtering_mode = mode; } +/** + * Get the UI filtering mode + * + * @return current filtering mode + */ e_eip712_filtering_mode ui_712_get_filtering_mode(void) { return ui_ctx->filtering_mode; } +/** + * Reset all the UI struct field flags + */ void ui_712_field_flags_reset(void) { ui_ctx->field_flags = 0; } +/** + * Add a struct to the UI review queue + * + * Makes it so the user will have to go through a "Review struct" screen + */ void ui_712_queue_struct_to_review(void) { if (N_storage.verbose_eip712)