Skip to content

Commit 12c0677

Browse files
committed
fix(core,legacy): Fix domain-only ethTypedData
When doing Ethereum signTypedData, and the primaryType="EIP712Domain", we completely ignore the "message" part and only sign the domain. According to the community, this is technically allowed by the spec, and may be used by ETH smart contracts to save on gas. Test case generated by @MetaMask/eth-sig-util's library. See: https://ethereum-magicians.org/t/eip-712-standards-clarification-primarytype-as-domaintype/3286
1 parent c0510fc commit 12c0677

File tree

4 files changed

+84
-28
lines changed

4 files changed

+84
-28
lines changed

common/tests/fixtures/ethereum/sign_typed_data.json

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,28 @@
44
"passphrase": ""
55
},
66
"tests": [
7+
{
8+
"name": "empty_message",
9+
"comment": "Bare minimum valid EIP-712 signature",
10+
"parameters": {
11+
"path": "m/44'/60'/0'/0/0",
12+
"metamask_v4_compat": true,
13+
"data": {
14+
"types": {
15+
"EIP712Domain": []
16+
},
17+
"primaryType": "EIP712Domain",
18+
"message": {},
19+
"domain": {}
20+
},
21+
"message_hash": "0x",
22+
"domain_separator_hash": "0x6192106f129ce05c9075d319c1fa6ea9b3ae37cbd0c1ef92e2be7137bb07baa1"
23+
},
24+
"result": {
25+
"address": "0x73d0385F4d8E00C5e6504C6030F47BF6212736A8",
26+
"sig": "0x18aaea9abed7cd88d3763a9a420e2e7b71a9f991685fbc62d74da86326cffa680644862d459d1973e422777a3933bc74190b1cae9a5418ddaea645a7d7630dd91c"
27+
}
28+
},
729
{
830
"name": "basic_data",
931
"parameters": {

core/src/apps/ethereum/sign_typed_data.py

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -71,32 +71,46 @@ async def generate_typed_data_hash(
7171
)
7272
await typed_data_envelope.collect_types()
7373

74+
# EIP-712 prefix
75+
parts = [b"\x19\x01"]
76+
7477
name, version = await get_name_and_version_for_domain(ctx, typed_data_envelope)
7578
show_domain = await should_show_domain(ctx, name, version)
76-
domain_separator = await typed_data_envelope.hash_struct(
77-
primary_type="EIP712Domain",
78-
member_path=[0],
79-
show_data=show_domain,
80-
parent_objects=["EIP712Domain"],
79+
# domain_seperator
80+
parts.append(
81+
await typed_data_envelope.hash_struct(
82+
primary_type="EIP712Domain",
83+
member_path=[0],
84+
show_data=show_domain,
85+
parent_objects=["EIP712Domain"],
86+
)
8187
)
8288

83-
show_message = await should_show_struct(
84-
ctx,
85-
description=primary_type,
86-
data_members=typed_data_envelope.types[primary_type].members,
87-
title="Confirm message",
88-
button_text="Show full message",
89-
)
90-
message_hash = await typed_data_envelope.hash_struct(
91-
primary_type=primary_type,
92-
member_path=[1],
93-
show_data=show_message,
94-
parent_objects=[primary_type],
95-
)
89+
# Setting the primary_type to "EIP712Domain" is technically in spec
90+
# In this case, we ignore the "message" part and only use the "domain" part
91+
# https://ethereum-magicians.org/t/eip-712-standards-clarification-primarytype-as-domaintype/3286
92+
if primary_type != "EIP712Domain":
93+
show_message = await should_show_struct(
94+
ctx,
95+
description=primary_type,
96+
data_members=typed_data_envelope.types[primary_type].members,
97+
title="Confirm message",
98+
button_text="Show full message",
99+
)
100+
parts.append(
101+
await typed_data_envelope.hash_struct(
102+
primary_type=primary_type,
103+
member_path=[1],
104+
show_data=show_message,
105+
parent_objects=[primary_type],
106+
)
107+
)
108+
109+
full_bytes_to_hash = b"".join(parts)
96110

97-
await confirm_hash(ctx, message_hash)
111+
await confirm_hash(ctx, full_bytes_to_hash)
98112

99-
return keccak256(b"\x19" + b"\x01" + domain_separator + message_hash)
113+
return keccak256(full_bytes_to_hash)
100114

101115

102116
def get_hash_writer() -> HashWriter:

legacy/firmware/ethereum.c

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -987,12 +987,27 @@ static void ethereum_typed_hash(const uint8_t domain_separator_hash[32],
987987
keccak_Final(&ctx, hash);
988988
}
989989

990+
// When primaryType="EIP712Domain", we ignore message_hash completely
991+
static void ethereum_domain_only_typed_hash(
992+
const uint8_t domain_separator_hash[32], uint8_t hash[32]) {
993+
struct SHA3_CTX ctx = {0};
994+
sha3_256_Init(&ctx);
995+
sha3_Update(&ctx, (const uint8_t *)"\x19\x01", 2);
996+
sha3_Update(&ctx, domain_separator_hash, 32);
997+
keccak_Final(&ctx, hash);
998+
}
999+
9901000
void ethereum_typed_hash_sign(const EthereumSignTypedHash *msg,
9911001
const HDNode *node,
9921002
EthereumTypedDataSignature *resp) {
9931003
uint8_t hash[32] = {0};
994-
ethereum_typed_hash(msg->domain_separator_hash.bytes, msg->message_hash.bytes,
995-
hash);
1004+
1005+
if (msg->message_hash.size) {
1006+
ethereum_typed_hash(msg->domain_separator_hash.bytes,
1007+
msg->message_hash.bytes, hash);
1008+
} else {
1009+
ethereum_domain_only_typed_hash(msg->domain_separator_hash.bytes, hash);
1010+
}
9961011

9971012
uint8_t v = 0;
9981013
if (ecdsa_sign_digest(&secp256k1, node->private_key, hash,

legacy/firmware/fsm_msg_ethereum.h

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -262,12 +262,17 @@ void fsm_msgEthereumSignTypedHash(const EthereumSignTypedHash *msg) {
262262
layoutHome();
263263
return;
264264
}
265-
layoutConfirmHash(&bmp_icon_warning, _("EIP-712 message hash"),
266-
msg->message_hash.bytes, 32);
267-
if (!protectButton(ButtonRequestType_ButtonRequest_Other, false)) {
268-
fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL);
269-
layoutHome();
270-
return;
265+
266+
// No message hash when setting primaryType="EIP712Domain"
267+
// https://ethereum-magicians.org/t/eip-712-standards-clarification-primarytype-as-domaintype/3286
268+
if (msg->message_hash.size) {
269+
layoutConfirmHash(&bmp_icon_warning, _("EIP-712 message hash"),
270+
msg->message_hash.bytes, 32);
271+
if (!protectButton(ButtonRequestType_ButtonRequest_Other, false)) {
272+
fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL);
273+
layoutHome();
274+
return;
275+
}
271276
}
272277

273278
ethereum_typed_hash_sign(msg, node, resp);

0 commit comments

Comments
 (0)