Extra ASN.1 validity checks

ecdsa/src/asn1.rs

@@ -192,6 +192,11 @@ where
         // encoding) so the resulting raw signature will have length
         // at most 254 bytes.
         //
+        // A SEQUENCE + INTEGER + INTEGER imply a length >= 2 + 3 + 3.
+        if bytes.len() < 8 {
+            return Err(Error::new());
+        }
+
         // First byte is SEQUENCE tag.
         if bytes[0] != SEQUENCE_TAG as u8 {
             return Err(Error::new());
@@ -209,6 +214,10 @@ where
             }
 
             zlen = bytes[2] as usize;
+            if zlen <= 127 {
+                // SEQUENCE length should have been the 1-byte encoding to be DER.
+                return Err(Error::new());
+            }
             3
         } else {
             2
@@ -343,6 +352,7 @@ fn trim_zeroes(mut bytes: &[u8], scalar_size: usize) -> Result<usize, Error> {
 
 #[cfg(all(feature = "dev", test))]
 mod tests {
+    use super::{INTEGER_TAG, SEQUENCE_TAG};
     use crate::dev::curve::Signature;
     use signature::Signature as _;
 
@@ -364,4 +374,44 @@ mod tests {
 
         assert_eq!(signature1, signature2);
     }
+
+    #[test]
+    fn test_asn1_too_short_signature() {
+        assert!(Signature::from_asn1(&[]).is_err());
+        assert!(Signature::from_asn1(&[SEQUENCE_TAG]).is_err());
+        assert!(Signature::from_asn1(&[SEQUENCE_TAG, 0x00]).is_err());
+        assert!(Signature::from_asn1(&[SEQUENCE_TAG, 0x03, INTEGER_TAG, 0x01, 0x01]).is_err());
+    }
+
+    #[test]
+    fn test_asn1_non_der_signature() {
+        // A minimal 8-byte ASN.1 signature parses OK.
+        assert!(Signature::from_asn1(&[
+            SEQUENCE_TAG,
+            0x06, // length of below
+            INTEGER_TAG,
+            0x01, // length of value
+            0x01, // value=1
+            INTEGER_TAG,
+            0x01, // length of value
+            0x01, // value=1
+        ])
+        .is_ok());
+
+        // But length fields that are not minimally encoded should be rejected, as they are not
+        // valid DER, cf.
+        // https://github.com/google/wycheproof/blob/2196000605e4/testvectors/ecdsa_secp256k1_sha256_test.json#L57-L66
+        assert!(Signature::from_asn1(&[
+            SEQUENCE_TAG,
+            0x81, // extended length: 1 length byte to come
+            0x06, // length of below
+            INTEGER_TAG,
+            0x01, // length of value
+            0x01, // value=1
+            INTEGER_TAG,
+            0x01, // length of value
+            0x01, // value=1
+        ])
+        .is_err());
+    }
 }