Merge remote-tracking branch 'agent/benma-agent/show-eip712-primary-type'
What the developer wrote
Authored by Marko Bencun
Merge remote-tracking branch 'agent/benma-agent/show-eip712-primary-type'
What changed, and why it matters
This update adds an extra confirmation screen when signing Ethereum typed messages (EIP-712). Before the user approves a signature, the device now shows the message's primary type, such as 'Authorize' or 'Revoke'. This helps users notice if a website is asking them to sign a different kind of message than they expect, reducing the risk of being tricked into approving a harmful signature.
No urgent action required. Treat as a routine security-hardening improvement. Users benefit from the added transparency during EIP-712 signing. Review whether additional context, such as the full message schema or a warning for unexpected primary types, is warranted.
Security signals we found
New user confirmation step before cryptographic signing
EIP-712 primary type displayed to mitigate misleading typed-data signing requests
Test coverage added for UI screen ordering
Evidence from the diff
The commit inserts a new confirm_value call in the EIP-712 signing flow (sign_typed_msg.rs) that displays the request.primary_type string under the title ‘Message type’ before proceeding to compute the sighash and sign. Tests are added to verify that the primary type screen appears and that removing it makes otherwise identical ‘Authorize’ and ‘Revoke’ message flows produce the same remaining screens. This is a hardening/usability improvement rather than a fix for a memory-safety or cryptographic bug.
Changed components
src/rust/bitbox02-rust/src/hww/api/ethereum/sign_typed_msg.rsEthereum EIP-712 typed message signing UI flowInspect captured patch +90 / −0
### CHANGELOG.md
@@ -7,6 +7,7 @@ customers cannot upgrade their bootloader, its changes are recorded separately.
## Firmware
### [Unreleased]
+- Ethereum: display the EIP-712 message type before signing
### v9.27.0
- Display long transaction and swap amounts in full instead of truncating them
### src/rust/bitbox02-rust/src/hww/api/ethereum/sign_typed_msg.rs
@@ -734,6 +734,18 @@ pub async fn process(
)
.await?;
+ confirm::confirm_value(
+ hal,
+ &ConfirmParams {
+ title: "Message type",
+ body: &request.primary_type,
+ scrollable: true,
+ accept_is_nextarrow: true,
+ ..Default::default()
+ },
+ )
+ .await?;
+
let sighash: [u8; 32] = eip712_sighash(
hal,
&request.types,
@@ -1711,6 +1723,78 @@ mod tests {
assert!(mock_hal.ui.screens.is_empty());
}
+ #[async_test::test]
+ async fn test_process_displays_primary_type() {
+ async fn run(primary_type: &'static str) -> Vec<Screen> {
+ mock_unlocked();
+ let typed_msg = alloc::rc::Rc::new(TypedMessage::new(
+ vec![
+ StructType {
+ name: DOMAIN_TYPE_NAME.into(),
+ members: vec![mk_member("chainId", mk_sized_type(DataType::Uint, 32))],
+ },
+ StructType {
+ name: "Authorize".into(),
+ members: vec![mk_member("amount", mk_sized_type(DataType::Uint, 32))],
+ },
+ StructType {
+ name: "Revoke".into(),
+ members: vec![mk_member("amount", mk_sized_type(DataType::Uint, 32))],
+ },
+ ],
+ primary_type,
+ Object::Struct(vec![Object::BigUint(BigUint::from(1u32))]),
+ Object::Struct(vec![Object::BigUint(BigUint::from(42u32))]),
+ ));
+ {
+ let typed_msg = typed_msg.clone();
+ *crate::hww::MOCK_NEXT_REQUEST.0.borrow_mut() = Some(Box::new(move |response| {
+ Ok(typed_msg.handle_host_response(&response).unwrap())
+ }));
+ }
+
+ let mut mock_hal = TestingHal::new();
+ let result = process(
+ &mut mock_hal,
+ &pb::EthSignTypedMessageRequest {
+ chain_id: 1,
+ keypath: vec![44 + HARDENED, 60 + HARDENED, 0 + HARDENED, 0, 0],
+ types: typed_msg.types.clone(),
+ primary_type: primary_type.into(),
+ host_nonce_commitment: None,
+ },
+ )
+ .await;
+
+ assert!(matches!(result, Ok(Response::Sign(_))));
+ mock_hal.ui.screens
+ }
+
+ let mut authorize_screens = run("Authorize").await;
+ let mut revoke_screens = run("Revoke").await;
+
+ assert_eq!(
+ authorize_screens[1],
+ Screen::Confirm {
+ title: "Message type".into(),
+ body: "Authorize".into(),
+ longtouch: false,
+ }
+ );
+ assert_eq!(
+ revoke_screens[1],
+ Screen::Confirm {
+ title: "Message type".into(),
+ body: "Revoke".into(),
+ longtouch: false,
+ }
+ );
+
+ authorize_screens.remove(1);
+ revoke_screens.remove(1);
+ assert_eq!(authorize_screens, revoke_screens);
+ }
+
/// Test computation of the domain separator, which is `hashStruct(domain)`.
#[async_test::test]
async fn test_domain_separator() {
@@ -2727,6 +2811,11 @@ mod tests {
body: address.clone(),
longtouch: false,
});
+ expected_screens.push(Screen::Confirm {
+ title: "Message type".into(),
+ body: tc.primary_type.clone(),
+ longtouch: false,
+ });
expected_screens.extend(tc.expected_screens.iter().map(|(title, body)| {
Screen::Confirm {
title: title.clone(),Why this scored 30/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.