From f49c617a9dacde725710fb23e27702462b82b391 Mon Sep 17 00:00:00 2001
From: "Tony Arcieri (iqlusion)" <tony@iqlusion.io>
Date: Sat, 7 Jan 2023 09:35:37 -0700
Subject: [PATCH] Improve parsing of serial numbers (#466)

Checks the length of the data returned when querying the serial number,
returning an error if it's longer than 4 bytes, and left-padding with
zeroes if it's too short.

This fixes some potential panics due to incorrect slice lengths as were
experienced in #465
---
 src/transaction.rs | 99 +++++++++++++++++++++++-----------------------
 src/yubikey.rs     | 14 +++++++
 2 files changed, 63 insertions(+), 50 deletions(-)

diff --git a/src/transaction.rs b/src/transaction.rs
index 7134676..0ac156f 100644
--- a/src/transaction.rs
+++ b/src/transaction.rs
@@ -105,61 +105,60 @@ impl<'tx> Transaction<'tx> {
 
     /// Get YubiKey device serial number.
     pub fn get_serial(&self, version: Version) -> Result<Serial> {
-        let response = if version.major < 5 {
-            // YK4 requires switching to the yk applet to retrieve the serial
-            let sw = Apdu::new(Ins::SelectApplication)
-                .p1(0x04)
-                .data(YK_AID)
-                .transmit(self, 0xFF)?
-                .status_words();
+        match version.major {
+            4 => {
+                // YK4 requires switching to the yk applet to retrieve the serial
+                let sw = Apdu::new(Ins::SelectApplication)
+                    .p1(0x04)
+                    .data(YK_AID)
+                    .transmit(self, 0xFF)?
+                    .status_words();
 
-            if !sw.is_success() {
-                error!("failed selecting yk application: {:04x}", sw.code());
-                return Err(Error::GenericError);
+                if !sw.is_success() {
+                    error!("failed selecting yk application: {:04x}", sw.code());
+                    return Err(Error::GenericError);
+                }
+
+                let response = Apdu::new(0x01).p1(0x10).transmit(self, 0xFF)?;
+
+                if !response.is_success() {
+                    error!(
+                        "failed retrieving serial number: {:04x}",
+                        response.status_words().code()
+                    );
+                    return Err(Error::GenericError);
+                }
+
+                // reselect the PIV applet
+                let sw = Apdu::new(Ins::SelectApplication)
+                    .p1(0x04)
+                    .data(PIV_AID)
+                    .transmit(self, 0xFF)?
+                    .status_words();
+
+                if !sw.is_success() {
+                    error!("failed selecting application: {:04x}", sw.code());
+                    return Err(Error::GenericError);
+                }
+
+                response.data().try_into()
             }
+            5 => {
+                // YK5 implements getting the serial as a PIV applet command (0xf8)
+                let response = Apdu::new(Ins::GetSerial).transmit(self, 0xFF)?;
 
-            let resp = Apdu::new(0x01).p1(0x10).transmit(self, 0xFF)?;
+                if !response.is_success() {
+                    error!(
+                        "failed retrieving serial number: {:04x}",
+                        response.status_words().code()
+                    );
+                    return Err(Error::GenericError);
+                }
 
-            if !resp.is_success() {
-                error!(
-                    "failed retrieving serial number: {:04x}",
-                    resp.status_words().code()
-                );
-                return Err(Error::GenericError);
+                response.data().try_into()
             }
-
-            // reselect the PIV applet
-            let sw = Apdu::new(Ins::SelectApplication)
-                .p1(0x04)
-                .data(PIV_AID)
-                .transmit(self, 0xFF)?
-                .status_words();
-
-            if !sw.is_success() {
-                error!("failed selecting application: {:04x}", sw.code());
-                return Err(Error::GenericError);
-            }
-
-            resp
-        } else {
-            // YK5 implements getting the serial as a PIV applet command (0xf8)
-            let resp = Apdu::new(Ins::GetSerial).transmit(self, 0xFF)?;
-
-            if !resp.is_success() {
-                error!(
-                    "failed retrieving serial number: {:04x}",
-                    resp.status_words().code()
-                );
-                return Err(Error::GenericError);
-            }
-
-            resp
-        };
-
-        response.data()[..4]
-            .try_into()
-            .map(|serial| Serial::from(u32::from_be_bytes(serial)))
-            .map_err(|_| Error::SizeError)
+            _ => Err(Error::NotSupported),
+        }
     }
 
     /// Verify device PIN.
diff --git a/src/yubikey.rs b/src/yubikey.rs
index 089d7a7..97e8c13 100644
--- a/src/yubikey.rs
+++ b/src/yubikey.rs
@@ -98,6 +98,20 @@ impl From<Serial> for u32 {
     }
 }
 
+impl TryFrom<&[u8]> for Serial {
+    type Error = Error;
+
+    fn try_from(bytes: &[u8]) -> Result<Self> {
+        if bytes.len() > 4 {
+            return Err(Error::SizeError);
+        }
+
+        let mut arr = [0u8; 4];
+        arr[(4 - bytes.len())..].copy_from_slice(bytes);
+        Ok(Self(u32::from_be_bytes(arr)))
+    }
+}
+
 impl FromStr for Serial {
     type Err = Error;