From 7901b21e065ecdbd275a285d7eb51f1d3ce3dcdd Mon Sep 17 00:00:00 2001
From: Rob Ede <robjtede@icloud.com>
Date: Sun, 26 Mar 2023 09:11:23 +0100
Subject: [PATCH] Improve diagnostics when key being deserializing is too long
 (#294)

---
 src/signing.rs   | 13 ++++++++++++
 src/verifying.rs | 14 +++++++++++++
 tests/ed25519.rs | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+)

diff --git a/src/signing.rs b/src/signing.rs
index 16f4ac6..fd59deb 100644
--- a/src/signing.rs
+++ b/src/signing.rs
@@ -666,6 +666,19 @@ impl<'d> Deserialize<'d> for SigningKey {
                         .next_element()?
                         .ok_or_else(|| serde::de::Error::invalid_length(i, &"expected 32 bytes"))?;
                 }
+
+                let remaining = (0..)
+                    .map(|_| seq.next_element::<u8>())
+                    .take_while(|el| matches!(el, Ok(Some(_))))
+                    .count();
+
+                if remaining > 0 {
+                    return Err(serde::de::Error::invalid_length(
+                        32 + remaining,
+                        &"expected 32 bytes",
+                    ));
+                }
+
                 SigningKey::try_from(bytes).map_err(serde::de::Error::custom)
             }
         }
diff --git a/src/verifying.rs b/src/verifying.rs
index 8816fec..6b0ad49 100644
--- a/src/verifying.rs
+++ b/src/verifying.rs
@@ -569,11 +569,25 @@ impl<'d> Deserialize<'d> for VerifyingKey {
                 A: serde::de::SeqAccess<'de>,
             {
                 let mut bytes = [0u8; 32];
+
                 for i in 0..32 {
                     bytes[i] = seq
                         .next_element()?
                         .ok_or_else(|| serde::de::Error::invalid_length(i, &"expected 32 bytes"))?;
                 }
+
+                let remaining = (0..)
+                    .map(|_| seq.next_element::<u8>())
+                    .take_while(|el| matches!(el, Ok(Some(_))))
+                    .count();
+
+                if remaining > 0 {
+                    return Err(serde::de::Error::invalid_length(
+                        32 + remaining,
+                        &"expected 32 bytes",
+                    ));
+                }
+
                 VerifyingKey::try_from(&bytes[..]).map_err(serde::de::Error::custom)
             }
         }
diff --git a/tests/ed25519.rs b/tests/ed25519.rs
index a3a7ebc..6632f01 100644
--- a/tests/ed25519.rs
+++ b/tests/ed25519.rs
@@ -542,6 +542,33 @@ mod serialisation {
         assert_eq!(verifying_key, decoded_verifying_key);
     }
 
+    #[test]
+    fn serialize_deserialize_verifying_key_json_too_long() {
+        // derived from `serialize_deserialize_verifying_key_json` test
+        // trailing zero elements makes key too long (34 bytes)
+        let encoded_verifying_key_too_long = "[130,39,155,15,62,76,188,63,124,122,26,251,233,253,225,220,14,41,166,120,108,35,254,77,160,83,172,58,219,42,86,120,0,0]";
+        let de_err = serde_json::from_str::<VerifyingKey>(&encoded_verifying_key_too_long)
+            .unwrap_err()
+            .to_string();
+        assert!(
+            de_err.contains("invalid length 34"),
+            "expected invalid length error, got: {de_err}",
+        );
+    }
+
+    #[test]
+    fn serialize_deserialize_verifying_key_json_too_short() {
+        // derived from `serialize_deserialize_verifying_key_json` test
+        let encoded_verifying_key_too_long = "[130,39,155,15]";
+        let de_err = serde_json::from_str::<VerifyingKey>(&encoded_verifying_key_too_long)
+            .unwrap_err()
+            .to_string();
+        assert!(
+            de_err.contains("invalid length 4"),
+            "expected invalid length error, got: {de_err}"
+        );
+    }
+
     #[test]
     fn serialize_deserialize_signing_key_bincode() {
         let signing_key = SigningKey::from_bytes(&SECRET_KEY_BYTES);
@@ -564,6 +591,33 @@ mod serialisation {
         }
     }
 
+    #[test]
+    fn serialize_deserialize_signing_key_json_too_long() {
+        // derived from `serialize_deserialize_signing_key_json` test
+        // trailing zero elements makes key too long (34 bytes)
+        let encoded_signing_key_too_long = "[62,70,27,163,92,182,11,3,77,234,98,4,11,127,79,228,243,187,150,73,201,137,76,22,85,251,152,2,241,42,72,54,0,0]";
+        let de_err = serde_json::from_str::<SigningKey>(&encoded_signing_key_too_long)
+            .unwrap_err()
+            .to_string();
+        assert!(
+            de_err.contains("invalid length 34"),
+            "expected invalid length error, got: {de_err}",
+        );
+    }
+
+    #[test]
+    fn serialize_deserialize_signing_key_json_too_short() {
+        // derived from `serialize_deserialize_signing_key_json` test
+        let encoded_signing_key_too_long = "[62,70,27,163]";
+        let de_err = serde_json::from_str::<SigningKey>(&encoded_signing_key_too_long)
+            .unwrap_err()
+            .to_string();
+        assert!(
+            de_err.contains("invalid length 4"),
+            "expected invalid length error, got: {de_err}"
+        );
+    }
+
     #[test]
     fn serialize_deserialize_signing_key_toml() {
         let demo = Demo {