]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
dns: better error handling when parsing names
authorJason Ish <jason.ish@oisf.net>
Tue, 1 Feb 2022 21:44:43 +0000 (15:44 -0600)
committerVictor Julien <vjulien@oisf.net>
Tue, 19 Apr 2022 21:56:01 +0000 (23:56 +0200)
The DNS name parser will error out with an error even if the
error is incomplete. Instead of manually generating errors,
use '?' to let the nom error ripple up the error handling chain.

The reason this wasn't done in the first place is this code
predates the ? operator, or we were not aware of it at the time.

This prevents the case where probing fails when there is enough data to
parse the header, but not enough to complete name parser. In such a case
a parse error is returned (instead of incomplete) resulting in the
payload not being detected as DNS.

Ticket #5034

rust/src/dns/parser.rs

index 2b1a30582bb9b36b4306160f94d524081bada964..6dd6466c0d7b26486b22d641ca003f2b71548768 100644 (file)
@@ -67,33 +67,21 @@ pub fn dns_parse_name<'a, 'b>(start: &'b [u8], message: &'b [u8]) -> IResult<&'b
             pos = &pos[1..];
             break;
         } else if len & 0b1100_0000 == 0 {
-            match length_data(be_u8)(pos) as IResult<&[u8], _> {
-                Ok((rem, label)) => {
-                    if name.len() > 0 {
-                        name.push('.' as u8);
-                    }
-                    name.extend(label);
-                    pos = rem;
-                }
-                _ => {
-                    return Err(Err::Error(error_position!(pos, ErrorKind::OctDigit)));
-                }
+            let (rem, label) = length_data(be_u8)(pos)?;
+            if name.len() > 0 {
+                name.push('.' as u8);
             }
+            name.extend(label);
+            pos = rem;
         } else if len & 0b1100_0000 == 0b1100_0000 {
-            match be_u16(pos) as IResult<&[u8], _> {
-                Ok((rem, leader)) => {
-                    let offset = usize::from(leader) & 0x3fff;
-                    if offset > message.len() {
-                        return Err(Err::Error(error_position!(pos, ErrorKind::OctDigit)));
-                    }
-                    pos = &message[offset..];
-                    if pivot == start {
-                        pivot = rem;
-                    }
-                }
-                _ => {
-                    return Err(Err::Error(error_position!(pos, ErrorKind::OctDigit)));
-                }
+            let (rem, leader) = be_u16(pos)?;
+            let offset = usize::from(leader) & 0x3fff;
+            if offset > message.len() {
+                return Err(Err::Error(error_position!(pos, ErrorKind::OctDigit)));
+            }
+            pos = &message[offset..];
+            if pivot == start {
+                pivot = rem;
             }
         } else {
             return Err(Err::Error(error_position!(pos, ErrorKind::OctDigit)));