From: Jason Ish <jason.ish@oisf.net>
Date: Mon, 18 Apr 2022 16:32:25 +0000 (-0600)
Subject: smb: fix smb2 header flag parsing
X-Git-Tag: suricata-5.0.9~38
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=04f1d9e5a8928f1eed85014b63ecc5bf9307370f;p=thirdparty%2Fsuricata.git

smb: fix smb2 header flag parsing

The bits were being parsed in the order they're displayed in Wireshark,
rather than the order they were being seen on the wire, resulting in
direction and async being 0 more often than they should be.

Instead of bits, take the 4 bytes as an le_u32 and just use bit masks to
extract what we need into a struct, I think its easier to reason about
this way when comparing to the Microsoft documentation.

(cherry picked from commit 7b659489c85eaed4921ed9c4b97ecf827560376e)
---

diff --git a/rust/src/smb/smb2_records.rs b/rust/src/smb/smb2_records.rs
index 5a6bfd95d3..d5f2afb0b7 100644
--- a/rust/src/smb/smb2_records.rs
+++ b/rust/src/smb/smb2_records.rs
@@ -20,6 +20,9 @@ use nom::{rest, le_u8, le_u16, le_u32, le_u64, IResult};
 use crate::smb::smb::*;
 use crate::smb::nbss_records::NBSS_MSGTYPE_SESSION_MESSAGE;
 
+const SMB2_FLAGS_SERVER_TO_REDIR: u32 = 0x0000_0001;
+const SMB2_FLAGS_ASYNC_COMMAND: u32 = 0x0000_0002;
+
 #[derive(Debug,PartialEq)]
 pub struct Smb2SecBlobRecord<'a> {
     pub data: &'a[u8],
@@ -66,6 +69,22 @@ impl<'a> Smb2Record<'a> {
     }
 }
 
+#[derive(Debug)]
+struct SmbFlags {
+    direction: u8,
+    async_command: u8,
+}
+
+fn parse_smb2_flags(i: &[u8]) -> IResult<&[u8], SmbFlags> {
+    let (i, val) = le_u32(i)?;
+    let direction = if val & SMB2_FLAGS_SERVER_TO_REDIR != 0 { 1 } else { 0 };
+    let async_command = if val & SMB2_FLAGS_ASYNC_COMMAND != 0 { 1 } else { 0 };
+    Ok((i, SmbFlags {
+        direction,
+        async_command,
+    }))
+}
+
 named!(pub parse_smb2_request_record<Smb2Record>,
     do_parse!(
             _server_component: tag!(b"\xfeSMB")
@@ -75,16 +94,7 @@ named!(pub parse_smb2_request_record<Smb2Record>,
         >>  _reserved: take!(2)
         >>  command: le_u16
         >>  _credits_requested: le_u16
-        >>  flags: bits!(tuple!(
-                take_bits!(u8, 2),      // reserved / unused
-                take_bits!(u8, 1),      // replay op
-                take_bits!(u8, 1),      // dfs op
-                take_bits!(u32, 24),    // reserved / unused
-                take_bits!(u8, 1),      // signing
-                take_bits!(u8, 1),      // chained
-                take_bits!(u8, 1),      // async
-                take_bits!(u8, 1)       // response
-            ))
+        >>  flags: parse_smb2_flags
         >> chain_offset: le_u32
         >> message_id: le_u64
         >> _process_id: le_u32
@@ -95,7 +105,7 @@ named!(pub parse_smb2_request_record<Smb2Record>,
         >> data_c: cond!(chain_offset > hlen as u32, take!(chain_offset - hlen as u32))
         >> data_r: cond!(chain_offset <= hlen as u32, rest)
         >> (Smb2Record {
-                direction: flags.7,
+                direction: flags.direction,
                 nt_status: 0,
                 command:command,
                 message_id: message_id,
@@ -506,28 +516,19 @@ named!(pub parse_smb2_response_record<Smb2Record>,
         >>  nt_status: le_u32
         >>  command: le_u16
         >>  _credit_granted: le_u16
-        >>  flags: bits!(tuple!(
-                take_bits!(u8, 2),      // reserved / unused
-                take_bits!(u8, 1),      // replay op
-                take_bits!(u8, 1),      // dfs op
-                take_bits!(u32, 24),    // reserved / unused
-                take_bits!(u8, 1),      // signing
-                take_bits!(u8, 1),      // chained
-                take_bits!(u8, 1),      // async
-                take_bits!(u8, 1)       // response
-            ))
+        >>  flags: parse_smb2_flags
         >> chain_offset: le_u32
         >> message_id: le_u64
-        >> _process_id: cond!(flags.6==0, le_u32)
-        >> tree_id: cond!(flags.6==0, le_u32)
-        >> async_id: cond!(flags.6==1, le_u64)
+        >> _process_id: cond!(flags.async_command == 0, le_u32)
+        >> tree_id: cond!(flags.async_command == 0, le_u32)
+        >> async_id: cond!(flags.async_command == 1, le_u64)
         >> session_id: le_u64
         >> _signature: take!(16)
         // there is probably a cleaner way to do this
         >> data_c: cond!(chain_offset > hlen as u32, take!(chain_offset - hlen as u32))
         >> data_r: cond!(chain_offset <= hlen as u32, rest)
         >> (Smb2Record {
-                direction: flags.7,
+                direction: flags.direction,
                 nt_status: nt_status,
                 message_id: message_id,
                 tree_id: tree_id.unwrap_or(0),