smb: handles records with trailing nbss data

If a file (read/write) SMB record has padding/trailing data
after the buffer being read or written, and that Suricata falls
in one case where it skips the data, it should skip until
the very end of the NBSS record, meaning it should also skip the
padding/trailing data.

Otherwise, an attacker may smuggle some NBSS/SMB record in this
trailing data, that will be interpreted by Suricata, but not
by the SMB client/server, leading to evasions.

Ticket: #5786
(cherry picked from commit 233ab1114873e632014b58d91c351505417fe8ed)

diff --git a/rust/src/smb/smb.rs b/rust/src/smb/smb.rs
index 2e194e7f339894bdec06b168782f14b9bcd7e3df..66e12ba406c127ec719e635fdf56f104d309ba22 100644 (file)
--- a/rust/src/smb/smb.rs
+++ b/rust/src/smb/smb.rs
@@ -1264,13 +1264,12 @@ impl SMBState {
         }
     }
 
-    pub fn set_skip(&mut self, direction: u8, rec_size: u32, data_size: u32)
+    pub fn set_skip(&mut self, direction: u8, nbss_remaining: u32)
     {
-        let skip = rec_size.saturating_sub(data_size);
         if direction == STREAM_TOSERVER {
-            self.skip_ts = skip;
+            self.skip_ts = nbss_remaining;
         } else {
-            self.skip_tc = skip;
+            self.skip_tc = nbss_remaining;
         }
     }
 

diff --git a/rust/src/smb/smb1.rs b/rust/src/smb/smb1.rs
index 0de13a5052569c35f0be2023fcc91b96d6c001da..85181f71fe765279429eec312f105d4992ae5f23 100644 (file)
--- a/rust/src/smb/smb1.rs
+++ b/rust/src/smb/smb1.rs
@@ -890,7 +890,7 @@ pub fn smb1_write_request_record<'b>(state: &mut SMBState, r: &SmbRecord<'b>, nb
                 // Record claims more bytes than are in NBSS record...
                 state.set_event(SMBEvent::WriteRequestTooLarge);
                 // Skip the remaining bytes of the record.
-                state.set_skip(STREAM_TOSERVER, nbss_remaining, 0);
+                state.set_skip(STREAM_TOSERVER, nbss_remaining);
                 return;
             }
             let mut file_fid = rd.fid.to_vec();
@@ -976,7 +976,7 @@ pub fn smb1_read_response_record<'b>(state: &mut SMBState, r: &SmbRecord<'b>, nb
                     // Record claims more bytes than are in NBSS record...
                     state.set_event(SMBEvent::ReadResponseTooLarge);
                     // Skip the remaining bytes of the record.
-                    state.set_skip(STREAM_TOCLIENT, nbss_remaining, 0);
+                    state.set_skip(STREAM_TOCLIENT, nbss_remaining);
                     return;
                 }
                 let fid_key = SMBCommonHdr::from1(r, SMBHDR_TYPE_OFFSET);
@@ -985,7 +985,7 @@ pub fn smb1_read_response_record<'b>(state: &mut SMBState, r: &SmbRecord<'b>, nb
                     None => {
                         SCLogDebug!("SMBv1 READ response: reply to unknown request: left {} {:?}",
                                 rd.len - rd.data.len() as u32, rd);
-                        state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32);
+                        state.set_skip(STREAM_TOCLIENT, nbss_remaining);
                         return;
                     },
                 };

diff --git a/rust/src/smb/smb2.rs b/rust/src/smb/smb2.rs
index af3bf174b2057e31be2cb27739aacc042728b2fc..aaa31df07719f3e34827a12901f0b64f3cd78a85 100644 (file)
--- a/rust/src/smb/smb2.rs
+++ b/rust/src/smb/smb2.rs
@@ -125,7 +125,7 @@ pub fn smb2_read_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n
                 // Record claims more bytes than are in NBSS record...
                 state.set_event(SMBEvent::ReadResponseTooLarge);
                 // Skip the remaining bytes of the record.
-                state.set_skip(STREAM_TOCLIENT, nbss_remaining, 0);
+                state.set_skip(STREAM_TOCLIENT, nbss_remaining);
                 return;
             }
             if r.nt_status == SMB_NTSTATUS_BUFFER_OVERFLOW {
@@ -134,7 +134,7 @@ pub fn smb2_read_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n
 
             } else if r.nt_status != SMB_NTSTATUS_SUCCESS {
                 SCLogDebug!("SMBv2: read response error code received: skip record");
-                state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32);
+                state.set_skip(STREAM_TOCLIENT, nbss_remaining);
                 return;
             }
 
@@ -142,7 +142,7 @@ pub fn smb2_read_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n
                (unsafe { SMB_CFG_MAX_READ_SIZE != 0 && SMB_CFG_MAX_READ_SIZE < rd.len })
             {
                 state.set_event(SMBEvent::ReadResponseTooLarge);
-                state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32);
+                state.set_skip(STREAM_TOCLIENT, nbss_remaining);
                 return;
             }
 
@@ -155,7 +155,7 @@ pub fn smb2_read_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n
                 Some(o) => (o.offset, o.guid),
                 None => {
                     SCLogDebug!("SMBv2 READ response: reply to unknown request {:?}",rd);
-                    state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32);
+                    state.set_skip(STREAM_TOCLIENT, nbss_remaining);
                     return;
                 },
             };
@@ -172,10 +172,10 @@ pub fn smb2_read_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n
                         }
                         if max_queue_size != 0 && tdf.file_tracker.get_inflight_size() + rd.len as u64 > max_queue_size.into() {
                             state.set_event(SMBEvent::ReadQueueSizeExceeded);
-                            state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32);
+                            state.set_skip(STREAM_TOCLIENT, nbss_remaining);
                         } else if max_queue_cnt != 0 && tdf.file_tracker.get_inflight_cnt() >= max_queue_cnt as usize {
                             state.set_event(SMBEvent::ReadQueueCntExceeded);
-                            state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32);
+                            state.set_skip(STREAM_TOCLIENT, nbss_remaining);
                         } else {
                             filetracker_newchunk(&mut tdf.file_tracker, files, flags,
                                     &tdf.file_name, rd.data, offset,
@@ -228,7 +228,7 @@ pub fn smb2_read_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n
                     smb_read_dcerpc_record(state, vercmd, hdr, &file_guid, rd.data);
                 } else if is_pipe {
                     SCLogDebug!("non-DCERPC pipe");
-                    state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32);
+                    state.set_skip(STREAM_TOCLIENT, nbss_remaining);
                 } else {
                     let file_name = match state.guid2name_map.get(&file_guid) {
                         Some(n) => { n.to_vec() }
@@ -248,10 +248,10 @@ pub fn smb2_read_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n
                         }
                         if max_queue_size != 0 && tdf.file_tracker.get_inflight_size() + rd.len as u64 > max_queue_size.into() {
                             state.set_event(SMBEvent::ReadQueueSizeExceeded);
-                            state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32);
+                            state.set_skip(STREAM_TOCLIENT, nbss_remaining);
                         } else if max_queue_cnt != 0 && tdf.file_tracker.get_inflight_cnt() >= max_queue_cnt as usize {
                             state.set_event(SMBEvent::ReadQueueCntExceeded);
-                            state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32);
+                            state.set_skip(STREAM_TOCLIENT, nbss_remaining);
                         } else {
                             filetracker_newchunk(&mut tdf.file_tracker, files, flags,
                                     &file_name, rd.data, offset,
@@ -290,13 +290,13 @@ pub fn smb2_write_request_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n
                 // Record claims more bytes than are in NBSS record...
                 state.set_event(SMBEvent::WriteRequestTooLarge);
                 // Skip the remaining bytes of the record.
-                state.set_skip(STREAM_TOSERVER, nbss_remaining, 0);
+                state.set_skip(STREAM_TOSERVER, nbss_remaining);
                 return;
             }
             if (state.max_write_size != 0 && wr.wr_len > state.max_write_size) ||
                (unsafe { SMB_CFG_MAX_WRITE_SIZE != 0 && SMB_CFG_MAX_WRITE_SIZE < wr.wr_len }) {
                 state.set_event(SMBEvent::WriteRequestTooLarge);
-                state.set_skip(STREAM_TOSERVER, wr.wr_len, wr.data.len() as u32);
+                state.set_skip(STREAM_TOSERVER, nbss_remaining);
                 return;
             }
 
@@ -320,10 +320,10 @@ pub fn smb2_write_request_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n
                         }
                         if max_queue_size != 0 && tdf.file_tracker.get_inflight_size() + wr.wr_len as u64 > max_queue_size.into() {
                             state.set_event(SMBEvent::WriteQueueSizeExceeded);
-                            state.set_skip(STREAM_TOSERVER, wr.wr_len, wr.data.len() as u32);
+                            state.set_skip(STREAM_TOSERVER, nbss_remaining);
                         } else if max_queue_cnt != 0 && tdf.file_tracker.get_inflight_cnt() >= max_queue_cnt as usize {
                             state.set_event(SMBEvent::WriteQueueCntExceeded);
-                            state.set_skip(STREAM_TOSERVER, wr.wr_len, wr.data.len() as u32);
+                            state.set_skip(STREAM_TOSERVER, nbss_remaining);
                         } else {
                             filetracker_newchunk(&mut tdf.file_tracker, files, flags,
                                     &file_name, wr.data, wr.wr_offset,
@@ -376,7 +376,7 @@ pub fn smb2_write_request_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n
                     smb_write_dcerpc_record(state, vercmd, hdr, wr.data);
                 } else if is_pipe {
                     SCLogDebug!("non-DCERPC pipe: skip rest of the record");
-                    state.set_skip(STREAM_TOSERVER, wr.wr_len, wr.data.len() as u32);
+                    state.set_skip(STREAM_TOSERVER, nbss_remaining);
                 } else {
                     let (tx, files, flags) = state.new_file_tx(&file_guid, &file_name, STREAM_TOSERVER);
                     tx.vercmd.set_smb2_cmd(SMB2_COMMAND_WRITE);
@@ -390,10 +390,10 @@ pub fn smb2_write_request_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n
 
                         if max_queue_size != 0 && tdf.file_tracker.get_inflight_size() + wr.wr_len as u64 > max_queue_size.into() {
                             state.set_event(SMBEvent::WriteQueueSizeExceeded);
-                            state.set_skip(STREAM_TOSERVER, wr.wr_len, wr.data.len() as u32);
+                            state.set_skip(STREAM_TOSERVER, nbss_remaining);
                         } else if max_queue_cnt != 0 && tdf.file_tracker.get_inflight_cnt() >= max_queue_cnt as usize {
                             state.set_event(SMBEvent::WriteQueueCntExceeded);
-                            state.set_skip(STREAM_TOSERVER, wr.wr_len, wr.data.len() as u32);
+                            state.set_skip(STREAM_TOSERVER, nbss_remaining);
                         } else {
                             filetracker_newchunk(&mut tdf.file_tracker, files, flags,
                                     &file_name, wr.data, wr.wr_offset,