Parsing error if target chain is ethereum Mainnet and update tests

app/rust/src/ffi.rs

@@ -19,20 +19,19 @@ use core::{mem::MaybeUninit, ptr::addr_of_mut};
 use crate::constants::{BIP32_PATH_SUFFIX_DEPTH, SIGN_HASH_TX_SIZE};
 use crate::handlers::avax::sign_hash::{Sign as SignHash, SignUI};
 use crate::handlers::avax::signing::Sign;
-use crate::handlers::resources::{EthAccessors, ETH_UI};
 use crate::parser::{parse_path_list, AvaxMessage, DisplayableItem, ObjectList, PathWrapper};
 use crate::parser::{FromBytes, ParserError, Transaction};
 use crate::ZxError;
 
 pub mod context;
+mod eth_tx;
 pub mod ext_public_key;
 pub mod nft_info;
 pub mod public_key;
 pub mod sign_hash;
 pub mod wallet_id;
-use bolos::ApduError;
 use context::{parser_context_t, Instruction};
-use zemu_sys::Viewable;
+use eth_tx::{get_eth_item, num_items_eth};
 
 /// Cast a *mut u8 to a *mut Transaction
 macro_rules! avax_tx_from_state {
@@ -254,29 +253,6 @@ unsafe fn num_items_avax(ctx: *const parser_context_t, num_items: &mut u8) -> u3
     ParserError::ParserOk as u32
 }
 
-#[inline(never)]
-unsafe fn num_items_eth(ctx: *const parser_context_t, num_items: &mut u8) -> u32 {
-    let Ok(tx_type) = Instruction::try_from((*ctx).ins) else {
-        return ParserError::InvalidTransactionType as u32;
-    };
-
-    if tx_type.is_avax() {
-        return ParserError::InvalidTransactionType as u32;
-    }
-
-    if let Some(obj) = ETH_UI.lock(EthAccessors::Tx) {
-        match obj.num_items() {
-            Ok(n) => {
-                *num_items = n;
-                ParserError::ParserOk as _
-            }
-            Err(e) => e as _,
-        }
-    } else {
-        ParserError::NoData as _
-    }
-}
-
 #[no_mangle]
 pub unsafe extern "C" fn _getItem(
     ctx: *const parser_context_t,
@@ -391,44 +367,6 @@ unsafe fn get_avax_item(
     }
 }
 
-#[inline(never)]
-unsafe fn get_eth_item(
-    ctx: *const parser_context_t,
-    display_idx: u8,
-    key: &mut [u8],
-    value: &mut [u8],
-    page_idx: u8,
-    page_count: &mut u8,
-) -> u32 {
-    *page_count = 0u8;
-
-    let page_count = &mut *page_count;
-
-    if ctx.is_null() {
-        return ParserError::ParserContextMismatch as _;
-    }
-
-    let Ok(tx_type) = Instruction::try_from((*ctx).ins) else {
-        return ParserError::InvalidTransactionType as u32;
-    };
-
-    if tx_type.is_avax() {
-        return ParserError::InvalidTransactionType as _;
-    }
-
-    if let Some(obj) = ETH_UI.lock(EthAccessors::Tx) {
-        match obj.render_item(display_idx, key, value, page_idx) {
-            Ok(page) => {
-                *page_count = page;
-                ParserError::ParserOk as _
-            }
-            Err(e) => e as _,
-        }
-    } else {
-        ParserError::NoData as _
-    }
-}
-
 // Returns the offset at which transaction data starts.
 // remember that this instruction comes with a list of change_path at the
 // begining of the buffer. those paths needs to be ignored when
@@ -461,23 +399,3 @@ pub unsafe extern "C" fn _tx_data_offset(
 
     ZxError::Ok as _
 }
-
-#[no_mangle]
-unsafe extern "C" fn _accept_eth_tx(tx: *mut u16, buffer: *mut u8, buffer_len: u32) -> u16 {
-    if tx.is_null() || buffer.is_null() || buffer_len == 0 {
-        return ApduError::DataInvalid as u16;
-    }
-
-    let data = std::slice::from_raw_parts_mut(buffer, buffer_len as usize);
-
-    let code = if let Some(obj) = ETH_UI.lock(EthAccessors::Tx) {
-        let (_tx, code) = obj.accept(data);
-        *tx = _tx as u16;
-        code
-    } else {
-        // No ethereum transaction has been processed yet
-        ApduError::DataInvalid as u16
-    };
-
-    code
-}

app/rust/src/ffi/eth_tx.rs

@@ -0,0 +1,90 @@
+use zemu_sys::Viewable;
+
+use crate::{
+    constants::ApduError,
+    handlers::resources::{EthAccessors, ETH_UI},
+    parser::ParserError,
+};
+
+use super::context::{parser_context_t, Instruction};
+
+#[inline(never)]
+pub unsafe fn num_items_eth(ctx: *const parser_context_t, num_items: &mut u8) -> u32 {
+    let Ok(tx_type) = Instruction::try_from((*ctx).ins) else {
+        return ParserError::InvalidTransactionType as u32;
+    };
+
+    if tx_type.is_avax() {
+        return ParserError::InvalidTransactionType as u32;
+    }
+
+    if let Some(obj) = ETH_UI.lock(EthAccessors::Tx) {
+        match obj.num_items() {
+            Ok(n) => {
+                *num_items = n;
+                ParserError::ParserOk as _
+            }
+            Err(e) => e as _,
+        }
+    } else {
+        ParserError::NoData as _
+    }
+}
+
+#[inline(never)]
+pub unsafe fn get_eth_item(
+    ctx: *const parser_context_t,
+    display_idx: u8,
+    key: &mut [u8],
+    value: &mut [u8],
+    page_idx: u8,
+    page_count: &mut u8,
+) -> u32 {
+    *page_count = 0u8;
+
+    let page_count = &mut *page_count;
+
+    if ctx.is_null() {
+        return ParserError::ParserContextMismatch as _;
+    }
+
+    let Ok(tx_type) = Instruction::try_from((*ctx).ins) else {
+        return ParserError::InvalidTransactionType as u32;
+    };
+
+    if tx_type.is_avax() {
+        return ParserError::InvalidTransactionType as _;
+    }
+
+    if let Some(obj) = ETH_UI.lock(EthAccessors::Tx) {
+        match obj.render_item(display_idx, key, value, page_idx) {
+            Ok(page) => {
+                *page_count = page;
+                ParserError::ParserOk as _
+            }
+            Err(e) => e as _,
+        }
+    } else {
+        ParserError::NoData as _
+    }
+}
+
+#[no_mangle]
+unsafe extern "C" fn _accept_eth_tx(tx: *mut u16, buffer: *mut u8, buffer_len: u32) -> u16 {
+    if tx.is_null() || buffer.is_null() || buffer_len == 0 {
+        return ApduError::DataInvalid as u16;
+    }
+
+    let data = std::slice::from_raw_parts_mut(buffer, buffer_len as usize);
+
+    let code = if let Some(obj) = ETH_UI.lock(EthAccessors::Tx) {
+        let (_tx, code) = obj.accept(data);
+        *tx = _tx as u16;
+        code
+    } else {
+        // No ethereum transaction has been processed yet
+        ApduError::DataInvalid as u16
+    };
+
+    code
+}

app/rust/src/handlers/eth/signing.rs

@@ -137,8 +137,6 @@ impl Sign {
         // since the tx type is at the start of the data
         // let to_hash = txdata.len() - rem.len();
         // let to_hash = &txdata[..to_hash];
-
-        // let unsigned_hash = Self::digest(to_hash).map_err(|_| ParserError::UnexpectedError)?;
         let tx = unsafe { tx.assume_init() };
 
         let ui = EthUi::Tx(SignUI {
@@ -337,7 +335,7 @@ impl ApduHandler for Sign {
 
 pub(crate) struct SignUI {
     // hash: [u8; Sign::SIGN_HASH_SIZE],
-    tx: EthTransaction<'static>,
+    pub(crate) tx: EthTransaction<'static>,
 }
 
 impl Viewable for SignUI {

app/rust/src/parser/constants.rs

@@ -19,6 +19,7 @@ pub const NETWORK_ID_MAINNET: u32 = 1;
 pub const NETWORK_ID_FUJI: u32 = 5;
 pub const NETWORK_ID_LOCAL: u32 = 12345;
 pub const NETWORK_ID_CUSTOM: u32 = 1337;
+pub const ETH_MAINNET_ID: u64 = 1;
 
 // hrp
 pub const HRP_MAINNET: &str = "avax";

app/rust/src/parser/coreth/native.rs

@@ -38,10 +38,6 @@ pub use eip1559::Eip1559;
 mod eip2930;
 pub use eip2930::Eip2930;
 
-#[derive(Clone, Copy, PartialEq, Eq)]
-#[cfg_attr(test, derive(Debug))]
-pub struct EthChainId<'b>(&'b [u8]);
-
 /// Renders an u256 in bytes.
 /// `input`: The big-indian bytes of the number to
 /// be rendered

app/rust/src/parser/coreth/native/base.rs

@@ -301,7 +301,7 @@ impl<'b> FromBytes<'b> for BaseLegacy<'b> {
         input: &'b [u8],
         out: &mut MaybeUninit<Self>,
     ) -> Result<&'b [u8], nom::Err<ParserError>> {
-        crate::sys::zemu_log_stack("EthBase::from_bytes_into\x00");
+        crate::zlog("EthBase::from_bytes_into\x00");
 
         // get out pointer
         let out = out.as_mut_ptr();

app/rust/src/parser/coreth/native/eip1559.rs

@@ -27,7 +27,7 @@ use crate::{
     },
     parser::{
         intstr_to_fpstr_inplace, Address, DisplayableItem, EthData, FromBytes, ParserError,
-        ADDRESS_LEN, WEI_AVAX_DIGITS, WEI_NAVAX_DIGITS,
+        ADDRESS_LEN, ETH_MAINNET_ID, WEI_AVAX_DIGITS, WEI_NAVAX_DIGITS,
     },
     utils::ApduPanic,
 };
@@ -70,7 +70,7 @@ impl<'b> FromBytes<'b> for Eip1559<'b> {
         input: &'b [u8],
         out: &mut MaybeUninit<Self>,
     ) -> Result<&'b [u8], nom::Err<ParserError>> {
-        crate::sys::zemu_log_stack("Eip1559::from_bytes_into\x00");
+        crate::zlog("Eip1559::from_bytes_into\x00");
 
         // get out pointer
         let out = out.as_mut_ptr();
@@ -126,6 +126,10 @@ impl<'b> FromBytes<'b> for Eip1559<'b> {
         }
 
         let chain_id = super::bytes_to_u64(id_bytes)?;
+        if chain_id == ETH_MAINNET_ID {
+            crate::zlog("Mainnet not allowed\x00");
+            return Err(ParserError::InvalidChainId.into());
+        }
 
         // check for erc721 call and chainID
         #[cfg(feature = "erc721")]

app/rust/src/parser/coreth/native/eip2930.rs

@@ -18,6 +18,7 @@ use zemu_sys::ViewError;
 
 use super::parse_rlp_item;
 use super::BaseLegacy;
+use crate::parser::ETH_MAINNET_ID;
 use crate::parser::{DisplayableItem, FromBytes, ParserError};
 
 #[derive(Clone, Copy, PartialEq, Eq)]
@@ -55,7 +56,7 @@ impl<'b> FromBytes<'b> for Eip2930<'b> {
         input: &'b [u8],
         out: &mut MaybeUninit<Self>,
     ) -> Result<&'b [u8], nom::Err<ParserError>> {
-        crate::sys::zemu_log_stack("Eip2930::from_bytes_into\x00");
+        crate::zlog("Eip2930::from_bytes_into\x00");
 
         // get out pointer
         let out = out.as_mut_ptr();
@@ -74,6 +75,11 @@ impl<'b> FromBytes<'b> for Eip2930<'b> {
 
         let chain_id = super::bytes_to_u64(id_bytes)?;
 
+        if chain_id == ETH_MAINNET_ID {
+            crate::zlog("Mainnet not allowed\x00");
+            return Err(ParserError::InvalidChainId.into());
+        }
+
         // check for erc721 call and chainID
         #[cfg(feature = "erc721")]
         {

app/rust/src/parser/coreth/native/legacy.rs

@@ -19,10 +19,12 @@ use zemu_sys::ViewError;
 
 use super::parse_rlp_item;
 use super::BaseLegacy;
+use crate::parser::ETH_MAINNET_ID;
 use crate::parser::U64_SIZE;
 use crate::parser::{DisplayableItem, FromBytes, ParserError};
 
 const MAX_CHAIN_LEN: usize = U64_SIZE;
+const MAINNET_ID: u64 = ETH_MAINNET_ID * 2 + 35;
 
 #[derive(Clone, Copy, PartialEq, Eq)]
 #[cfg_attr(test, derive(Debug))]
@@ -53,18 +55,17 @@ impl<'b> FromBytes<'b> for Legacy<'b> {
         input: &'b [u8],
         out: &mut MaybeUninit<Self>,
     ) -> Result<&'b [u8], nom::Err<ParserError>> {
-        crate::sys::zemu_log_stack("Legacy::from_bytes_into\x00");
+        crate::zlog("Legacy::from_bytes_into\x00");
 
         // get out pointer
         let out = out.as_mut_ptr();
 
         let data_out = unsafe { &mut *addr_of_mut!((*out).base).cast() };
         let rem = BaseLegacy::from_bytes_into(input, data_out)?;
 
-        // two cases:
-        // - legacy no EIP155 compliant which should be supported
-        // - legacy EIP155 in which case should come with empty r and s values
+        // Handle cases based on the presence of RLP-encoded chain ID.
         if rem.is_empty() {
+            crate::zlog("chain_id_empty\x00");
             unsafe {
                 // write an empty chain-id as it is used to compute the right V component
                 // when transaction is signed
@@ -96,13 +97,12 @@ impl<'b> FromBytes<'b> for Legacy<'b> {
             }
         }
 
-        // r and s if not empty, should contain only one value
-        // which must be zero.
+        // Validate R and S are either empty or zero.
         if !r.is_empty() && !s.is_empty() {
-            crate::sys::zemu_log_stack("r_s_not_empty\x00");
+            crate::zlog("r_s_not_empty\x00");
             let no_zero = r.iter().any(|v| *v != 0) && s.iter().any(|v| *v != 0);
             if no_zero || r.len() != 1 || s.len() != 1 {
-                crate::sys::zemu_log_stack("Legacy::invalid_r_s\x00");
+                crate::zlog("Legacy::invalid_r_s\x00");
                 return Err(ParserError::UnexpectedData.into());
             }
         }
@@ -112,6 +112,16 @@ impl<'b> FromBytes<'b> for Legacy<'b> {
             return Err(ParserError::InvalidChainId.into());
         }
 
+        // Check that tx is not targeting ethereum mainnet
+        let chain_id = super::bytes_to_u64(id_bytes)?;
+
+        // Check for one in case a wallet does not follow EIP-155
+        // and just append directly the chainId value at the end of the transaction.
+        if chain_id == 1 || chain_id == MAINNET_ID {
+            crate::zlog("Mainnet not allowed\x00");
+            return Err(ParserError::InvalidChainId.into());
+        }
+
         unsafe {
             addr_of_mut!((*out).chain_id).write(id_bytes);
         }