Pr 40

.github/workflows/rust.yml

@@ -10,6 +10,14 @@ env:
   CARGO_TERM_COLOR: always
 
 jobs:
+  clippy:
+    name: Clippy
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - name: Clippy
+      run: cargo clippy --all-features -- -D warnings 
+
   test-dev:
     name: Test Dev on ${{ matrix.os }}
     runs-on: ${{ matrix.os }}
@@ -19,12 +27,10 @@ jobs:
 
     steps:
     - uses: actions/checkout@v2
-    - name: Clippy
-      run: cargo clippy --all-features -- -D warnings 
     - name: Build
-      run: cargo build --verbose --all-features
+      run: cargo build --workspace --exclude simple-dns-fuzz --verbose --all-features
     - name: Run tests
-      run: cargo test --verbose --all-features
+      run: cargo test --workspace --exclude simple-dns-fuzz --verbose --all-features
 
   test-release:
     name: Test Release on ${{ matrix.os }}
@@ -35,9 +41,25 @@ jobs:
 
     steps:
     - uses: actions/checkout@v2
-    - name: Clippy
-      run: cargo clippy --release --all-features -- -D warnings 
     - name: Build
-      run: cargo build --verbose --release --all-features
+      run: cargo build --workspace --exclude simple-dns-fuzz --verbose --release --all-features
     - name: Run tests
-      run: cargo test --verbose --release --all-features
+      run: cargo test --workspace --exclude simple-dns-fuzz --verbose --release --all-features
+
+  fuzzing:
+    name: Fuzz 
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - name: Install Rust
+      uses: actions-rs/toolchain@v1
+      with:
+        toolchain: nightly
+        override: true
+    - name: Install cargo-fuzz
+      run: |
+        cargo install cargo-fuzz
+    - name: Run Fuzzing
+      run: |
+        cd simple-dns
+        cargo fuzz run packet_parse -- -max_total_time=300

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
     }
 }
 
@@ -84,22 +90,22 @@ impl<'a> TryFrom<&'a str> for CharacterString<'a> {
     }
 }
 
-impl<'a> TryFrom<String> for CharacterString<'a> {
+impl TryFrom<String> for CharacterString<'_> {
     type Error = crate::SimpleDnsError;
 
     fn try_from(value: String) -> Result<Self, Self::Error> {
         CharacterString::internal_new(Cow::Owned(value.as_bytes().into()))
     }
 }
 
-impl<'a> Display for CharacterString<'a> {
+impl Display for CharacterString<'_> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         let s = std::str::from_utf8(&self.data).unwrap();
         f.write_str(s)
     }
 }
 
-impl<'a> std::fmt::Debug for CharacterString<'a> {
+impl std::fmt::Debug for CharacterString<'_> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("CharacterString")
             .field("data", &self.to_string())

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
     }
 }
 
@@ -274,7 +276,7 @@ impl<'a, const N: usize> From<[Label<'a>; N]> for Name<'a> {
     }
 }
 
-impl<'a> Display for Name<'a> {
+impl Display for Name<'_> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         for (i, label) in self.iter().enumerate() {
             if i != 0 {
@@ -288,7 +290,7 @@ impl<'a> Display for Name<'a> {
     }
 }
 
-impl<'a> std::fmt::Debug for Name<'a> {
+impl std::fmt::Debug for Name<'_> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_tuple("Name")
             .field(&format!("{}", self))
@@ -297,13 +299,13 @@ impl<'a> std::fmt::Debug for Name<'a> {
     }
 }
 
-impl<'a> PartialEq for Name<'a> {
+impl PartialEq for Name<'_> {
     fn eq(&self, other: &Self) -> bool {
         self.labels == other.labels
     }
 }
 
-impl<'a> Hash for Name<'a> {
+impl Hash for Name<'_> {
     fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
         self.labels.hash(state);
     }
@@ -417,7 +419,7 @@ impl<'a> Label<'a> {
     }
 }
 
-impl<'a> Display for Label<'a> {
+impl Display for Label<'_> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match std::str::from_utf8(&self.data) {
             Ok(s) => f.write_str(s),
@@ -426,7 +428,7 @@ impl<'a> Display for Label<'a> {
     }
 }
 
-impl<'a> std::fmt::Debug for Label<'a> {
+impl std::fmt::Debug for Label<'_> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("Label")
             .field("data", &self.to_string())

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 {