Skip to content

Commit 31c8486

Browse files
committed
eth: preserve signed integer encodings
EIP-712 intN values are sent as signed big-endian two's-complement bytes. The JSON-number path strips the leading zero supplied by the signed BigInt serializer, so e.g. int16(128) becomes -128 on the device. The string path already preserves the sign correctly. Use to_signed_bytes_be directly for JSON numbers as well, without stripping its sign byte. Add boundary cases for positive values, negative values and zero, checking that numeric and string inputs produce the same encoding. Add a simulator regression that signs int16(+128), int8(-128) and int8(0) and checks the signature against an independently calculated EIP-712 digest. This covers the signed values through the public API and firmware. Unsigned integer encoding and the firmware protocol are unchanged. No firmware version gate is needed. Related PRs: - Go: BitBoxSwiss/bitbox02-api-go#189 - TypeScript: BitBoxSwiss/bitbox-api-ts#14
1 parent 1b07ba4 commit 31c8486

3 files changed

Lines changed: 87 additions & 7 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Changelog
22

33
## [Unreleased]
4+
- Preserve the sign of positive EIP-712 signed integers supplied as JSON numbers.
45
- Reset unfinished sessions when connecting to firmware v9.28.0 or newer, allowing host
56
reconnects while the device remains powered on.
67
- Remove the `wasm` feature, the public `wasm` module, and the TypeScript/WASM package. The

‎src/eth.rs‎

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -333,12 +333,7 @@ fn encode_value(typ: &MemberType, value: &Value) -> Result<Vec<u8>, String> {
333333
if (v64 as f64) != v {
334334
Err("Number is not an int".to_string())
335335
} else {
336-
let mut bytes = BigInt::from(v64).to_signed_bytes_be();
337-
// Drop leading zero. There can be at most one.
338-
if let [0, ..] = bytes.as_slice() {
339-
bytes.remove(0);
340-
}
341-
Ok(bytes)
336+
Ok(BigInt::from(v64).to_signed_bytes_be())
342337
}
343338
} else {
344339
Err("Number is not an int".to_string())
@@ -1164,7 +1159,7 @@ mod tests {
11641159
&2983742332.0.into(),
11651160
)
11661161
.unwrap();
1167-
assert_eq!(vec![0xb1, 0xd8, 0x4b, 0x7c], encoded);
1162+
assert_eq!(vec![0x00, 0xb1, 0xd8, 0x4b, 0x7c], encoded);
11681163

11691164
let encoded = encode_value(
11701165
&parse_type_no_err("int64", &HashMap::new()),
@@ -1218,6 +1213,40 @@ mod tests {
12181213
assert_eq!(b"\x00\x00\x03\xe8".to_vec(), encoded);
12191214
}
12201215

1216+
#[test]
1217+
fn test_encode_value_signed_integers() {
1218+
let typ = parse_type_no_err("int64", &HashMap::new());
1219+
for (value, expected) in [
1220+
(0i64, "00"),
1221+
(1, "01"),
1222+
(-1, "ff"),
1223+
(127, "7f"),
1224+
(128, "0080"),
1225+
(129, "0081"),
1226+
(-127, "81"),
1227+
(-128, "80"),
1228+
(-129, "ff7f"),
1229+
(255, "00ff"),
1230+
(256, "0100"),
1231+
(-255, "ff01"),
1232+
(-256, "ff00"),
1233+
(32767, "7fff"),
1234+
(32768, "008000"),
1235+
(-32768, "8000"),
1236+
(-32769, "ff7fff"),
1237+
(65535, "00ffff"),
1238+
(65536, "010000"),
1239+
] {
1240+
for input in [Value::from(value), Value::from(value.to_string())] {
1241+
assert_eq!(
1242+
hex::encode(encode_value(&typ, &input).unwrap()),
1243+
expected,
1244+
"{input:?}",
1245+
);
1246+
}
1247+
}
1248+
}
1249+
12211250
#[test]
12221251
fn test_get_value() {
12231252
let msg: Eip712Message = serde_json::from_str(EIP712_MSG).unwrap();

‎tests/test_eth.rs‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,56 @@ async fn test_eth_sign_typed_message_antiklepto_enabled() {
290290
.await
291291
}
292292

293+
#[tokio::test]
294+
async fn test_eth_sign_typed_message_signed_integers() {
295+
test_initialized_simulators(async |paired_bitbox| {
296+
let msg = r#"{
297+
"types": {
298+
"EIP712Domain": [{"name": "name", "type": "string"}],
299+
"SignedIntegers": [
300+
{"name": "positive", "type": "int16"},
301+
{"name": "negative", "type": "int8"},
302+
{"name": "zero", "type": "int8"}
303+
]
304+
},
305+
"primaryType": "SignedIntegers",
306+
"domain": {"name": "Signed integers"},
307+
"message": {"positive": 128, "negative": -128, "zero": 0}
308+
}"#;
309+
let signature = paired_bitbox
310+
.eth_sign_typed_message(1, &"m/44'/60'/0'/0/0".try_into().unwrap(), msg, true)
311+
.await
312+
.unwrap();
313+
assert_eq!(signature.len(), 65);
314+
315+
// Compute the EIP-712 digest independently of the wire encoder. The device
316+
// must sign +128, -128 and zero, not reinterpret or reject their sign bytes.
317+
let domain_hash = keccak256(
318+
&[
319+
keccak256(b"EIP712Domain(string name)"),
320+
keccak256(b"Signed integers"),
321+
]
322+
.concat(),
323+
);
324+
let mut positive = [0u8; 32];
325+
positive[31] = 0x80;
326+
let mut negative = [0xff; 32];
327+
negative[31] = 0x80;
328+
let message_hash = keccak256(
329+
&[
330+
keccak256(b"SignedIntegers(int16 positive,int8 negative,int8 zero)"),
331+
positive,
332+
negative,
333+
[0u8; 32],
334+
]
335+
.concat(),
336+
);
337+
let digest = keccak256(&[b"\x19\x01".as_slice(), &domain_hash, &message_hash].concat());
338+
verify_eth_signature(&digest, &signature);
339+
})
340+
.await
341+
}
342+
293343
#[tokio::test]
294344
async fn test_eth_sign_typed_message_antiklepto_disabled() {
295345
test_initialized_simulators(async |paired_bitbox| {

0 commit comments

Comments
 (0)