Merge pull request #10 from Devolutions/fuzzer

Add fuzzer + fix issues found using it

Author: awakecoding

Cargo.lock

@@ -0,0 +1,16 @@
+# Changelog
+
+## [0.2.0] 2019-12-23
+
+### Added
+
+- Add `from_reader_with_max_len` deserialization function to limit how many bytes can be read at most.
+
+### Changed
+
+- `from_reader` function has a default limit of 10240 bytes before returning a truncated data error. Uses `from_reader_with_max_len` to change the limit.
+
+### Fixed
+
+- Fix various crash found by fuzzing.
+

picky-asn1-der/CHANGELOG.md

@@ -1,6 +1,6 @@
 [package]
 name = "picky-asn1-der"
-version = "0.1.0"
+version = "0.2.0"
 edition = "2018"
 authors = [
     "KizzyCode Software Labs./Keziah Biermann <[email protected]>",

picky-asn1-der/Cargo.toml

@@ -13,6 +13,8 @@ use picky_asn1::{tag::Tag, wrapper::*, Asn1Type};
 use serde::{de::Visitor, Deserialize};
 use std::io::{Cursor, Read};
 
+const DEFAULT_MAX_LEN: usize = 10240;
+
 /// Deserializes `T` from `bytes`
 pub fn from_bytes<'a, T: Deserialize<'a>>(bytes: &'a [u8]) -> Result<T> {
     debug_log!("deserialization using `from_bytes`");
@@ -22,8 +24,16 @@ pub fn from_bytes<'a, T: Deserialize<'a>>(bytes: &'a [u8]) -> Result<T> {
 
 /// Deserializes `T` from `reader`
 pub fn from_reader<'a, T: Deserialize<'a>>(reader: impl Read + 'a) -> Result<T> {
-    debug_log!("deserialization using `from_reader`");
-    let mut deserializer = Deserializer::new_from_reader(reader);
+    from_reader_with_max_len(reader, DEFAULT_MAX_LEN)
+}
+
+/// Deserializes `T` from `reader` reading at most n bytes.
+pub fn from_reader_with_max_len<'a, T: Deserialize<'a>>(reader: impl Read + 'a, max_len: usize) -> Result<T> {
+    debug_log!(
+        "deserialization using `from_reader_with_max_len`, max_len = {}",
+        max_len
+    );
+    let mut deserializer = Deserializer::new_from_reader(reader, max_len);
     T::deserialize(&mut deserializer)
 }
 
@@ -33,20 +43,22 @@ pub struct Deserializer<'de> {
     buf: Vec<u8>,
     encapsulator_tag_stack: Vec<Tag>,
     header_only: bool,
+    max_len: usize,
 }
 
 impl<'de> Deserializer<'de> {
     /// Creates a new deserializer over `bytes`
     pub fn new_from_bytes(bytes: &'de [u8]) -> Self {
-        Self::new_from_reader(Cursor::new(bytes))
+        Self::new_from_reader(Cursor::new(bytes), bytes.len())
     }
     /// Creates a new deserializer for `reader`
-    pub fn new_from_reader(reader: impl Read + 'de) -> Self {
+    pub fn new_from_reader(reader: impl Read + 'de, max_len: usize) -> Self {
         Self {
             reader: PeekableReader::new(Box::new(reader)),
             buf: Vec::new(),
             encapsulator_tag_stack: Vec::with_capacity(3),
             header_only: false,
+            max_len,
         }
     }
 
@@ -74,8 +86,13 @@ impl<'de> Deserializer<'de> {
             (tag, len)
         };
 
+        if len > self.max_len {
+            debug_log!("TRUNCATED DATA (invalid len: found {}, max is {})", len, self.max_len);
+            return Err(Asn1DerError::TruncatedData);
+        }
+
         self.buf.resize(len, 0);
-        self.reader.read_exact(&mut self.buf)?;
+        self.reader.read_exact(self.buf.as_mut_slice())?;
 
         Ok(tag)
     }
@@ -122,7 +139,7 @@ impl<'de> Deserializer<'de> {
                 };
             }
 
-            if peeked.len() < cursor {
+            if peeked.len() <= cursor {
                 debug_log!("peek_object: TRUNCATED DATA (couldn't read object tag)");
                 return Err(Asn1DerError::TruncatedData);
             }

picky-asn1-der/src/de/mod.rs

@@ -6,12 +6,14 @@ pub struct Sequence<'a, 'de> {
     de: &'a mut Deserializer<'de>,
     len: usize,
 }
+
 impl<'a, 'de> Sequence<'a, 'de> {
     /// Creates a lazy deserializer that can walk through the sequence's sub-elements
     pub fn deserialize_lazy(de: &'a mut Deserializer<'de>, len: usize) -> Self {
         Self { de, len }
     }
 }
+
 impl<'a, 'de> SeqAccess<'de> for Sequence<'a, 'de> {
     type Error = Asn1DerError;
 
@@ -24,7 +26,13 @@ impl<'a, 'de> SeqAccess<'de> for Sequence<'a, 'de> {
         // Deserialize the element
         let pos = self.de.reader.pos();
         let element = seed.deserialize(&mut *self.de)?;
-        self.len -= self.de.reader.pos() - pos;
+
+        let read = self.de.reader.pos() - pos;
+        if self.len < read {
+            debug_log!("TRUNCATED DATA (read more than necessary??)");
+            return Err(Asn1DerError::TruncatedData);
+        }
+        self.len -= read;
 
         Ok(Some(element))
     }

picky-asn1-der/src/de/sequence.rs

@@ -77,7 +77,7 @@ mod misc;
 mod ser;
 
 pub use crate::{
-    de::{from_bytes, from_reader, Deserializer},
+    de::{from_bytes, from_reader, from_reader_with_max_len, Deserializer},
     ser::{to_byte_buf, to_bytes, to_vec, to_writer, Serializer},
 };
 

picky-asn1-der/src/lib.rs

@@ -13,6 +13,7 @@ pub struct Sequence<'a, 'se> {
     buf: Cursor<Vec<u8>>,
     tag: Tag,
 }
+
 impl<'a, 'se> Sequence<'a, 'se> {
     /// Creates a lazy serializer that will serialize the sequence's sub-elements to `writer`
     pub fn serialize_lazy(ser: &'a mut Serializer<'se>, tag: Tag) -> Self {
@@ -28,6 +29,7 @@ impl<'a, 'se> Sequence<'a, 'se> {
         to_writer(value, &mut self.buf)?;
         Ok(())
     }
+
     /// Finalizes the sequence
     fn finalize(self) -> Result<usize> {
         // Reclaim buffer
@@ -39,6 +41,7 @@ impl<'a, 'se> Sequence<'a, 'se> {
         Ok(written)
     }
 }
+
 impl<'a, 'se> serde::ser::SerializeSeq for Sequence<'a, 'se> {
     type Ok = usize;
     type Error = Asn1DerError;
@@ -50,6 +53,7 @@ impl<'a, 'se> serde::ser::SerializeSeq for Sequence<'a, 'se> {
         self.finalize()
     }
 }
+
 impl<'a, 'se> serde::ser::SerializeTuple for Sequence<'a, 'se> {
     type Ok = usize;
     type Error = Asn1DerError;
@@ -61,6 +65,7 @@ impl<'a, 'se> serde::ser::SerializeTuple for Sequence<'a, 'se> {
         self.finalize()
     }
 }
+
 impl<'a, 'se> serde::ser::SerializeStruct for Sequence<'a, 'se> {
     type Ok = usize;
     type Error = Asn1DerError;
@@ -72,6 +77,7 @@ impl<'a, 'se> serde::ser::SerializeStruct for Sequence<'a, 'se> {
         self.finalize()
     }
 }
+
 impl<'a, 'se> serde::ser::SerializeTupleStruct for Sequence<'a, 'se> {
     type Ok = usize;
     type Error = Asn1DerError;

picky-asn1-der/src/ser/sequence.rs

@@ -1,6 +1,6 @@
 [package]
 name = "picky-server"
-version = "4.2.0"
+version = "4.2.1"
 authors = [
     "jtrepanier-devolutions <[email protected]>",
     "Benoît CORTIER <[email protected]>",
@@ -11,7 +11,7 @@ license = "MIT OR Apache-2.0"
 repository = "https://github.com/Devolutions/picky-rs"
 
 [dependencies]
-picky = { version = "4.2", default-features = false, features = ["x509", "chrono_conversion"] }
+picky = { version = "4.5", default-features = false, features = ["x509", "chrono_conversion"] }
 picky-asn1 = "0.1"
 mongodb = { package = "mongodb_cwal", version = "0.6", features = ["ssl"] }
 curl = { git = "https://github.com/Devolutions/curl-rust", branch = "wayk" }

picky-server/Cargo.toml

@@ -1,5 +1,5 @@
 use picky::{
-    key::{KeyError, OwnedPublicKey, PrivateKey},
+    key::{KeyError, PrivateKey, PublicKey},
     oids,
     pem::PemError,
     signature::SignatureHashType,
@@ -83,7 +83,7 @@ impl Picky {
 
     pub fn generate_intermediate(
         intermediate_name: &str,
-        intermediate_key: OwnedPublicKey,
+        intermediate_key: PublicKey,
         issuer_cert: &Cert,
         issuer_key: &PrivateKey,
         signature_hash_type: SignatureHashType,

picky-server/src/picky_controller.rs

@@ -1,6 +1,6 @@
 [package]
 name = "picky"
-version = "4.4.2"
+version = "4.5.0"
 authors = [
     "jtrepanier-devolutions <[email protected]>",
     "Benoît CORTIER <[email protected]>",
@@ -12,7 +12,7 @@ repository = "https://github.com/Devolutions/picky-rs"
 
 [dependencies]
 picky-asn1 = "0.1"
-picky-asn1-der = "0.1"
+picky-asn1-der = "0.2"
 serde = { version = "1.0", features = ["derive"] }
 oid = { version = "^0.1.1", features = ["serde_support"] }
 base64 = "0.10"