Skip to content

Commit 5c4703c

Browse files
aloisklinkmatejcik
aloisklink
authored and
matejcik
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 b2136e3 commit 5c4703c

File tree

15 files changed

+163
-45
lines changed

15 files changed

+163
-45
lines changed

‎common/protob/messages-ethereum.proto

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ message EthereumVerifyMessage {
159159
message EthereumSignTypedHash {
160160
repeated uint32 address_n = 1; // BIP-32 path to derive the key from master node
161161
required bytes domain_separator_hash = 2; // Hash of domainSeparator of typed data to be signed
162-
required bytes message_hash = 3; // Hash of the data of typed data to be signed
162+
optional bytes message_hash = 3; // Hash of the data of typed data to be signed (empty if domain-only data)
163163
}
164164

165165
/**

‎common/tests/fixtures/ethereum/sign_typed_data.json

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,77 @@
44
"passphrase": ""
55
},
66
"tests": [
7+
{
8+
"name": "bare_minimum",
9+
"comment": "Bare minimum EIP-712 message (domain only)",
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": null,
22+
"domain_separator_hash": "0x6192106f129ce05c9075d319c1fa6ea9b3ae37cbd0c1ef92e2be7137bb07baa1"
23+
},
24+
"result": {
25+
"address": "0x73d0385F4d8E00C5e6504C6030F47BF6212736A8",
26+
"sig": "0x18aaea9abed7cd88d3763a9a420e2e7b71a9f991685fbc62d74da86326cffa680644862d459d1973e422777a3933bc74190b1cae9a5418ddaea645a7d7630dd91c"
27+
}
28+
},
29+
{
30+
"name": "full_domain_empty_message",
31+
"comment": "Domain only EIP-712 message",
32+
"parameters": {
33+
"path": "m/44'/60'/0'/0/0",
34+
"metamask_v4_compat": true,
35+
"data": {
36+
"types": {
37+
"EIP712Domain": [
38+
{
39+
"name": "name",
40+
"type": "string"
41+
},
42+
{
43+
"name": "version",
44+
"type": "string"
45+
},
46+
{
47+
"name": "chainId",
48+
"type": "uint256"
49+
},
50+
{
51+
"name": "verifyingContract",
52+
"type": "address"
53+
},
54+
{
55+
"name": "salt",
56+
"type": "bytes32"
57+
}
58+
]
59+
},
60+
"primaryType": "EIP712Domain",
61+
"message": {},
62+
"domain": {
63+
"name": "Trezor",
64+
"version": "Test v0.0.0",
65+
"chainId": 1,
66+
"verifyingContract": "0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC",
67+
"salt": "0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
68+
}
69+
},
70+
"message_hash": null,
71+
"domain_separator_hash": "0xf85aaf157e9a36dc6e12643fff450fdf8d98fd0d0e41c5b42bb1f7aae6c83388"
72+
},
73+
"result": {
74+
"address": "0x73d0385F4d8E00C5e6504C6030F47BF6212736A8",
75+
"sig": "0x98a3e66f738002da98c70b976ef131c11ed8b94aad872140574ed2a2d4a2bac53a9350e284994274f0a7ce1191cf79bf13f2f0d0a862dcf0dd86ad8141eb90dc1c"
76+
}
77+
},
778
{
879
"name": "basic_data",
980
"parameters": {

‎core/.changelog.d/2036.fixed

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix domain-only EIP-712 hashes (i.e. when `primaryType`=`EIP712Domain`)

‎core/src/apps/ethereum/layout.py

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from trezor.messages import EthereumFieldType, EthereumStructMember
77
from trezor.strings import format_amount, format_plural
88
from trezor.ui.layouts import (
9+
confirm_action,
910
confirm_address,
1011
confirm_amount,
1112
confirm_blob,
@@ -111,16 +112,27 @@ def require_confirm_data(ctx: Context, data: bytes, data_total: int) -> Awaitabl
111112
)
112113

113114

114-
async def confirm_hash(ctx: Context, message_hash: bytes) -> None:
115-
await confirm_blob(
115+
async def confirm_typed_data_final(ctx: Context) -> None:
116+
await confirm_action(
116117
ctx,
117-
"confirm_hash",
118-
title="Confirm hash",
119-
data="0x" + hexlify(message_hash).decode(),
118+
"confirm_typed_data_final",
119+
title="Confirm typed data",
120+
action="Really sign EIP-712 typed data?",
121+
verb="Hold to confirm",
120122
hold=True,
121123
)
122124

123125

126+
def confirm_empty_typed_message(ctx: Context) -> Awaitable[None]:
127+
return confirm_text(
128+
ctx,
129+
"confirm_empty_typed_message",
130+
title="Confirm message",
131+
data="",
132+
description="No message field",
133+
)
134+
135+
124136
async def should_show_domain(ctx: Context, name: bytes, version: bytes) -> bool:
125137
domain_name = decode_typed_data(name, "string")
126138
domain_version = decode_typed_data(version, "string")

‎core/src/apps/ethereum/sign_typed_data.py

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@
2020
from .helpers import address_from_bytes, get_type_name
2121
from .keychain import PATTERNS_ADDRESS, with_keychain_from_path
2222
from .layout import (
23-
confirm_hash,
23+
confirm_empty_typed_message,
24+
confirm_typed_data_final,
2425
confirm_typed_value,
2526
should_show_array,
2627
should_show_domain,
@@ -82,23 +83,30 @@ async def generate_typed_data_hash(
8283
parent_objects=["EIP712Domain"],
8384
)
8485

85-
show_message = await should_show_struct(
86-
ctx,
87-
description=primary_type,
88-
data_members=typed_data_envelope.types[primary_type].members,
89-
title="Confirm message",
90-
button_text="Show full message",
91-
)
92-
message_hash = await typed_data_envelope.hash_struct(
93-
primary_type=primary_type,
94-
member_path=[1],
95-
show_data=show_message,
96-
parent_objects=[primary_type],
97-
)
86+
# Setting the primary_type to "EIP712Domain" is technically in spec
87+
# In this case, we ignore the "message" part and only use the "domain" part
88+
# https://ethereum-magicians.org/t/eip-712-standards-clarification-primarytype-as-domaintype/3286
89+
if primary_type == "EIP712Domain":
90+
await confirm_empty_typed_message(ctx)
91+
message_hash = b""
92+
else:
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+
message_hash = await typed_data_envelope.hash_struct(
101+
primary_type=primary_type,
102+
member_path=[1],
103+
show_data=show_message,
104+
parent_objects=[primary_type],
105+
)
98106

99-
await confirm_hash(ctx, message_hash)
107+
await confirm_typed_data_final(ctx)
100108

101-
return keccak256(b"\x19" + b"\x01" + domain_separator + message_hash)
109+
return keccak256(b"\x19\x01" + domain_separator + message_hash)
102110

103111

104112
def get_hash_writer() -> HashWriter:

‎core/src/trezor/messages.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3234,14 +3234,14 @@ def is_type_of(cls, msg: protobuf.MessageType) -> TypeGuard["EthereumVerifyMessa
32343234
class EthereumSignTypedHash(protobuf.MessageType):
32353235
address_n: "list[int]"
32363236
domain_separator_hash: "bytes"
3237-
message_hash: "bytes"
3237+
message_hash: "bytes | None"
32383238

32393239
def __init__(
32403240
self,
32413241
*,
32423242
domain_separator_hash: "bytes",
3243-
message_hash: "bytes",
32443243
address_n: "list[int] | None" = None,
3244+
message_hash: "bytes | None" = None,
32453245
) -> None:
32463246
pass
32473247

‎legacy/firmware/.changelog.d/2036.fixed

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix domain-only EIP-712 hashes (i.e. when `primaryType`=`EIP712Domain`)

‎legacy/firmware/ethereum.c

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -976,23 +976,30 @@ int ethereum_message_verify(const EthereumVerifyMessage *msg) {
976976
return 0;
977977
}
978978

979+
/*
980+
* EIP-712 hashes might have no message_hash if primaryType="EIP712Domain".
981+
* In this case, set has_message_hash=false.
982+
*/
979983
static void ethereum_typed_hash(const uint8_t domain_separator_hash[32],
980984
const uint8_t message_hash[32],
981-
uint8_t hash[32]) {
985+
bool has_message_hash, uint8_t hash[32]) {
982986
struct SHA3_CTX ctx = {0};
983987
sha3_256_Init(&ctx);
984988
sha3_Update(&ctx, (const uint8_t *)"\x19\x01", 2);
985989
sha3_Update(&ctx, domain_separator_hash, 32);
986-
sha3_Update(&ctx, message_hash, 32);
990+
if (has_message_hash) {
991+
sha3_Update(&ctx, message_hash, 32);
992+
}
987993
keccak_Final(&ctx, hash);
988994
}
989995

990996
void ethereum_typed_hash_sign(const EthereumSignTypedHash *msg,
991997
const HDNode *node,
992998
EthereumTypedDataSignature *resp) {
993999
uint8_t hash[32] = {0};
1000+
9941001
ethereum_typed_hash(msg->domain_separator_hash.bytes, msg->message_hash.bytes,
995-
hash);
1002+
msg->has_message_hash, hash);
9961003

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

‎legacy/firmware/fsm_msg_ethereum.h

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,8 @@ void fsm_msgEthereumSignTypedHash(const EthereumSignTypedHash *msg) {
216216

217217
CHECK_PIN
218218

219-
if (msg->domain_separator_hash.size != 32 || msg->message_hash.size != 32) {
219+
if (msg->domain_separator_hash.size != 32 ||
220+
(msg->has_message_hash && msg->message_hash.size != 32)) {
220221
fsm_sendFailure(FailureType_Failure_DataError, _("Invalid hash length"));
221222
return;
222223
}
@@ -256,12 +257,17 @@ void fsm_msgEthereumSignTypedHash(const EthereumSignTypedHash *msg) {
256257
layoutHome();
257258
return;
258259
}
259-
layoutConfirmHash(&bmp_icon_warning, _("EIP-712 message hash"),
260-
msg->message_hash.bytes, 32);
261-
if (!protectButton(ButtonRequestType_ButtonRequest_Other, false)) {
262-
fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL);
263-
layoutHome();
264-
return;
260+
261+
// No message hash when setting primaryType="EIP712Domain"
262+
// https://ethereum-magicians.org/t/eip-712-standards-clarification-primarytype-as-domaintype/3286
263+
if (msg->has_message_hash) {
264+
layoutConfirmHash(&bmp_icon_warning, _("EIP-712 message hash"),
265+
msg->message_hash.bytes, 32);
266+
if (!protectButton(ButtonRequestType_ButtonRequest_Other, false)) {
267+
fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL);
268+
layoutHome();
269+
return;
270+
}
265271
}
266272

267273
ethereum_typed_hash_sign(msg, node, resp);

‎python/.changelog.d/2036.fixed

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Allow passing empty `message_hash` for domain-only EIP-712 hashes
2+
for Trezor T1 (i.e. when `primaryType`=`EIP712Domain`)

‎python/src/trezorlib/cli/ethereum.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -455,10 +455,12 @@ def sign_typed_data_hash(
455455
Sign hash of typed data (EIP-712) with Ethereum address.
456456
457457
For T1 backward compatibility.
458+
459+
MESSAGE_HASH_HEX can be set to an empty string '' for domain-only hashes.
458460
"""
459461
address_n = tools.parse_path(address)
460462
domain_hash = ethereum.decode_hex(domain_hash_hex)
461-
message_hash = ethereum.decode_hex(message_hash_hex)
463+
message_hash = ethereum.decode_hex(message_hash_hex) if message_hash_hex else None
462464
ret = ethereum.sign_typed_data_hash(client, address_n, domain_hash, message_hash)
463465
output = {
464466
"domain_hash": domain_hash_hex,

‎python/src/trezorlib/ethereum.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,10 @@ def verify_message(
361361

362362
@expect(messages.EthereumTypedDataSignature)
363363
def sign_typed_data_hash(
364-
client: "TrezorClient", n: "Address", domain_hash: bytes, message_hash: bytes
364+
client: "TrezorClient",
365+
n: "Address",
366+
domain_hash: bytes,
367+
message_hash: Optional[bytes],
365368
) -> "MessageType":
366369
return client.call(
367370
messages.EthereumSignTypedHash(

‎python/src/trezorlib/messages.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4742,15 +4742,15 @@ class EthereumSignTypedHash(protobuf.MessageType):
47424742
FIELDS = {
47434743
1: protobuf.Field("address_n", "uint32", repeated=True, required=False),
47444744
2: protobuf.Field("domain_separator_hash", "bytes", repeated=False, required=True),
4745-
3: protobuf.Field("message_hash", "bytes", repeated=False, required=True),
4745+
3: protobuf.Field("message_hash", "bytes", repeated=False, required=False),
47464746
}
47474747

47484748
def __init__(
47494749
self,
47504750
*,
47514751
domain_separator_hash: "bytes",
4752-
message_hash: "bytes",
47534752
address_n: Optional[Sequence["int"]] = None,
4753+
message_hash: Optional["bytes"] = None,
47544754
) -> None:
47554755
self.address_n: Sequence["int"] = address_n if address_n is not None else []
47564756
self.domain_separator_hash = domain_separator_hash

‎tests/device_tests/ethereum/test_sign_typed_data.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,10 @@ def test_ethereum_sign_typed_data_blind(client, parameters, result):
5050
client,
5151
address_n,
5252
ethereum.decode_hex(parameters["domain_separator_hash"]),
53-
ethereum.decode_hex(parameters["message_hash"]),
53+
# message hash is empty for domain-only hashes
54+
ethereum.decode_hex(parameters["message_hash"])
55+
if parameters["message_hash"]
56+
else None,
5457
)
5558
assert ret.address == result["address"]
5659
assert f"0x{ret.signature.hex()}" == result["sig"]

‎tests/ui_tests/fixtures.json

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -602,13 +602,15 @@
602602
"ethereum-test_getpublickey.py::test_ethereum_getpublickey[parameters0-result0]": "095af81ec79e9b510c90d9fa34fed343f3840807190c67bc237af885695ae687",
603603
"ethereum-test_getpublickey.py::test_ethereum_getpublickey[parameters1-result1]": "095af81ec79e9b510c90d9fa34fed343f3840807190c67bc237af885695ae687",
604604
"ethereum-test_getpublickey.py::test_ethereum_getpublickey[parameters2-result2]": "095af81ec79e9b510c90d9fa34fed343f3840807190c67bc237af885695ae687",
605-
"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[basic_data]": "4f512d6beb0222079aaa878a2bcd2c41ac389fdd47edad3e0b0b5779677d8697",
606-
"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[complex_data]": "275a8630a795c966419f6fc6de834bb576bfc3dbc16a1fd7605aa2b8ceae666e",
607-
"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[struct_list_non_v4]": "7dd23b14bd273b937836a24cf056c745e7b3461139f895caefd4624e0d5545f5",
608-
"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[struct_list_v4]": "b7e3475d4906942bc0e8d62203ae91a13ea0d702c3a7a53b9777bea670c4a7f7",
609-
"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[structs_arrays_v4]": "31cc5b5c1d9d94f0761208f5dc6423d283b9118b12b87cf878d20fa144f4f252",
605+
"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[bare_minimum]": "e13b237f58a9977c6edf1917730748b881749acecc3d040b1fd9d794e6e02299",
606+
"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[basic_data]": "84f1d23e031919fedb4ef6b3f3dc1cb667836b9efcd0c5a0da586010e38e1fd6",
607+
"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[complex_data]": "27ee23894c19e5704d02ae684d50a0f0d14678d3f74a95e67fc4975818a1faa8",
608+
"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[full_domain_empty_message]": "c3c299087e9cac4d7554314abc8637d041da69f0c0ee2863c5526f1f89448ae2",
609+
"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[struct_list_non_v4]": "84f1d23e031919fedb4ef6b3f3dc1cb667836b9efcd0c5a0da586010e38e1fd6",
610+
"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[struct_list_v4]": "84f1d23e031919fedb4ef6b3f3dc1cb667836b9efcd0c5a0da586010e38e1fd6",
611+
"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[structs_arrays_v4]": "84f1d23e031919fedb4ef6b3f3dc1cb667836b9efcd0c5a0da586010e38e1fd6",
610612
"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data_cancel": "08712efae2d007610289bbfb3a8fe6800547e884636c83c5bf0e25f33728789e",
611-
"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data_show_more_button": "1adbea797586685ce09aae58b0a2b89e1617e4eaad23a8c1ac6fc10b041e57a5",
613+
"ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data_show_more_button": "72959d95278c136cb2071f6452b2a61782b9ac3e6946d8fb5f8db8f153171e6f",
612614
"ethereum-test_sign_verify_message.py::test_signmessage[parameters0-result0]": "7aa14b29e5005d8fdc0a8b497ed5d3ebea15c7017f9c457d09214f2d05fbc532",
613615
"ethereum-test_sign_verify_message.py::test_signmessage[parameters1-result1]": "c5fb9393267c3d9b9bf5839aab6c641d3931286411f291cd1d8e937cb224ae2d",
614616
"ethereum-test_sign_verify_message.py::test_signmessage[parameters2-result2]": "8499b87474becc06010e9b4356398a3e29ee2ef152e04653f15dcc227fc486d6",

0 commit comments

Comments
 (0)