From 31c8486670f6af245e84dd01efbf39334e650366 Mon Sep 17 00:00:00 2001 From: benma's agent Date: Fri, 2 Oct 2026 12:39:08 +0200 Subject: [PATCH] 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: https://github.com/BitBoxSwiss/bitbox02-api-go/pull/189 - TypeScript: https://github.com/BitBoxSwiss/bitbox-api-ts/pull/14 --- CHANGELOG.md | 1 + src/eth.rs | 43 +++++++++++++++++++++++++++++++++------- tests/test_eth.rs | 50 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6e39ba..8df68d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## [Unreleased] +- Preserve the sign of positive EIP-712 signed integers supplied as JSON numbers. - Reset unfinished sessions when connecting to firmware v9.28.0 or newer, allowing host reconnects while the device remains powered on. - Remove the `wasm` feature, the public `wasm` module, and the TypeScript/WASM package. The diff --git a/src/eth.rs b/src/eth.rs index 5cfc595..a17c4e2 100644 --- a/src/eth.rs +++ b/src/eth.rs @@ -333,12 +333,7 @@ fn encode_value(typ: &MemberType, value: &Value) -> Result, String> { if (v64 as f64) != v { Err("Number is not an int".to_string()) } else { - let mut bytes = BigInt::from(v64).to_signed_bytes_be(); - // Drop leading zero. There can be at most one. - if let [0, ..] = bytes.as_slice() { - bytes.remove(0); - } - Ok(bytes) + Ok(BigInt::from(v64).to_signed_bytes_be()) } } else { Err("Number is not an int".to_string()) @@ -1164,7 +1159,7 @@ mod tests { &2983742332.0.into(), ) .unwrap(); - assert_eq!(vec![0xb1, 0xd8, 0x4b, 0x7c], encoded); + assert_eq!(vec![0x00, 0xb1, 0xd8, 0x4b, 0x7c], encoded); let encoded = encode_value( &parse_type_no_err("int64", &HashMap::new()), @@ -1218,6 +1213,40 @@ mod tests { assert_eq!(b"\x00\x00\x03\xe8".to_vec(), encoded); } + #[test] + fn test_encode_value_signed_integers() { + let typ = parse_type_no_err("int64", &HashMap::new()); + for (value, expected) in [ + (0i64, "00"), + (1, "01"), + (-1, "ff"), + (127, "7f"), + (128, "0080"), + (129, "0081"), + (-127, "81"), + (-128, "80"), + (-129, "ff7f"), + (255, "00ff"), + (256, "0100"), + (-255, "ff01"), + (-256, "ff00"), + (32767, "7fff"), + (32768, "008000"), + (-32768, "8000"), + (-32769, "ff7fff"), + (65535, "00ffff"), + (65536, "010000"), + ] { + for input in [Value::from(value), Value::from(value.to_string())] { + assert_eq!( + hex::encode(encode_value(&typ, &input).unwrap()), + expected, + "{input:?}", + ); + } + } + } + #[test] fn test_get_value() { let msg: Eip712Message = serde_json::from_str(EIP712_MSG).unwrap(); diff --git a/tests/test_eth.rs b/tests/test_eth.rs index a52d059..d6c5f8b 100644 --- a/tests/test_eth.rs +++ b/tests/test_eth.rs @@ -290,6 +290,56 @@ async fn test_eth_sign_typed_message_antiklepto_enabled() { .await } +#[tokio::test] +async fn test_eth_sign_typed_message_signed_integers() { + test_initialized_simulators(async |paired_bitbox| { + let msg = r#"{ + "types": { + "EIP712Domain": [{"name": "name", "type": "string"}], + "SignedIntegers": [ + {"name": "positive", "type": "int16"}, + {"name": "negative", "type": "int8"}, + {"name": "zero", "type": "int8"} + ] + }, + "primaryType": "SignedIntegers", + "domain": {"name": "Signed integers"}, + "message": {"positive": 128, "negative": -128, "zero": 0} + }"#; + let signature = paired_bitbox + .eth_sign_typed_message(1, &"m/44'/60'/0'/0/0".try_into().unwrap(), msg, true) + .await + .unwrap(); + assert_eq!(signature.len(), 65); + + // Compute the EIP-712 digest independently of the wire encoder. The device + // must sign +128, -128 and zero, not reinterpret or reject their sign bytes. + let domain_hash = keccak256( + &[ + keccak256(b"EIP712Domain(string name)"), + keccak256(b"Signed integers"), + ] + .concat(), + ); + let mut positive = [0u8; 32]; + positive[31] = 0x80; + let mut negative = [0xff; 32]; + negative[31] = 0x80; + let message_hash = keccak256( + &[ + keccak256(b"SignedIntegers(int16 positive,int8 negative,int8 zero)"), + positive, + negative, + [0u8; 32], + ] + .concat(), + ); + let digest = keccak256(&[b"\x19\x01".as_slice(), &domain_hash, &message_hash].concat()); + verify_eth_signature(&digest, &signature); + }) + .await +} + #[tokio::test] async fn test_eth_sign_typed_message_antiklepto_disabled() { test_initialized_simulators(async |paired_bitbox| {