From fc1e220a999c2a4d298bfd9765341a37042ff3f8 Mon Sep 17 00:00:00 2001
From: Josh Megnauth <jo.sh@tutanota.com>
Date: Mon, 18 Nov 2024 13:43:22 +0000
Subject: [PATCH] fix(netdb): Buffer overrun when parsing DNS

---
 src/header/netdb/lookup.rs | 116 +++++++++++++++++++++++++++++++++++--
 1 file changed, 111 insertions(+), 5 deletions(-)

diff --git a/src/header/netdb/lookup.rs b/src/header/netdb/lookup.rs
index 4b267b2943..4656015932 100644
--- a/src/header/netdb/lookup.rs
+++ b/src/header/netdb/lookup.rs
@@ -247,16 +247,122 @@ pub fn lookup_addr(addr: in_addr) -> Result<Vec<Vec<u8>>, c_int> {
 }
 
 fn parse_revdns_answer(data: &[u8]) -> Vec<u8> {
+    if data.is_empty() || data[0] == 0 {
+        return vec![0];
+    }
+
     let mut cursor = 0;
     let mut index = 0;
-    let mut output = data.to_vec();
-    while index < data.len() - 1 {
+    // First byte is a length; discard
+    let mut output = data[1..].to_vec();
+    let length = data.len() - 1;
+    while index < length {
         let offset = data[index] as usize;
+        // CVE-2024-21342
+        if offset > length {
+            return vec![0];
+        }
         index = cursor + offset + 1;
-        output[index] = b'.';
+        // First byte was skipped so index is one less
+        output[index - 1] = b'.';
         cursor = index;
     }
-    //we don't want an extra period at the end
-    output.pop();
+    // Response is NUL terminated so we must preserve that for C
+    match output.last_mut() {
+        Some(nul) => *nul = b'\0',
+        // XXX: Likely unreachable
+        None => {
+            debug_assert!(output.is_empty());
+            output = vec![0];
+        }
+    }
     output
 }
+
+#[cfg(test)]
+mod tests {
+    use alloc::str;
+    use core::ffi::CStr;
+
+    use super::parse_revdns_answer;
+
+    // Actual response from a query
+    const DNS_GOOGLE: &[u8] = &[3, 100, 110, 115, 6, 103, 111, 111, 103, 108, 101, 0];
+    const EXPECTED_DNS_GOOGLE: &str = "dns.google\0";
+    const EXPECTED_DNS_GOOGLE_RT: &str = "dns.google";
+
+    // Fake response that has numbers within the name (e.g. like CDNs)
+    const FAKE_WITH_NUMS: &[u8] = &[
+        14, 102, 97, 107, 101, 45, 49, 48, 48, 45, 50, 45, 51, 45, 52, 8, 102, 111, 111, 98, 97,
+        114, 50, 52, 4, 102, 97, 107, 101, 0,
+    ];
+    const EXPECTED_FAKE_RESPONSE: &str = "fake-100-2-3-4.foobar24.fake\0";
+    const EXPECTED_FAKE_RESPONSE_RT: &str = "fake-100-2-3-4.foobar24.fake";
+
+    const EMPTY_RESPONSE: &[u8] = &[0];
+    const EXPECTED_EMPTY_RESPONSE: &str = "\0";
+    const EXPECTED_EMPTY_RESPONSE_RT: &str = "";
+
+    #[test]
+    fn dns_response_dns_google() {
+        let response = parse_revdns_answer(DNS_GOOGLE);
+        assert_eq!(
+            0,
+            *response.last().unwrap(),
+            "Response should end with a NUL byte"
+        );
+
+        let response_str = str::from_utf8(&response)
+            .expect("Response is valid UTF-8; parsing shouldn't change that");
+        assert_eq!(EXPECTED_DNS_GOOGLE, response_str);
+
+        let response_cstr = CStr::from_bytes_with_nul(&response)
+            .expect("Parsed response should have only one NUL byte");
+        let response_cstr_str = response_cstr
+            .to_str()
+            .expect("Valid UTF-8 bytes to CStr to Rust str should be valid");
+        assert_eq!(EXPECTED_DNS_GOOGLE_RT, response_cstr_str);
+    }
+
+    #[test]
+    fn dns_response_fake_with_nums() {
+        let response = parse_revdns_answer(FAKE_WITH_NUMS);
+        assert_eq!(
+            0,
+            *response.last().unwrap(),
+            "Response should end with a NUL byte"
+        );
+
+        let response_str = str::from_utf8(&response)
+            .expect("Response is valid UTF-8; parsing shouldn't change that");
+        assert_eq!(EXPECTED_FAKE_RESPONSE, response_str);
+
+        let response_cstr = CStr::from_bytes_with_nul(&response)
+            .expect("Parsed response should have only one NUL byte");
+        let response_cstr_str = response_cstr
+            .to_str()
+            .expect("Valid UTF-8 bytes to CStr to Rust str should be valid");
+        assert_eq!(EXPECTED_FAKE_RESPONSE_RT, response_cstr_str);
+    }
+
+    #[test]
+    fn dns_response_empty() {
+        let response = parse_revdns_answer(EMPTY_RESPONSE);
+        assert_eq!(
+            0,
+            *response.last().unwrap(),
+            "Response should end with a NUL byte"
+        );
+
+        let response_str = str::from_utf8(&response)
+            .expect("Response is valid UTF-8; parsing shouldn't change that");
+        assert_eq!(EXPECTED_EMPTY_RESPONSE, response_str);
+
+        let response_cstr = CStr::from_bytes_with_nul(&response)
+            .expect("Parsed response should have only one NUL byte");
+        let response_cstr_str = response_cstr
+            .to_str()
+            .expect("Valid UTF-8 bytes to CStr to Rust str should be valid");
+        assert_eq!(EXPECTED_EMPTY_RESPONSE_RT, response_cstr_str);
+    }
+}