From: Victor Julien <victor@inliniac.net>
Date: Sat, 24 Mar 2018 09:30:26 +0000 (+0100)
Subject: smb: improve skip handling
X-Git-Tag: suricata-4.1.0-rc1~197
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=aa8d64c2b8026444d3cabb4495794d3d29f17acf;p=thirdparty%2Fsuricata.git

smb: improve skip handling

When skipping records the skip tracker could underflow if the record
parsing had more data than expected.

Enforce the calculation by moving it into a method and make the actual
fields private.
---

diff --git a/rust/src/smb/smb.rs b/rust/src/smb/smb.rs
index bf0da44b11..60f1507d29 100644
--- a/rust/src/smb/smb.rs
+++ b/rust/src/smb/smb.rs
@@ -698,8 +698,8 @@ pub struct SMBState<> {
 
     pub files: SMBFiles,
 
-    pub skip_ts: u32,
-    pub skip_tc: u32,
+    skip_ts: u32,
+    skip_tc: u32,
 
     pub file_ts_left : u32,
     pub file_tc_left : u32,
@@ -1107,6 +1107,16 @@ impl SMBState {
         }
     }
 
+    pub fn set_skip(&mut self, direction: u8, rec_size: u32, data_size: u32)
+    {
+        let skip = if data_size >= rec_size { 0 } else { rec_size - data_size };
+        if direction == STREAM_TOSERVER {
+            self.skip_ts = skip;
+        } else {
+            self.skip_tc = skip;
+        }
+    }
+
     // return how much data we consumed
     fn handle_skip(&mut self, direction: u8, input_size: u32) -> u32 {
         let mut skip_left = if direction == STREAM_TOSERVER {
diff --git a/rust/src/smb/smb1.rs b/rust/src/smb/smb1.rs
index df59686dee..ebaa0314bc 100644
--- a/rust/src/smb/smb1.rs
+++ b/rust/src/smb/smb1.rs
@@ -887,7 +887,7 @@ pub fn smb1_read_response_record<'b>(state: &mut SMBState, r: &SmbRecord<'b>)
                     None => {
                         SCLogDebug!("SMBv1 READ response: reply to unknown request: left {} {:?}",
                                 rd.len - rd.data.len() as u32, rd);
-                        state.skip_tc = rd.len - rd.data.len() as u32;
+                        state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32);
                         return;
                     },
                 };
diff --git a/rust/src/smb/smb2.rs b/rust/src/smb/smb2.rs
index 7d7b17f353..0da71ab617 100644
--- a/rust/src/smb/smb2.rs
+++ b/rust/src/smb/smb2.rs
@@ -130,8 +130,8 @@ pub fn smb2_read_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>)
             let (offset, file_guid) = match state.ssn2vecoffset_map.remove(&guid_key) {
                 Some(o) => (o.offset, o.guid),
                 None => {
-                    SCLogDebug!("SMBv2 READ response: reply to unknown request");
-                    state.skip_tc = rd.len - rd.data.len() as u32;
+                    SCLogDebug!("SMBv2 READ response: reply to unknown request {:?}",rd);
+                    state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32);
                     return;
                 },
             };
@@ -166,7 +166,7 @@ pub fn smb2_read_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>)
                     smb_read_dcerpc_record(state, vercmd, hdr, &file_guid, rd.data);
                 } else if is_pipe {
                     SCLogDebug!("non-DCERPC pipe");
-                    state.skip_tc = rd.len - rd.data.len() as u32;
+                    state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32);
                 } else {
                     let file_name = match state.guid2name_map.get(&file_guid) {
                         Some(n) => { n.to_vec() },
@@ -243,7 +243,7 @@ pub fn smb2_write_request_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>)
                     smb_write_dcerpc_record(state, vercmd, hdr, wr.data);
                 } else if is_pipe {
                     SCLogDebug!("non-DCERPC pipe: skip rest of the record");
-                    state.skip_ts = wr.wr_len - wr.data.len() as u32;
+                    state.set_skip(STREAM_TOSERVER, wr.wr_len, wr.data.len() as u32);
                 } else {
                     let (tx, files, flags) = state.new_file_tx(&file_guid, &file_name, STREAM_TOSERVER);
                     if let Some(SMBTransactionTypeData::FILE(ref mut tdf)) = tx.type_data {