Skip to content

Commit 9efa84e

Browse files
franco-gondifranco-gondi
andauthored
error eip712 signature with nested fields (#905)
* error signature * make test more explicit * fix: hash ctx stack in arrays * fix: build * chore: removed whitespace * fix: format --------- Co-authored-by: franco-gondi <[email protected]>
1 parent a5b57b7 commit 9efa84e

File tree

2 files changed

+95
-13
lines changed

2 files changed

+95
-13
lines changed

src/features/signMessageEIP712/path.c

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,21 @@ cx_sha3_t *get_last_hash_ctx(void) {
156156
return &((s_hash_ctx *) hash_ctx)->hash;
157157
}
158158

159+
/**
160+
* Get the previous hashing context before the given one
161+
*
162+
* @return pointer to the hashing context
163+
*/
164+
cx_sha3_t *get_previous_hash_ctx(cx_sha3_t *hash_ctx) {
165+
cx_sha3_t *prev_ctx = NULL;
166+
// TODO: using a doubly-linked list would improve this
167+
for (s_hash_ctx *tmp = g_hash_ctxs; &tmp->hash != hash_ctx;
168+
tmp = (s_hash_ctx *) ((s_flist_node *) tmp)->next) {
169+
prev_ctx = &tmp->hash;
170+
}
171+
return prev_ctx;
172+
}
173+
159174
// to be used as a \ref f_list_node_del
160175
static void delete_hash_ctx(s_hash_ctx *ctx) {
161176
app_mem_free(ctx);
@@ -510,6 +525,7 @@ bool path_new_array_depth(const uint8_t *data, uint8_t length) {
510525
uint8_t array_size;
511526
uint8_t array_depth_count_bak;
512527
cx_err_t error = CX_INTERNAL_ERROR;
528+
cx_sha3_t *start_hash_ctx = get_last_hash_ctx();
513529

514530
if (path_struct == NULL) {
515531
apdu_response_code = SWO_INCORRECT_DATA;
@@ -561,22 +577,21 @@ bool path_new_array_depth(const uint8_t *data, uint8_t length) {
561577
}
562578
if (is_custom) {
563579
cx_sha3_t *hash_ctx = get_last_hash_ctx();
564-
cx_sha3_t *old_ctx = NULL;
565-
566-
// TODO: using a doubly-linked list would improve this
567-
for (s_hash_ctx *tmp = g_hash_ctxs; &tmp->hash != hash_ctx;
568-
tmp = (s_hash_ctx *) ((s_flist_node *) tmp)->next) {
569-
old_ctx = &tmp->hash;
570-
}
580+
cx_sha3_t *prev_ctx = get_previous_hash_ctx(hash_ctx);
581+
while (prev_ctx != start_hash_ctx) {
582+
if (prev_ctx == NULL) return false;
571583

572-
if (old_ctx == NULL) return false;
584+
if (array_size > 0) {
585+
memcpy(hash_ctx, prev_ctx, sizeof(*prev_ctx));
586+
} else {
587+
CX_CHECK(cx_keccak_init_no_throw(hash_ctx, 256));
588+
}
589+
CX_CHECK(cx_keccak_init_no_throw(prev_ctx, 256));
573590

574-
if (array_size > 0) {
575-
memcpy(hash_ctx, old_ctx, sizeof(*old_ctx));
576-
} else {
577-
CX_CHECK(cx_keccak_init_no_throw(hash_ctx, 256));
591+
hash_ctx = prev_ctx;
592+
prev_ctx = get_previous_hash_ctx(hash_ctx);
578593
}
579-
CX_CHECK(cx_keccak_init_no_throw(old_ctx, 256));
594+
CX_CHECK(cx_keccak_init_no_throw(hash_ctx, 256));
580595
}
581596
if (array_size == 0) {
582597
do {

tests/ragger/test_eip712.py

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -791,3 +791,70 @@ def test_eip712_calldata_empty_send(scenario_navigator: NavigateWithScenario, te
791791

792792
def test_eip712_calldata_no_param(scenario_navigator: NavigateWithScenario, test_name: str):
793793
eip712_calldata_common(scenario_navigator, test_name, "safe_calldata_no_param", gcs_handler_no_param)
794+
795+
796+
def test_eip712_payload(scenario_navigator: NavigateWithScenario):
797+
"""Test a basic EIP712 payload signature"""
798+
799+
# Enable blind signing for EIP712
800+
settings_toggle(scenario_navigator.backend.device, scenario_navigator.navigator, [SettingID.BLIND_SIGNING])
801+
802+
# Define a simple EIP712 payload
803+
data = {
804+
"types": {
805+
"EIP712Domain": [
806+
{
807+
"name": "name",
808+
"type": "string"
809+
},
810+
{
811+
"name": "version",
812+
"type": "string"
813+
},
814+
{
815+
"name": "chainId",
816+
"type": "uint256"
817+
},
818+
{
819+
"name": "verifyingContract",
820+
"type": "address"
821+
}
822+
],
823+
"Root": [
824+
{
825+
"name": "child",
826+
"type": "Inner[]"
827+
},
828+
],
829+
"Inner": [
830+
{
831+
"name": "child",
832+
"type": "Leaf"
833+
},
834+
],
835+
"Leaf": [
836+
{
837+
"name": "value",
838+
"type": "uint256"
839+
},
840+
],
841+
},
842+
"primaryType": "Root",
843+
"domain": {
844+
"name": "DOMAIN",
845+
"version": "3.1",
846+
"chainId": 31337,
847+
"verifyingContract": "0x95401dc811bb5740090279ba06cfa8fcf6113778"
848+
},
849+
"message": {
850+
"child": [
851+
{
852+
"child": {
853+
"value": 2,
854+
},
855+
}
856+
],
857+
}
858+
}
859+
eip712_new_common(scenario_navigator, data, None, None, with_warning=True)
860+

0 commit comments

Comments
 (0)