Skip to content

Commit 732c33f

Browse files
committed
eth/signed_typed_msg: allow newlines in strings
Values of type string that had newlines were previously rejected, even though dApps use it in practice. A user reported this error while using Collab.land, wihch contained a many-line string message as part of the EIP712 message. We show all lines individually, similar to when signign a regular message. I chose not to skip over empty lines (like we do in signmsg) as it's more explicit and makes the code easier (no need to handle the case of an empty string). Not considered in this commit: strings that contian non-ascii chars - this continues to fail and could be fixed in another commit..
1 parent 8e1868c commit 732c33f

File tree

2 files changed

+38
-21
lines changed

2 files changed

+38
-21
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ customers cannot upgrade their bootloader, its changes are recorded separately.
77
## Firmware
88

99
### [Unreleased]
10+
- Ethereum: allow signing EIP-712 messages containing multi-line strings
1011

1112
### 9.18.0
1213
- Add support for deriving BIP-39 mnemonics according to BIP-85

src/rust/bitbox02-rust/src/hww/api/ethereum/sign_typed_msg.rs

Lines changed: 37 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ fn encode_value(typ: &MemberType, value: Vec<u8>) -> Result<(Vec<u8>, String), E
257257
)
258258
}
259259
DataType::String => {
260-
if !util::ascii::is_printable_ascii(&value, util::ascii::Charset::All) {
260+
if !util::ascii::is_printable_ascii(&value, util::ascii::Charset::AllNewline) {
261261
return Err(Error::InvalidInput);
262262
}
263263
(
@@ -312,18 +312,30 @@ async fn encode_member<U: sha3::digest::Update>(
312312
} else {
313313
let value = get_value_from_host(root_object, path).await?;
314314
let (value_encoded, value_formatted) = encode_value(member_type, value)?;
315-
confirm::confirm(&confirm::Params {
316-
title: &format!(
317-
"{}{}",
318-
confirm_title(root_object),
319-
title_suffix.as_deref().unwrap_or("")
320-
),
321-
body: &format!("{}: {}", formatted_path.join("."), value_formatted),
322-
scrollable: true,
323-
accept_is_nextarrow: true,
324-
..Default::default()
325-
})
326-
.await?;
315+
let lines: Vec<&str> = value_formatted.split('\n').collect();
316+
for (i, &line) in lines.iter().enumerate() {
317+
confirm::confirm(&confirm::Params {
318+
title: &format!(
319+
"{}{}",
320+
confirm_title(root_object),
321+
title_suffix.as_deref().unwrap_or("")
322+
),
323+
body: &format!(
324+
"{}{}: {}",
325+
formatted_path.join("."),
326+
if lines.len() > 1 {
327+
format!(", line {}/{}", i + 1, lines.len())
328+
} else {
329+
"".into()
330+
},
331+
line
332+
),
333+
scrollable: true,
334+
accept_is_nextarrow: true,
335+
..Default::default()
336+
})
337+
.await?;
338+
}
327339
hasher.update(&value_encoded);
328340
}
329341
Ok(())
@@ -1031,7 +1043,7 @@ mod tests {
10311043
/// str: 'str',
10321044
/// emptyArray: [],
10331045
/// name_address: '0xa21A16EC22a940990922220E4ab5bF4C2310F556',
1034-
/// name_string: ['', 'a', 'aa', '|@#!$', 'long long long long long long long long'],
1046+
/// name_string: ['', 'a', 'aa', '|@#!$', 'long long long long long long long long', 'multi\n\nline'],
10351047
/// name_bytes: ['', '0xaabbcc'],
10361048
/// name_bytes1: '0xaa',
10371049
/// name_bytes10: '0x112233445566778899aa',
@@ -1069,12 +1081,15 @@ mod tests {
10691081
("Message (1/23)", "str: str"),
10701082
("Message (2/23)", "emptyArray: (empty list)"),
10711083
("Message (3/23)", "name_address: 0xa21A16EC22a940990922220E4ab5bF4C2310F556"),
1072-
("Message (4/23)", "name_string: list with 5 elements"),
1073-
("Message (4/23)", "name_string[1/5]: "),
1074-
("Message (4/23)", "name_string[2/5]: a"),
1075-
("Message (4/23)", "name_string[3/5]: aa"),
1076-
("Message (4/23)", "name_string[4/5]: |@#!$"),
1077-
("Message (4/23)", "name_string[5/5]: long long long long long long long long"),
1084+
("Message (4/23)", "name_string: list with 6 elements"),
1085+
("Message (4/23)", "name_string[1/6]: "),
1086+
("Message (4/23)", "name_string[2/6]: a"),
1087+
("Message (4/23)", "name_string[3/6]: aa"),
1088+
("Message (4/23)", "name_string[4/6]: |@#!$"),
1089+
("Message (4/23)", "name_string[5/6]: long long long long long long long long"),
1090+
("Message (4/23)", "name_string[6/6], line 1/3: multi"),
1091+
("Message (4/23)", "name_string[6/6], line 2/3: "),
1092+
("Message (4/23)", "name_string[6/6], line 3/3: line"),
10781093
("Message (5/23)", "name_bytes: list with 2 elements"),
10791094
("Message (5/23)", "name_bytes[1/2]: 0x"),
10801095
("Message (5/23)", "name_bytes[2/2]: 0xaabbcc"),
@@ -1252,6 +1267,7 @@ mod tests {
12521267
Object::String("aa"),
12531268
Object::String("|@#!$"),
12541269
Object::String("long long long long long long long long"),
1270+
Object::String("multi\n\nline"),
12551271
]),
12561272
// name_bytes
12571273
Object::List(vec![Object::Bytes(b""), Object::Bytes(b"\xaa\xbb\xcc")]),
@@ -1404,7 +1420,7 @@ mod tests {
14041420
let sighash = block_on(eip712_sighash(&typed_msg.types, typed_msg.primary_type)).unwrap();
14051421
assert_eq!(
14061422
sighash,
1407-
*b"\xc5\x4f\xa7\x87\x13\x18\xb6\xc2\xd8\x71\x62\xde\xbe\xff\x4c\xdf\x13\xf2\x85\x45\x12\xf3\x43\x6a\x04\xa6\x0c\xd1\xa7\xcf\x47\xc5",
1423+
*b"\x0e\xfe\x31\xa8\x81\x9b\x6c\x38\x1c\x9e\x97\xcf\xd2\x99\x5a\xa6\xf2\x1e\x4a\x72\x87\x9a\xc1\x31\xb2\xf6\x48\xd0\x83\x28\x1c\x83",
14081424
);
14091425
assert_eq!(unsafe { UI_COUNTER }, EXPECTED_DIALOGS.len());
14101426
}

0 commit comments

Comments
 (0)