From: Lancer Cheng <b1tg@protonmail.ch>
Date: Tue, 14 Feb 2023 11:26:57 +0000 (+0000)
Subject: smb: fix parser of ntlmssp negotiateflags
X-Git-Tag: suricata-7.0.0-rc2~542
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9207012e4b683eebd3920a216f64ac4c87b78540;p=thirdparty%2Fsuricata.git

smb: fix parser of ntlmssp negotiateflags

Fix endian-conversion bug in function parse_ntlm_auth_nego_flags

Bug OISF#5783
---

diff --git a/rust/src/smb/auth.rs b/rust/src/smb/auth.rs
index 0953dcbf1b..3d57905d18 100644
--- a/rust/src/smb/auth.rs
+++ b/rust/src/smb/auth.rs
@@ -72,6 +72,7 @@ fn parse_secblob_spnego_start(blob: &[u8]) -> IResult<&[u8], &[u8], SecBlobError
     Ok((rem, d))
 }
 
+#[derive(Debug, PartialEq)]
 pub struct SpnegoRequest {
     pub krb: Option<Kerberos5Ticket>,
     pub ntlmssp: Option<NtlmsspData>,
@@ -229,3 +230,31 @@ pub fn parse_secblob(blob: &[u8]) -> Option<SpnegoRequest>
         },
     }
 }
+#[cfg(test)]
+mod tests {
+    use super::*;
+    #[test]
+    fn test_parse_secblob() {
+        // smb2.security_blob
+        let blob = hex::decode("a18202313082022da0030a0101a28202100482020c4e544c4d5353500003000000180018009c00000048014801b40000001e001e005800000008000800760000001e001e007e00000010001000fc010000158288e20a005a290000000fc6107a73184fb65fe684f6a1641464be4400450053004b0054004f0050002d0032004100450046004d003700470075007300650072004400450053004b0054004f0050002d0032004100450046004d003700470000000000000000000000000000000000000000000000000028a0c9f4e792c408913d2878feaa9a22010100000000000078a7ed218527d2010cf876f08a0b3bfa0000000002001e004400450053004b0054004f0050002d00560031004600410030005500510001001e004400450053004b0054004f0050002d00560031004600410030005500510004001e004400450053004b0054004f0050002d00560031004600410030005500510003001e004400450053004b0054004f0050002d0056003100460041003000550051000700080078a7ed218527d20106000400020000000800300030000000000000000100000000200000ad865b6d08a95d0e76a94e2ca013ab3f69c4fd945cca01b277700fd2b305ca010a001000000000000000000000000000000000000900280063006900660073002f003100390032002e003100360038002e003100390039002e003100330033000000000000000000000000005858824ec4a47b3b42ad3132ab84a5c3a31204100100000092302d756840453f00000000").unwrap();
+        let result = parse_secblob(&blob);
+        assert_eq!(
+            result,
+            Some(SpnegoRequest {
+                krb: None,
+                ntlmssp: Some(NtlmsspData {
+                    host: b"DESKTOP-2AEFM7G".to_vec(),
+                    user: b"user".to_vec(),
+                    domain: b"DESKTOP-2AEFM7G".to_vec(),
+                    version: Some(NTLMSSPVersion {
+                        ver_major: 10,
+                        ver_minor: 0,
+                        ver_build: 10586,
+                        ver_ntlm_rev: 15,
+                    },),
+                    warning: false,
+                }),
+            })
+        );
+    }
+}
diff --git a/rust/src/smb/ntlmssp_records.rs b/rust/src/smb/ntlmssp_records.rs
index c923421127..cde31cecc5 100644
--- a/rust/src/smb/ntlmssp_records.rs
+++ b/rust/src/smb/ntlmssp_records.rs
@@ -68,8 +68,8 @@ pub struct NTLMSSPAuthRecord<'a> {
     pub warning: bool,
 }
 
-fn parse_ntlm_auth_nego_flags(i: &[u8]) -> IResult<&[u8], (u8, u8, u32)> {
-    bits(tuple((take_bits(6u8), take_bits(1u8), take_bits(25u32))))(i)
+fn parse_ntlm_auth_nego_flags(i: &[u8]) -> IResult<&[u8], (u32, u8, u8)> {
+    bits(tuple((take_bits(25u8), take_bits(1u8), take_bits(6u32))))(i)
 }
 
 const NTLMSSP_IDTYPE_LEN: usize = 12;
@@ -154,3 +154,30 @@ pub fn parse_ntlmssp(i: &[u8]) -> IResult<&[u8], NTLMSSPRecord> {
     let record = NTLMSSPRecord { msg_type, data };
     Ok((i, record))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use nom7::Err;
+    #[test]
+    fn test_parse_auth_nego_flags() {
+        // ntlmssp.negotiateflags
+        let blob = [0x15, 0x82, 0x88, 0xe2];
+        let result = parse_ntlm_auth_nego_flags(&blob);
+        match result {
+            Ok((remainder, (_, version_flag, _))) => {
+                assert_eq!(version_flag, 1);
+                assert_eq!(remainder.len(), 0);
+            }
+            Err(Err::Error(err)) => {
+                panic!("Result should not be an error: {:?}.", err.code);
+            }
+            Err(Err::Incomplete(_)) => {
+                panic!("Result should not have been incomplete.");
+            }
+            _ => {
+                panic!("Unexpected behavior!");
+            }
+        }
+    }
+}