Some improvements to avoid panicking and enhance remainder handling i…

…n case of multisig transactions

app/rust/src/parser/ffi.rs

@@ -225,9 +225,12 @@ pub unsafe extern "C" fn _last_block_ptr(
 ) -> u16 {
     if let Some(tx) = parsed_obj_from_state(tx_t as _).and_then(|obj| obj.transaction()) {
         let block = tx.last_transaction_block();
+
         *block_ptr = block.as_ptr();
         return block.len() as _;
     }
+
+    *block_ptr = core::ptr::null_mut();
     0
 }
 

app/rust/src/parser/parser_common.rs

@@ -20,7 +20,8 @@ pub const C32_ENCODED_ADDRS_LENGTH: usize = 48;
 pub const NUM_SUPPORTED_POST_CONDITIONS: usize = 16;
 pub const SIGNATURE_LEN: usize = 65;
 pub const PUBKEY_LEN: usize = 33;
-pub const TOKEN_TRANSFER_MEMO_LEN: usize = 34;
+pub const MEMO_LEN: usize = 34;
+pub const AMOUNT_LEN: usize = 8;
 
 // A recursion limit use to control ram usage when parsing
 // contract-call arguments that comes in a transaction

app/rust/src/parser/spending_condition.rs

@@ -404,6 +404,7 @@ impl<'a> MultisigSpendingCondition<'a> {
     fn clear_as_singlesig(&mut self) {
         // TODO: check if it involves shrinking
         // the general transaction buffer
+        // function is not being called anywhere
         todo!();
     }
 }

app/rust/src/parser/structured_msg.rs

@@ -82,13 +82,12 @@ impl<'a> Domain<'a> {
             let id = value.value_id();
 
             if id == ValueId::UInt {
-                // wont panic as this was checked by the parser
-                let chain_id = value.uint().unwrap();
+                let chain_id = value.uint().ok_or(ParserError::UnexpectedValue)?;
                 let num = chain_id.numtoa_str(10, &mut buff).as_bytes();
 
                 pageString(out_value, num, page_idx)
             } else {
-                let string = value.string_ascii().unwrap();
+                let string = value.string_ascii().ok_or(ParserError::UnexpectedValue)?;
 
                 pageString(out_value, string.content(), page_idx)
             }

app/rust/src/parser/transaction.rs

@@ -219,6 +219,11 @@ pub type TxTuple<'a> = (
 
 impl<'a> From<(&'a [u8], TxTuple<'a>)> for Transaction<'a> {
     fn from(raw: (&'a [u8], TxTuple<'a>)) -> Self {
+        let mut remainder = None;
+        if !raw.0.is_empty() {
+            remainder = Some(raw.0);
+        }
+
         Self {
             version: (raw.1).0,
             chain_id: (raw.1).1,
@@ -228,7 +233,7 @@ impl<'a> From<(&'a [u8], TxTuple<'a>)> for Transaction<'a> {
             payload: (raw.1).5,
             // At this point the signer is unknown
             signer: SignerId::Invalid,
-            remainder: raw.0,
+            remainder,
         }
     }
 }
@@ -249,23 +254,26 @@ pub struct Transaction<'a> {
     // with them, we can construct the pre_sig_hash for the current signer
     // we would ideally verify it, but we can lend such responsability to the application
     // which has more resources
-    // If this is not a multisig transaction, this field should be an empty array
-    pub remainder: &'a [u8],
+    // If this is not a multisig transaction, this field should be None
+    pub remainder: Option<&'a [u8]>,
 }
 
 impl<'a> Transaction<'a> {
     fn update_remainder(&mut self, data: &'a [u8]) {
-        self.remainder = data;
+        if !data.is_empty() {
+            self.remainder = Some(data);
+        } else {
+            self.remainder = None;
+        }
     }
 
     #[inline(never)]
     pub fn read(&mut self, data: &'a [u8]) -> Result<(), ParserError> {
-        self.update_remainder(data);
-        self.read_header()?;
-        self.read_auth()?;
-        self.read_transaction_modes()?;
-        self.read_post_conditions()?;
-        self.read_payload()?;
+        let rem = self.read_header(data)?;
+        let rem = self.read_auth(rem)?;
+        let rem = self.read_transaction_modes(rem)?;
+        let rem = self.read_post_conditions(rem)?;
+        let rem = self.read_payload(rem)?;
 
         let is_token_transfer = self.payload.is_token_transfer_payload();
         let is_standard_auth = self.transaction_auth.is_standard_auth();
@@ -276,13 +284,21 @@ impl<'a> Transaction<'a> {
 
         // At this point we do not know who the signer is
         self.signer = SignerId::Invalid;
+
+        self.remainder = None;
+
+        // set the remainder if this is mutltisig
+        if self.is_multisig() && !rem.is_empty() {
+            self.update_remainder(rem);
+        }
+
         Ok(())
     }
 
     #[inline(never)]
-    fn read_header(&mut self) -> Result<(), ParserError> {
-        let (next_data, version) = TransactionVersion::from_bytes(self.remainder)
-            .map_err(|_| ParserError::UnexpectedValue)?;
+    fn read_header(&mut self, data: &'a [u8]) -> Result<&'a [u8], ParserError> {
+        let (next_data, version) =
+            TransactionVersion::from_bytes(data).map_err(|_| ParserError::UnexpectedValue)?;
 
         let (next_data, chain_id) =
             be_u32::<_, ParserError>(next_data).map_err(|_| ParserError::UnexpectedValue)?;
@@ -291,52 +307,46 @@ impl<'a> Transaction<'a> {
         self.chain_id = chain_id;
         check_canary!();
 
-        self.update_remainder(next_data);
-
-        Ok(())
+        Ok(next_data)
     }
 
     #[inline(never)]
-    fn read_auth(&mut self) -> Result<(), ParserError> {
-        let (next_data, auth) = TransactionAuth::from_bytes(self.remainder)
-            .map_err(|_| ParserError::InvalidAuthType)?;
+    fn read_auth(&mut self, data: &'a [u8]) -> Result<&'a [u8], ParserError> {
+        let (next_data, auth) =
+            TransactionAuth::from_bytes(data).map_err(|_| ParserError::InvalidAuthType)?;
         self.transaction_auth = auth;
-        self.update_remainder(next_data);
         check_canary!();
-        Ok(())
+        Ok(next_data)
     }
 
     #[inline(never)]
-    fn read_transaction_modes(&mut self) -> Result<(), ParserError> {
+    fn read_transaction_modes(&mut self, data: &'a [u8]) -> Result<&'a [u8], ParserError> {
         // two modes are included here,
         // anchor mode and postcondition mode
-        let (raw, _) = take::<_, _, ParserError>(2usize)(self.remainder)
+        let (raw, _) = take::<_, _, ParserError>(2usize)(data)
             .map_err(|_| ParserError::UnexpectedBufferEnd)?;
-        let modes = arrayref::array_ref!(self.remainder, 0, 2);
+        let modes = arrayref::array_ref!(data, 0, 2);
         self.transaction_modes = modes;
-        self.update_remainder(raw);
         check_canary!();
-        Ok(())
+        Ok(raw)
     }
 
     #[inline(never)]
-    fn read_post_conditions(&mut self) -> Result<(), ParserError> {
-        let (raw, conditions) = PostConditions::from_bytes(self.remainder)
-            .map_err(|_| ParserError::PostConditionFailed)?;
+    fn read_post_conditions(&mut self, data: &'a [u8]) -> Result<&'a [u8], ParserError> {
+        let (raw, conditions) =
+            PostConditions::from_bytes(data).map_err(|_| ParserError::PostConditionFailed)?;
         self.post_conditions = conditions;
-        self.update_remainder(raw);
         check_canary!();
-        Ok(())
+        Ok(raw)
     }
 
     #[inline(never)]
-    fn read_payload(&mut self) -> Result<(), ParserError> {
-        let (raw, payload) = TransactionPayload::from_bytes(self.remainder)
+    fn read_payload(&mut self, data: &'a [u8]) -> Result<&'a [u8], ParserError> {
+        let (raw, payload) = TransactionPayload::from_bytes(data)
             .map_err(|_| ParserError::InvalidTransactionPayload)?;
         self.payload = payload;
-        self.update_remainder(raw);
         check_canary!();
-        Ok(())
+        Ok(raw)
     }
 
     pub fn from_bytes(bytes: &'a [u8]) -> Result<Self, ParserError> {
@@ -532,16 +542,44 @@ impl<'a> Transaction<'a> {
 
     // returns a slice of the last block to be used in the presighash calculation
     pub fn last_transaction_block(&self) -> &[u8] {
-        unsafe {
-            let len =
-                (self.remainder.as_ptr() as usize - self.transaction_modes.as_ptr() as usize) as _;
-            core::slice::from_raw_parts(self.transaction_modes.as_ptr(), len)
+        match self.remainder {
+            Some(remainder) => {
+                let remainder_ptr = remainder.as_ptr() as usize;
+                let tx_modes_ptr = self.transaction_modes.as_ptr() as usize;
+
+                unsafe {
+                    let len = remainder_ptr - tx_modes_ptr;
+                    core::slice::from_raw_parts(self.transaction_modes.as_ptr(), len)
+                }
+            }
+            None => {
+                // If there's no remainder, return everything from transaction_modes to the end of payload
+                let payload = self.payload.raw_payload();
+                unsafe {
+                    let payload_end = payload.as_ptr().add(payload.len());
+                    let len = payload_end as usize - self.transaction_modes.as_ptr() as usize;
+                    core::slice::from_raw_parts(self.transaction_modes.as_ptr(), len)
+                }
+            }
         }
     }
+    // pub fn last_transaction_block(&self) -> Option<&[u8]> {
+    //     self.remainder.map(|remainder| {
+    //         let remainder_ptr = remainder.as_ptr() as usize;
+    //         let tx_modes_ptr = self.transaction_modes.as_ptr() as usize;
+    //
+    //         unsafe {
+    //             let len = remainder_ptr - tx_modes_ptr;
+    //             core::slice::from_raw_parts(self.transaction_modes.as_ptr(), len)
+    //         }
+    //     })
+    // }
 
     pub fn previous_signer_data(&self) -> Option<&[u8]> {
-        if self.is_multisig() && self.remainder.len() >= MULTISIG_PREVIOUS_SIGNER_DATA_LEN {
-            return Some(&self.remainder[..MULTISIG_PREVIOUS_SIGNER_DATA_LEN]);
+        let remainder = self.remainder?;
+
+        if self.is_multisig() && remainder.len() >= MULTISIG_PREVIOUS_SIGNER_DATA_LEN {
+            return Some(&remainder[..MULTISIG_PREVIOUS_SIGNER_DATA_LEN]);
         }
         None
     }

app/rust/src/parser/transaction_payload.rs

@@ -11,8 +11,8 @@ use arrayvec::ArrayVec;
 use numtoa::NumToA;
 
 use super::{
-    utils::ApduPanic, ClarityName, ContractName, PrincipalData, StacksAddress,
-    C32_ENCODED_ADDRS_LENGTH, HASH160_LEN, TX_DEPTH_LIMIT,
+    utils::ApduPanic, ClarityName, ContractName, PrincipalData, StacksAddress, AMOUNT_LEN,
+    C32_ENCODED_ADDRS_LENGTH, HASH160_LEN, MEMO_LEN, TX_DEPTH_LIMIT,
 };
 use crate::parser::error::ParserError;
 
@@ -64,19 +64,19 @@ impl<'a> StxTokenTransfer<'a> {
             TokenTranferPrincipal::Contract => PrincipalData::contract_principal_from_bytes(id.0)?,
         };
         // Besides principal we take 34-bytes being the MEMO message + 8-bytes amount of stx
-        let len = bytes.len() - raw.len() + 34 + 8;
+        let len = bytes.len() - raw.len() + MEMO_LEN + AMOUNT_LEN;
         let (raw, data) = take(len)(bytes)?;
         Ok((raw, Self(data)))
     }
 
     pub fn memo(&self) -> &[u8] {
-        let at = self.0.len() - 34;
+        let at = self.0.len() - MEMO_LEN;
         // safe to unwrap as parser checked for proper len
         self.0.get(at..).apdu_unwrap()
     }
 
     pub fn amount(&self) -> Result<u64, ParserError> {
-        let at = self.0.len() - 34 - 8;
+        let at = self.0.len() - MEMO_LEN - AMOUNT_LEN;
         let amount = self.0.get(at..).ok_or(ParserError::NoData)?;
         be_u64::<_, ParserError>(amount)
             .map(|res| res.1)
@@ -753,6 +753,15 @@ impl<'a> TransactionPayload<'a> {
             }
         }
     }
+
+    pub fn raw_payload(&self) -> &'a [u8] {
+        match self {
+            Self::TokenTransfer(token) => token.0,
+            Self::SmartContract(contract) => contract.0,
+            Self::ContractCall(call) => call.0,
+            Self::VersionedSmartContract(deploy) => deploy.0,
+        }
+    }
 }
 
 #[cfg(test)]

app/rust/src/zxformat.rs

@@ -177,8 +177,11 @@ pub(crate) fn fpstr_to_str(
     }
 
     let fp = in_len - decimals as usize;
-    let left = str.get(0..fp).unwrap();
-    let right = str.get(fp..in_len).unwrap();
+    let left = str.get(0..fp).ok_or(ParserError::UnexpectedBufferEnd)?;
+    let right = str
+        .get(fp..in_len)
+        .ok_or(ParserError::UnexpectedBufferEnd)?;
+
     write!(&mut writer, "{}.{}", left, right)
         .map(|_| writer.offset)
         .map_err(|_| ParserError::UnexpectedBufferEnd)