Bug fix and improvements in BIP32 path parsing (#45)

* Bug fix and improvements in BIP32 path parsing

Fixes a minor bug in size checking during BIP32 path parsing, and
created a new Bip32Path struct to store a path and its length in a more
rusty way.

src/handlers/get_public_key.rs

@@ -16,7 +16,7 @@
  *****************************************************************************/
 
 use crate::app_ui::address::ui_display_pk;
-use crate::utils::{read_bip32_path, MAX_ALLOWED_PATH_LEN};
+use crate::utils::Bip32Path;
 use crate::AppSW;
 use ledger_device_sdk::ecc::{Secp256k1, SeedDerive};
 use ledger_device_sdk::io::Comm;
@@ -26,11 +26,10 @@ use ledger_secure_sdk_sys::{
 };
 
 pub fn handler_get_public_key(comm: &mut Comm, display: bool) -> Result<(), AppSW> {
-    let mut path = [0u32; MAX_ALLOWED_PATH_LEN];
     let data = comm.get_data().map_err(|_| AppSW::WrongApduLength)?;
-    let path_len = read_bip32_path(data, &mut path)?;
+    let path: Bip32Path = data.try_into()?;
 
-    let pk = Secp256k1::derive_from_path(&path[..path_len])
+    let pk = Secp256k1::derive_from_path(path.as_ref())
         .public_key()
         .map_err(|_| AppSW::KeyDeriveFail)?;
 

src/handlers/sign_tx.rs

@@ -15,7 +15,7 @@
  *  limitations under the License.
  *****************************************************************************/
 use crate::app_ui::sign::ui_display_tx;
-use crate::utils::{read_bip32_path, MAX_ALLOWED_PATH_LEN};
+use crate::utils::Bip32Path;
 use crate::AppSW;
 use ledger_device_sdk::ecc::{Secp256k1, SeedDerive};
 use ledger_device_sdk::io::Comm;
@@ -42,8 +42,7 @@ pub struct Tx<'a> {
 pub struct TxContext {
     raw_tx: [u8; MAX_TRANSACTION_LEN], // raw transaction serialized
     raw_tx_len: usize,                 // length of raw transaction
-    path: [u32; MAX_ALLOWED_PATH_LEN], // BIP32 path for key derivation
-    path_len: usize,                   // length of BIP32 path
+    path: Bip32Path,
 }
 
 // Implement constructor for TxInfo with default values
@@ -52,16 +51,14 @@ impl TxContext {
         TxContext {
             raw_tx: [0u8; MAX_TRANSACTION_LEN],
             raw_tx_len: 0,
-            path: [0u32; MAX_ALLOWED_PATH_LEN],
-            path_len: 0,
+            path: Default::default(),
         }
     }
     // Implement reset for TxInfo
     fn reset(&mut self) {
         self.raw_tx = [0u8; MAX_TRANSACTION_LEN];
         self.raw_tx_len = 0;
-        self.path = [0u32; MAX_ALLOWED_PATH_LEN];
-        self.path_len = 0;
+        self.path = Default::default();
     }
 }
 
@@ -78,7 +75,7 @@ pub fn handler_sign_tx(
         // Reset transaction context
         ctx.reset();
         // This will propagate the error if the path is invalid
-        ctx.path_len = read_bip32_path(data, &mut ctx.path)?;
+        ctx.path = data.try_into()?;
         Ok(())
     // Next chunks, append data to raw_tx and return or parse
     // the transaction if it is the last chunk.
@@ -132,7 +129,7 @@ fn compute_signature_and_append(comm: &mut Comm, ctx: &mut TxContext) -> Result<
         }
     }
 
-    let (sig, siglen, parity) = Secp256k1::derive_from_path(&ctx.path[..ctx.path_len])
+    let (sig, siglen, parity) = Secp256k1::derive_from_path(ctx.path.as_ref())
         .deterministic_sign(&message_hash)
         .map_err(|_| AppSW::TxSignFail)?;
     comm.append(&[siglen as u8]);

src/utils.rs

@@ -1,33 +1,64 @@
 use crate::AppSW;
 use core::str::from_utf8;
 
-pub const MAX_ALLOWED_PATH_LEN: usize = 10;
+/// BIP32 path stored as an array of [`u32`].
+///
+/// # Generic arguments
+///
+/// * `S` - Maximum possible path length, i.e. the capacity of the internal buffer.
+pub struct Bip32Path<const S: usize = 10> {
+    buffer: [u32; S],
+    len: usize,
+}
 
-/// Convert serialized derivation path to u32 array elements
-pub fn read_bip32_path(data: &[u8], path: &mut [u32]) -> Result<usize, AppSW> {
-    // Check input length and path buffer capacity
-    if data.is_empty() || path.len() < data.len() / 4 {
-        return Err(AppSW::WrongApduLength);
+impl AsRef<[u32]> for Bip32Path {
+    fn as_ref(&self) -> &[u32] {
+        &self.buffer[..self.len]
     }
+}
 
-    let path_len = data[0] as usize; // First byte is the length of the path
-    let path_data = &data[1..];
-
-    // Check path data length and alignment
-    if path_data.len() != path_len * 4
-        || path_data.len() > MAX_ALLOWED_PATH_LEN * 4
-        || path_data.len() % 4 != 0
-    {
-        return Err(AppSW::WrongApduLength);
+impl<const S: usize> Default for Bip32Path<S> {
+    fn default() -> Self {
+        Self {
+            buffer: [0u32; S],
+            len: 0,
+        }
     }
+}
 
-    let mut idx = 0;
-    for (i, chunk) in path_data.chunks(4).enumerate() {
-        path[idx] = u32::from_be_bytes(chunk.try_into().unwrap());
-        idx = i + 1;
-    }
+impl<const S: usize> TryFrom<&[u8]> for Bip32Path<S> {
+    type Error = AppSW;
 
-    Ok(idx)
+    /// Constructs a [`Bip32Path`] from a given byte array.
+    ///
+    /// This method will return an error in the following cases:
+    /// - the input array is empty,
+    /// - the number of bytes in the input array is not a multiple of 4,
+    /// - the input array exceeds the capacity of the [`Bip32Path`] internal buffer.
+    ///
+    /// # Arguments
+    ///
+    /// * `data` - Encoded BIP32 path. First byte is the length of the path, as encoded by ragger.
+    fn try_from(data: &[u8]) -> Result<Self, Self::Error> {
+        let input_path_len = (data.len() - 1) / 4;
+        // Check data length
+        if data.is_empty() // At least the length byte is required
+            || (input_path_len > S)
+            || (data[0] as usize * 4 != data.len() - 1)
+        {
+            return Err(AppSW::WrongApduLength);
+        }
+
+        let mut path = [0; S];
+        for (chunk, p) in data[1..].chunks(4).zip(path.iter_mut()) {
+            *p = u32::from_be_bytes(chunk.try_into().unwrap());
+        }
+
+        Ok(Self {
+            buffer: path,
+            len: input_path_len,
+        })
+    }
 }
 
 /// Returns concatenated strings, or an error if the concatenation buffer is too small.