From: Victor Julien <victor@inliniac.net>
Date: Wed, 28 Feb 2018 16:25:08 +0000 (+0100)
Subject: smb/nbss: work around bad traffic
X-Git-Tag: suricata-4.1.0-beta1~104
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7dff9b99696fe52eb8a11d20144056a41527ca3c;p=thirdparty%2Fsuricata.git

smb/nbss: work around bad traffic
---

diff --git a/rust/src/smb/nbss_records.rs b/rust/src/smb/nbss_records.rs
index c594529e78..fd062992ea 100644
--- a/rust/src/smb/nbss_records.rs
+++ b/rust/src/smb/nbss_records.rs
@@ -17,6 +17,13 @@
 
 use nom::{rest};
 
+pub const NBSS_MSGTYPE_SESSION_MESSAGE:         u8 = 0x00;
+pub const NBSS_MSGTYPE_SESSION_REQUEST:         u8 = 0x81;
+pub const NBSS_MSGTYPE_POSITIVE_SSN_RESPONSE:   u8 = 0x82;
+pub const NBSS_MSGTYPE_NEGATIVE_SSN_RESPONSE:   u8 = 0x83;
+pub const NBSS_MSGTYPE_RETARG_RESPONSE:         u8 = 0x84;
+pub const NBSS_MSGTYPE_KEEP_ALIVE:              u8 = 0x85;
+
 #[derive(Debug,PartialEq)]
 pub struct NbssRecord<'a> {
     pub message_type: u8,
@@ -24,6 +31,29 @@ pub struct NbssRecord<'a> {
     pub data: &'a[u8],
 }
 
+impl<'a> NbssRecord<'a> {
+    pub fn is_smb(&self) -> bool {
+        let valid = match self.message_type {
+            NBSS_MSGTYPE_SESSION_MESSAGE |
+            NBSS_MSGTYPE_SESSION_REQUEST |
+            NBSS_MSGTYPE_POSITIVE_SSN_RESPONSE |
+            NBSS_MSGTYPE_NEGATIVE_SSN_RESPONSE |
+            NBSS_MSGTYPE_RETARG_RESPONSE |
+            NBSS_MSGTYPE_KEEP_ALIVE => true,
+            _ => false,
+        };
+        let smb = if self.data.len() >= 4 &&
+            self.data[1] == 'S' as u8 && self.data[2] == 'M' as u8 && self.data[3] == 'B' as u8
+        {
+            true
+        } else {
+            false
+        };
+
+        valid && smb
+    }
+}
+
 named!(pub parse_nbss_record<NbssRecord>,
    do_parse!(
        type_and_len: bits!(tuple!(
@@ -37,20 +67,13 @@ named!(pub parse_nbss_record<NbssRecord>,
         })
 ));
 
-#[derive(Debug,PartialEq)]
-pub struct NbssRecordPartial<'a> {
-    pub message_type: u8,
-    pub length: u32,
-    pub data: &'a[u8],
-}
-
-named!(pub parse_nbss_record_partial<NbssRecordPartial>,
+named!(pub parse_nbss_record_partial<NbssRecord>,
    do_parse!(
        type_and_len: bits!(tuple!(
                take_bits!(u8, 8),
                take_bits!(u32, 24)))
        >> data: rest
-       >> (NbssRecordPartial {
+       >> (NbssRecord {
             message_type:type_and_len.0,
             length:type_and_len.1,
             data:data,
diff --git a/rust/src/smb/smb.rs b/rust/src/smb/smb.rs
index f2c4615738..0e3df09ff4 100644
--- a/rust/src/smb/smb.rs
+++ b/rust/src/smb/smb.rs
@@ -59,9 +59,6 @@ pub extern "C" fn rs_smb_init(context: &'static mut SuricataFileContext)
     }
 }
 
-pub const NBSS_MSGTYPE_SESSION_MESSAGE: u8 = 0x00;
-//const NBSS_MSGTYPE_SESSION_REQUEST: u8 = 0x81;
-
 pub const SMB_NTSTATUS_SUCCESS:                    u32 = 0;
 pub const SMB_NTSTATUS_PENDING:                    u32 = 0x00000103;
 pub const SMB_NTSTATUS_BUFFER_OVERFLOW:            u32 = 0x80000005;
@@ -1029,6 +1026,21 @@ impl SMBState {
     {
         SCLogDebug!("incomplete of size {}", input.len());
         if input.len() < 512 {
+            // check for malformed data. Wireshark reports as
+            // 'NBSS continuation data'. If it's invalid we're
+            // lost so we give up.
+            if input.len() > 8 {
+                match parse_nbss_record_partial(input) {
+                    IResult::Done(_, ref hdr) => {
+                        if !hdr.is_smb() {
+                            SCLogDebug!("partial NBSS, not SMB and no known msg type {}", hdr.message_type);
+                            self.trunc_ts();
+                            return 0;
+                        }
+                    },
+                    _ => {},
+                }
+            }
             return 0;
         }
 
@@ -1228,6 +1240,21 @@ impl SMBState {
     {
         SCLogDebug!("incomplete of size {}", input.len());
         if input.len() < 512 {
+            // check for malformed data. Wireshark reports as
+            // 'NBSS continuation data'. If it's invalid we're
+            // lost so we give up.
+            if input.len() > 8 {
+                match parse_nbss_record_partial(input) {
+                    IResult::Done(_, ref hdr) => {
+                        if !hdr.is_smb() {
+                            SCLogDebug!("partial NBSS, not SMB and no known msg type {}", hdr.message_type);
+                            self.trunc_tc();
+                            return 0;
+                        }
+                    },
+                    _ => {},
+                }
+            }
             return 0;
         }
 
@@ -1403,8 +1430,8 @@ impl SMBState {
                     }
                     cur_i = rem;
                 },
-                IResult::Incomplete(_) => {
-                    SCLogDebug!("INCOMPLETE have {}", cur_i.len());
+                IResult::Incomplete(needed) => {
+                    SCLogDebug!("INCOMPLETE have {} needed {:?}", cur_i.len(), needed);
                     let consumed = self.parse_tcp_data_tc_partial(cur_i);
                     cur_i = &cur_i[consumed ..];
 
@@ -1472,6 +1499,7 @@ impl SMBState {
     pub fn trunc_ts(&mut self) {
         SCLogDebug!("TRUNC TS");
         self.ts_trunc = true;
+        self.tcp_buffer_ts.clear();
 
         for tx in &mut self.transactions {
             if !tx.request_done {
@@ -1483,6 +1511,7 @@ impl SMBState {
     pub fn trunc_tc(&mut self) {
         SCLogDebug!("TRUNC TC");
         self.tc_trunc = true;
+        self.tcp_buffer_tc.clear();
 
         for tx in &mut self.transactions {
             if !tx.response_done {