smb: fix file reopening issue

Fuzzing highlighted an issue where a command sequence on the same file
id triggered a logging issue:

file data for id N
close id N
file data for id N

If this happened in a single blob of data passed to the parser, the
existing file tx would be reused, the file "reopened", confusing the
file logging logic. This would trigger a debug assert.

This patch makes sure a new file tx is created for the file data
coming in after the first file tx is closed.

Bug: #5567.

diff --git a/rust/src/smb/files.rs b/rust/src/smb/files.rs
index d0762e4c7b3e21e328dde28ca015fff449a65392..d99a988c8df7e7f3ffe8dbc038b4a899ff0838c0 100644 (file)
--- a/rust/src/smb/files.rs
+++ b/rust/src/smb/files.rs
@@ -89,6 +89,34 @@ impl SMBState {
         return tx_ref.unwrap();
     }
 
+    /// get file tx for a open file. Returns None if a file for the fuid exists,
+    /// but has already been closed.
+    pub fn get_file_tx_by_fuid_with_open_file(&mut self, fuid: &[u8], direction: Direction)
+        -> Option<&mut SMBTransaction>
+    {
+        let f = fuid.to_vec();
+        for tx in &mut self.transactions {
+            let found = match tx.type_data {
+                Some(SMBTransactionTypeData::FILE(ref mut d)) => {
+                    direction == d.direction && f == d.fuid && !d.file_tracker.is_done()
+                },
+                _ => { false },
+            };
+
+            if found {
+                SCLogDebug!("SMB: Found SMB file TX with ID {}", tx.id);
+                if let Some(SMBTransactionTypeData::FILE(ref mut d)) = tx.type_data {
+                    tx.tx_data.update_file_flags(self.state_data.file_flags);
+                    d.update_file_flags(tx.tx_data.file_flags);
+                }
+                return Some(tx);
+            }
+        }
+        SCLogDebug!("SMB: Failed to find SMB TX with FUID {:?}", fuid);
+        return None;
+    }
+
+    /// get file tx for a fuid. File may already have been closed.
     pub fn get_file_tx_by_fuid(&mut self, fuid: &[u8], direction: Direction)
         -> Option<&mut SMBTransaction>
     {

diff --git a/rust/src/smb/smb1.rs b/rust/src/smb/smb1.rs
index 5b3bfc37df8b7af235f728602f84ffa008872665..e072a4185b6e0f17201fa73cf0b300ad7f28ca62 100644 (file)
--- a/rust/src/smb/smb1.rs
+++ b/rust/src/smb/smb1.rs
@@ -964,7 +964,7 @@ pub fn smb1_write_request_record<'b>(state: &mut SMBState, r: &SmbRecord<'b>, an
                 None => b"<unknown>".to_vec(),
             };
             let mut set_event_fileoverlap = false;
-            let found = match state.get_file_tx_by_fuid(&file_fid, Direction::ToServer) {
+            let found = match state.get_file_tx_by_fuid_with_open_file(&file_fid, Direction::ToServer) {
                 Some(tx) => {
                     let file_id : u32 = tx.id as u32;
                     if let Some(SMBTransactionTypeData::FILE(ref mut tdf)) = tx.type_data {
@@ -1061,7 +1061,7 @@ pub fn smb1_read_response_record<'b>(state: &mut SMBState, r: &SmbRecord<'b>, an
                         None => Vec::new(),
                     };
                     let mut set_event_fileoverlap = false;
-                    let found = match state.get_file_tx_by_fuid(&file_fid, Direction::ToClient) {
+                    let found = match state.get_file_tx_by_fuid_with_open_file(&file_fid, Direction::ToClient) {
                         Some(tx) => {
                             if let Some(SMBTransactionTypeData::FILE(ref mut tdf)) = tx.type_data {
                                 let file_id : u32 = tx.id as u32;

diff --git a/rust/src/smb/smb2.rs b/rust/src/smb/smb2.rs
index 059579f4b3b694ffc46498f0a709e6f5200ee1ba..ac02201cf6f5a3e02299fa9fbdfc247a16d75096 100644 (file)
--- a/rust/src/smb/smb2.rs
+++ b/rust/src/smb/smb2.rs
@@ -157,7 +157,7 @@ pub fn smb2_read_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>)
 
             let mut set_event_fileoverlap = false;
             // look up existing tracker and if we have it update it
-            let found = match state.get_file_tx_by_fuid(&file_guid, Direction::ToClient) {
+            let found = match state.get_file_tx_by_fuid_with_open_file(&file_guid, Direction::ToClient) {
                 Some(tx) => {
                     if let Some(SMBTransactionTypeData::FILE(ref mut tdf)) = tx.type_data {
                         let file_id : u32 = tx.id as u32;
@@ -297,7 +297,7 @@ pub fn smb2_write_request_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>)
             };
 
             let mut set_event_fileoverlap = false;
-            let found = match state.get_file_tx_by_fuid(&file_guid, Direction::ToServer) {
+            let found = match state.get_file_tx_by_fuid_with_open_file(&file_guid, Direction::ToServer) {
                 Some(tx) => {
                     if let Some(SMBTransactionTypeData::FILE(ref mut tdf)) = tx.type_data {
                         let file_id : u32 = tx.id as u32;