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 bf0da44b116cb0a17db63cde5016d91536495ddd..60f1507d299a421ace4ce510494dc7fe734bcccf 100644 (file)
--- 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 df59686deef3d529221fc95198ff055b20acc14f..ebaa0314bcae5f75428b2a897c208f092f048ddd 100644 (file)
--- 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 7d7b17f353d840030c30d71bc576b5b1fdb76947..0da71ab617011f6971a1867657c257bef1aaf73a 100644 (file)
--- 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 {