fix(simple-dns): segfaults in WireFormat::parse()

Cargo.toml

@@ -1,5 +1,5 @@
 [workspace]
 resolver = "2"
 
-members = ["simple-dns", "simple-mdns"]
+members = ["simple-dns", "simple-mdns", "simple-dns/fuzz"]
 

simple-dns/fuzz/.gitignore

@@ -0,0 +1,4 @@
+target
+corpus
+artifacts
+coverage

simple-dns/fuzz/Cargo.toml

@@ -0,0 +1,21 @@
+[package]
+name = "simple-dns-fuzz"
+version = "0.0.0"
+publish = false
+edition = "2021"
+
+[package.metadata]
+cargo-fuzz = true
+
+[dependencies]
+libfuzzer-sys = "0.4"
+
+[dependencies.simple-dns]
+path = ".."
+
+[[bin]]
+name = "packet_parse"
+path = "fuzz_targets/packet_parse.rs"
+test = false
+doc = false
+bench = false

simple-dns/fuzz/fuzz_targets/packet_parse.rs

@@ -0,0 +1,18 @@
+#![no_main]
+
+use libfuzzer_sys::fuzz_target;
+
+use simple_dns::Packet;
+
+fuzz_target!(|data: &[u8]| {
+    // fuzzed code goes here
+    if let Ok(original) = Packet::parse(data) {
+        let compressed = original.build_bytes_vec_compressed().unwrap();
+
+        if let Ok(decompressed) = Packet::parse(&compressed) {
+            let encoded = decompressed.build_bytes_vec().unwrap();
+
+            assert_eq!(encoded, original.build_bytes_vec().unwrap());
+        }
+    }
+});

simple-dns/src/dns/character_string.rs

@@ -48,7 +48,9 @@ impl<'a> TryFrom<CharacterString<'a>> for String {
 }
 
 impl<'a> WireFormat<'a> for CharacterString<'a> {
-    fn parse(data: &'a [u8], position: &mut usize) -> crate::Result<Self>
+    const MINIMUM_LEN: usize = 1;
+
+    fn parse_after_check(data: &'a [u8], position: &mut usize) -> crate::Result<Self>
     where
         Self: Sized,
     {
@@ -57,6 +59,10 @@ impl<'a> WireFormat<'a> for CharacterString<'a> {
             return Err(SimpleDnsError::InvalidCharacterString);
         }
 
+        if *position + 1 + length > data.len() {
+            return Err(crate::SimpleDnsError::InsufficientData);
+        }
+
         let data = &data[*position + 1..*position + 1 + length];
         *position += length + 1;
 
@@ -72,7 +78,7 @@ impl<'a> WireFormat<'a> for CharacterString<'a> {
     }
 
     fn len(&self) -> usize {
-        self.data.len() + 1
+        self.data.len() + Self::MINIMUM_LEN
     }
 }
 

simple-dns/src/dns/name.rs

@@ -73,7 +73,7 @@ impl<'a> Name<'a> {
     }
 
     /// Returns an Iter of this Name Labels
-    pub fn iter(&'a self) -> std::slice::Iter<Label<'a>> {
+    pub fn iter(&'a self) -> std::slice::Iter<'a, Label<'a>> {
         self.labels.iter()
     }
 
@@ -160,7 +160,9 @@ impl<'a> Name<'a> {
 }
 
 impl<'a> WireFormat<'a> for Name<'a> {
-    fn parse(data: &'a [u8], position: &mut usize) -> crate::Result<Self>
+    const MINIMUM_LEN: usize = 1;
+
+    fn parse_after_check(data: &'a [u8], position: &mut usize) -> crate::Result<Self>
     where
         Self: Sized,
     {
@@ -173,7 +175,7 @@ impl<'a> WireFormat<'a> for Name<'a> {
         let mut name_size = 0usize;
 
         loop {
-            if *position >= data.len() {
+            if pointer_position >= data.len() {
                 return Err(crate::SimpleDnsError::InsufficientData);
             }
 
@@ -250,7 +252,7 @@ impl<'a> WireFormat<'a> for Name<'a> {
             .iter()
             .map(|label| label.len() + 1)
             .sum::<usize>()
-            + 1
+            + Self::MINIMUM_LEN
     }
 }
 

simple-dns/src/dns/question.rs

@@ -53,11 +53,17 @@ impl<'a> Question<'a> {
 }
 
 impl<'a> WireFormat<'a> for Question<'a> {
+    const MINIMUM_LEN: usize = 4;
+
+    // Disable redundant length check.
     fn parse(data: &'a [u8], position: &mut usize) -> crate::Result<Self> {
+        Self::parse_after_check(data, position)
+    }
+
+    fn parse_after_check(data: &'a [u8], position: &mut usize) -> crate::Result<Self> {
         let qname = Name::parse(data, position)?;
-        if *position + 4 > data.len() {
-            return Err(crate::SimpleDnsError::InsufficientData);
-        }
+
+        Self::check_len(data, position)?;
 
         let qtype = u16::from_be_bytes(data[*position..*position + 2].try_into()?);
         let qclass = u16::from_be_bytes(data[*position + 2..*position + 4].try_into()?);
@@ -73,7 +79,7 @@ impl<'a> WireFormat<'a> for Question<'a> {
     }
 
     fn len(&self) -> usize {
-        self.qname.len() + 4
+        self.qname.len() + Self::MINIMUM_LEN
     }
 
     fn write_to<T: std::io::Write>(&self, out: &mut T) -> crate::Result<()> {

simple-dns/src/dns/rdata/a.rs

@@ -15,7 +15,9 @@ impl RR for A {
 }
 
 impl<'a> WireFormat<'a> for A {
-    fn parse(data: &'a [u8], position: &mut usize) -> crate::Result<Self>
+    const MINIMUM_LEN: usize = 4;
+
+    fn parse_after_check(data: &'a [u8], position: &mut usize) -> crate::Result<Self>
     where
         Self: Sized,
     {
@@ -28,10 +30,6 @@ impl<'a> WireFormat<'a> for A {
         out.write_all(&self.address.to_be_bytes())
             .map_err(crate::SimpleDnsError::from)
     }
-
-    fn len(&self) -> usize {
-        4
-    }
 }
 
 impl A {

simple-dns/src/dns/rdata/aaaa.rs

@@ -15,7 +15,9 @@ impl RR for AAAA {
 }
 
 impl<'a> WireFormat<'a> for AAAA {
-    fn parse(data: &'a [u8], position: &mut usize) -> crate::Result<Self>
+    const MINIMUM_LEN: usize = 16;
+
+    fn parse_after_check(data: &'a [u8], position: &mut usize) -> crate::Result<Self>
     where
         Self: Sized,
     {
@@ -24,10 +26,6 @@ impl<'a> WireFormat<'a> for AAAA {
         Ok(Self { address })
     }
 
-    fn len(&self) -> usize {
-        16
-    }
-
     fn write_to<T: std::io::Write>(&self, out: &mut T) -> crate::Result<()> {
         out.write_all(&self.address.to_be_bytes())
             .map_err(crate::SimpleDnsError::from)

simple-dns/src/dns/rdata/afsdb.rs

@@ -28,7 +28,9 @@ impl<'a> AFSDB<'a> {
 }
 
 impl<'a> WireFormat<'a> for AFSDB<'a> {
-    fn parse(data: &'a [u8], position: &mut usize) -> crate::Result<Self>
+    const MINIMUM_LEN: usize = 2;
+
+    fn parse_after_check(data: &'a [u8], position: &mut usize) -> crate::Result<Self>
     where
         Self: Sized,
     {
@@ -54,7 +56,7 @@ impl<'a> WireFormat<'a> for AFSDB<'a> {
     }
 
     fn len(&self) -> usize {
-        self.hostname.len() + 2
+        self.hostname.len() + Self::MINIMUM_LEN
     }
 }
 

simple-dns/src/dns/rdata/caa.rs

@@ -33,7 +33,9 @@ impl<'a> CAA<'a> {
 }
 
 impl<'a> WireFormat<'a> for CAA<'a> {
-    fn parse(data: &'a [u8], position: &mut usize) -> crate::Result<Self>
+    const MINIMUM_LEN: usize = 1;
+
+    fn parse_after_check(data: &'a [u8], position: &mut usize) -> crate::Result<Self>
     where
         Self: Sized,
     {
@@ -54,7 +56,7 @@ impl<'a> WireFormat<'a> for CAA<'a> {
     }
 
     fn len(&self) -> usize {
-        self.tag.len() + self.value.len() + 1
+        self.tag.len() + self.value.len() + Self::MINIMUM_LEN
     }
 }
 

simple-dns/src/dns/rdata/cert.rs

@@ -16,13 +16,14 @@ pub struct CERT<'a> {
     pub certificate: Cow<'a, [u8]>,
 }
 
-
 impl<'a> RR for CERT<'a> {
     const TYPE_CODE: u16 = 37;
 }
 
 impl<'a> WireFormat<'a> for CERT<'a> {
-    fn parse(data: &'a [u8], position: &mut usize) -> crate::Result<Self>
+    const MINIMUM_LEN: usize = 5;
+
+    fn parse_after_check(data: &'a [u8], position: &mut usize) -> crate::Result<Self>
     where
         Self: Sized,
     {
@@ -53,7 +54,7 @@ impl<'a> WireFormat<'a> for CERT<'a> {
     }
 
     fn len(&self) -> usize {
-        5 + self.certificate.len()
+        self.certificate.len() + Self::MINIMUM_LEN
     }
 }
 
@@ -108,12 +109,8 @@ mod tests {
         assert_eq!(sample_rdata.type_code, 3);
         assert_eq!(sample_rdata.key_tag, 0);
         assert_eq!(sample_rdata.algorithm, 0);
-        assert_eq!(
-            *sample_rdata.certificate,
-            *b"\x00\x00\x00\x00\x00"
-        );
-
+        assert_eq!(*sample_rdata.certificate, *b"\x00\x00\x00\x00\x00");
+
         Ok(())
     }
-
 }

simple-dns/src/dns/rdata/dhcid.rs

@@ -11,15 +11,17 @@ pub struct DHCID<'a> {
     /// Digest type code
     pub digest_type: u8,
     /// Digest (length depends on digest type)
-    pub digest: Cow<'a, [u8]>
+    pub digest: Cow<'a, [u8]>,
 }
 
 impl<'a> RR for DHCID<'a> {
     const TYPE_CODE: u16 = 49;
 }
 
 impl<'a> WireFormat<'a> for DHCID<'a> {
-    fn parse(data: &'a [u8], position: &mut usize) -> crate::Result<Self>
+    const MINIMUM_LEN: usize = 3;
+
+    fn parse_after_check(data: &'a [u8], position: &mut usize) -> crate::Result<Self>
     where
         Self: Sized,
     {
@@ -48,7 +50,7 @@ impl<'a> WireFormat<'a> for DHCID<'a> {
     }
 
     fn len(&self) -> usize {
-        2 + 1 + self.digest.len()
+        self.digest.len() + Self::MINIMUM_LEN
     }
 }
 
@@ -101,5 +103,4 @@ mod tests {
 
         Ok(())
     }
-
 }