2.8.2: remove magic numbers

Author: Lbqds

app/src/handler.rs

@@ -12,6 +12,14 @@ use crate::{
     },
 };
 
+const MAX_TOKEN_SIZE: u8 = 5;
+const PATH_LENGTH: usize = 20;
+const HASH_LENGTH: usize = 32;
+const PATH_HEX_LENGTH: usize = PATH_LENGTH * 2;
+const FIRST_FRAME_PREFIX_LENGTH: usize = PATH_LENGTH + 1; // path + 1 byte token size
+const CALL_CONTRACT_FLAG: u8 = 0x01;
+const SCRIPT_OFFSET: usize = 3; // the encoded script offset in the tx
+
 #[repr(u8)]
 pub enum Ins {
     GetVersion,
@@ -58,23 +66,24 @@ pub fn handle_apdu(
         }
         Ins::GetPubKey => {
             let data = comm.get_data()?;
-            if data.len() != 21 {
+            // 1 byte flag indicating whether address verification is needed
+            if data.len() != PATH_LENGTH + 1 {
                 return Err(ErrorCode::BadLen.into());
             }
-            let raw_path = &data[..20];
+            let raw_path = &data[..PATH_LENGTH];
             deserialize_path::<io::Reply>(
                 raw_path,
                 &mut path,
                 ErrorCode::HDPathDecodingFailed.into(),
             )?;
 
             println("raw path");
-            println_slice::<40>(raw_path);
+            println_slice::<PATH_HEX_LENGTH>(raw_path);
             let p1 = apdu_header.p1;
             let p2 = apdu_header.p2;
             let (pk, hd_index) = derive_pub_key(&mut path, p1, p2)?;
 
-            let need_to_display = data[20] != 0;
+            let need_to_display = data[PATH_LENGTH] != 0;
             if need_to_display {
                 review_address(&pk)?;
             }
@@ -84,17 +93,17 @@ pub fn handle_apdu(
         }
         Ins::SignHash => {
             let data = comm.get_data()?;
-            if data.len() != 4 * 5 + 32 {
+            if data.len() != PATH_LENGTH + HASH_LENGTH {
                 return Err(ErrorCode::BadLen.into());
             }
             // This check can be removed, but we keep it for double checking
             deserialize_path::<io::Reply>(
-                &data[..20],
+                &data[..PATH_LENGTH],
                 &mut path,
                 ErrorCode::HDPathDecodingFailed.into(),
             )?;
 
-            match sign_hash_ui(&path, &data[20..]) {
+            match sign_hash_ui(&path, &data[PATH_LENGTH..]) {
                 Ok((signature_buf, length, _)) => comm.append(&signature_buf[..length as usize]),
                 Err(code) => return Err(code.into()),
             }
@@ -127,32 +136,31 @@ pub fn handle_apdu(
     Ok(())
 }
 
-const MAX_TOKEN_SIZE: u8 = 5;
-
 fn handle_sign_tx(
     apdu_header: &ApduHeader,
     data: &[u8],
     sign_tx_context: &mut SignTxContext,
     tx_reviewer: &mut TxReviewer,
 ) -> Result<(), ErrorCode> {
     match apdu_header.p1 {
-        0 if data.len() < 21 => Err(ErrorCode::BadLen), // 20 bytes path + 1 byte token size
+        0 if data.len() < FIRST_FRAME_PREFIX_LENGTH => Err(ErrorCode::BadLen),
         0 => {
-            sign_tx_context.init(data)?;
-            let token_size = data[20];
+            sign_tx_context.init(&data[..PATH_LENGTH])?;
+            let token_size = data[FIRST_FRAME_PREFIX_LENGTH - 1];
             if token_size > MAX_TOKEN_SIZE {
                 return Err(ErrorCode::InvalidTokenSize);
             }
-            let tx_data_index: usize = 21 + TOKEN_METADATA_SIZE * (token_size as usize);
-            if data.len() < tx_data_index + 3 {
+            let tx_data_index: usize =
+                FIRST_FRAME_PREFIX_LENGTH + TOKEN_METADATA_SIZE * (token_size as usize);
+            if data.len() < tx_data_index + SCRIPT_OFFSET {
                 return Err(ErrorCode::BadLen);
             }
             let tx_data = &data[tx_data_index..];
-            let is_tx_execute_script = tx_data[2] == 0x01;
+            let is_tx_execute_script = tx_data[SCRIPT_OFFSET - 1] == CALL_CONTRACT_FLAG;
             if is_tx_execute_script {
                 check_blind_signing()?;
             }
-            let token_metadata = &data[21..tx_data_index];
+            let token_metadata = &data[FIRST_FRAME_PREFIX_LENGTH..tx_data_index];
             check_token_metadata(token_size, token_metadata)?;
             tx_reviewer.init(is_tx_execute_script, token_metadata)?;
             sign_tx_context.handle_data(apdu_header, tx_data, tx_reviewer)

app/src/sign_tx_context.rs

@@ -44,7 +44,7 @@ impl SignTxContext {
     }
 
     pub fn init(&mut self, data: &[u8]) -> Result<(), ErrorCode> {
-        deserialize_path(&data[..20], &mut self.path, ErrorCode::HDPathDecodingFailed)?;
+        deserialize_path(data, &mut self.path, ErrorCode::HDPathDecodingFailed)?;
 
         self.tx_decoder.reset();
         self.current_step = DecodeStep::Init;

utils/src/types/bigint.rs

@@ -2,21 +2,25 @@ use crate::buffer::{Buffer, Writable};
 use crate::decode::*;
 use crate::types::compact_integer::*;
 
+const ENCODED_SIZE: usize = 33;
+
 #[cfg_attr(test, derive(Debug))]
 #[derive(Clone)]
 pub struct BigInt {
-    pub bytes: [u8; 33],
+    pub bytes: [u8; ENCODED_SIZE],
 }
 
 impl Reset for BigInt {
     fn reset(&mut self) {
-        self.bytes = [0; 33];
+        self.bytes = [0; ENCODED_SIZE];
     }
 }
 
 impl Default for BigInt {
     fn default() -> Self {
-        BigInt { bytes: [0; 33] }
+        BigInt {
+            bytes: [0; ENCODED_SIZE],
+        }
     }
 }
 
@@ -29,8 +33,8 @@ impl PartialEq for BigInt {
 impl BigInt {
     #[cfg(test)]
     pub fn from_bytes(bytes: &[u8]) -> Self {
-        assert!(bytes.len() == 33);
-        let mut bs = [0u8; 33];
+        assert!(bytes.len() == ENCODED_SIZE);
+        let mut bs = [0u8; ENCODED_SIZE];
         bs.copy_from_slice(bytes);
         Self { bytes: bs }
     }