From: Victor Julien <vjulien@oisf.net>
Date: Tue, 30 Jul 2024 12:01:14 +0000 (+0200)
Subject: smb: update to GAP handling
X-Git-Tag: suricata-8.0.0-beta1~724
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c2124f0b9e65c26ef1ef935dec92117824e7e025;p=thirdparty%2Fsuricata.git

smb: update to GAP handling

Don't tag the session as gap'd when the GAP is in a precise location:

1. in "skip" data, where the GAP just fits the skip data

2. in file data, where we pass the GAP on to the file

This reduces load of GAP post-processing that is unnecessary in these
case.
---

diff --git a/rust/src/smb/smb.rs b/rust/src/smb/smb.rs
index 7dab8debbc..f80191b03b 100644
--- a/rust/src/smb/smb.rs
+++ b/rust/src/smb/smb.rs
@@ -28,7 +28,6 @@
 use std;
 use std::str;
 use std::ffi::{self, CString};
-
 use std::collections::HashMap;
 use std::collections::VecDeque;
  
@@ -1860,19 +1859,30 @@ impl SMBState {
     /// handle a gap in the TOSERVER direction
     /// returns: 0 ok, 1 unrecoverable error
     pub fn parse_tcp_data_ts_gap(&mut self, gap_size: u32) -> AppLayerResult {
+        SCLogDebug!("GAP of size {} in toserver direction", gap_size);
         let consumed = self.handle_skip(Direction::ToServer, gap_size);
+        if consumed == gap_size {
+            /* no need to tag ssn as gap'd as we got it in our skip logic. */
+            return AppLayerResult::ok();
+        }
+
         if consumed < gap_size {
             let new_gap_size = gap_size - consumed;
             let gap = vec![0; new_gap_size as usize];
 
             let consumed2 = self.filetracker_update(Direction::ToServer, &gap, new_gap_size);
+            if consumed2 == new_gap_size {
+                /* no need to tag ssn as gap'd as we got it in our file logic. */
+                return AppLayerResult::ok();
+            }
+
             if consumed2 > new_gap_size {
                 SCLogDebug!("consumed more than GAP size: {} > {}", consumed2, new_gap_size);
                 self.set_event(SMBEvent::InternalError);
                 return AppLayerResult::err();
             }
         }
-        SCLogDebug!("GAP of size {} in toserver direction", gap_size);
+
         self.ts_ssn_gap = true;
         self.ts_gap = true;
         return AppLayerResult::ok();
@@ -1881,19 +1891,30 @@ impl SMBState {
     /// handle a gap in the TOCLIENT direction
     /// returns: 0 ok, 1 unrecoverable error
     pub fn parse_tcp_data_tc_gap(&mut self, gap_size: u32) -> AppLayerResult {
+        SCLogDebug!("GAP of size {} in toclient direction", gap_size);
         let consumed = self.handle_skip(Direction::ToClient, gap_size);
+        if consumed == gap_size {
+            /* no need to tag ssn as gap'd as we got it in our skip logic. */
+            return AppLayerResult::ok();
+        }
+
         if consumed < gap_size {
             let new_gap_size = gap_size - consumed;
             let gap = vec![0; new_gap_size as usize];
 
             let consumed2 = self.filetracker_update(Direction::ToClient, &gap, new_gap_size);
+            if consumed2 == new_gap_size {
+                /* no need to tag ssn as gap'd as we got it in our file logic. */
+                return AppLayerResult::ok();
+            }
+
             if consumed2 > new_gap_size {
                 SCLogDebug!("consumed more than GAP size: {} > {}", consumed2, new_gap_size);
                 self.set_event(SMBEvent::InternalError);
                 return AppLayerResult::err();
             }
         }
-        SCLogDebug!("GAP of size {} in toclient direction", gap_size);
+
         self.tc_ssn_gap = true;
         self.tc_gap = true;
         return AppLayerResult::ok();