From: Shivani Bhardwaj <shivani@oisf.net>
Date: Wed, 18 Sep 2024 09:01:28 +0000 (+0530)
Subject: dcerpc: tidy up code
X-Git-Tag: suricata-8.0.0-beta1~492
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9daf8528b72c0ba84b863a7ef30a867db8f49aa7;p=thirdparty%2Fsuricata.git

dcerpc: tidy up code

- remove unneeded variables
- remove unnecessary tracking of bytes in state
- modify calculations as indicated by failing tests
---

diff --git a/rust/src/dcerpc/dcerpc.rs b/rust/src/dcerpc/dcerpc.rs
index 6270386242..e247ab9eb9 100644
--- a/rust/src/dcerpc/dcerpc.rs
+++ b/rust/src/dcerpc/dcerpc.rs
@@ -314,11 +314,7 @@ pub struct DCERPCState {
     tx_index_completed: usize,
     pub pad: u8,
     pub padleft: u16,
-    pub bytes_consumed: i32,
     pub tx_id: u64,
-    pub query_completed: bool,
-    pub data_needed_for_dir: Direction,
-    pub prev_dir: Direction,
     pub ts_gap: bool,
     pub tc_gap: bool,
     pub ts_ssn_gap: bool,
@@ -340,8 +336,6 @@ impl State<DCERPCTransaction> for DCERPCState {
 impl DCERPCState {
     pub fn new() -> Self {
         return Self {
-            data_needed_for_dir: Direction::ToServer,
-            prev_dir: Direction::ToServer,
             ..Default::default()
         }
     }
@@ -446,26 +440,6 @@ impl DCERPCState {
         None
     }
 
-    pub fn clean_buffer(&mut self, direction: Direction) {
-        match direction {
-            Direction::ToServer => {
-                self.ts_gap = false;
-            }
-            Direction::ToClient => {
-                self.tc_gap = false;
-            }
-        }
-        self.bytes_consumed = 0;
-    }
-
-    pub fn reset_direction(&mut self, direction: Direction) {
-        if direction == Direction::ToServer {
-            self.data_needed_for_dir = Direction::ToClient;
-        } else {
-            self.data_needed_for_dir = Direction::ToServer;
-        }
-    }
-
     /// Get transaction as per the given transaction ID. Transaction ID with
     /// which the lookup is supposed to be done as per the calls from AppLayer
     /// parser in C. This requires an internal transaction ID to be maintained.
@@ -903,9 +877,6 @@ impl DCERPCState {
         let mut parsed = 0;
         let retval;
         let mut cur_i = input;
-        let mut input_len = cur_i.len();
-        // Set any query's completion status to false in the beginning
-        self.query_completed = false;
 
         // Skip the record since this means that its in the middle of a known length record
         if (self.ts_gap && direction == Direction::ToServer) || (self.tc_gap && direction == Direction::ToClient) {
@@ -938,6 +909,7 @@ impl DCERPCState {
             }
         }
 
+        let mut frag_bytes_consumed: u16 = 0;
         // Check if header data was complete. In case of EoF or incomplete data, wait for more
         // data else return error
         if self.header.is_none() && !cur_i.is_empty() {
@@ -948,17 +920,15 @@ impl DCERPCState {
             if parsed < 0 {
                 return AppLayerResult::err();
             }
-            self.bytes_consumed += parsed;
-            input_len -= parsed as usize;
+        } else {
+            frag_bytes_consumed = DCERPC_HDR_LEN;
         }
 
         let fraglen = self.get_hdr_fraglen().unwrap_or(0);
 
-        if (input_len + self.bytes_consumed as usize) < fraglen as usize {
+        if cur_i.len() < (fraglen - frag_bytes_consumed) as usize {
             SCLogDebug!("Possibly fragmented data, waiting for more..");
-            return AppLayerResult::incomplete(self.bytes_consumed as u32, (fraglen - self.bytes_consumed as u16) as u32);
-        } else {
-            self.query_completed = true;
+            return AppLayerResult::incomplete(parsed as u32, fraglen as u32 - parsed as u32);
         }
 
         let current_call_id = self.get_hdr_call_id().unwrap_or(0);
@@ -1022,7 +992,6 @@ impl DCERPCState {
                 }
                 _ => {
                     SCLogDebug!("Unrecognized packet type: {:?}", x);
-                    self.clean_buffer(direction);
                     return AppLayerResult::err();
                 }
             },
@@ -1030,15 +999,8 @@ impl DCERPCState {
                 return AppLayerResult::err();
             }
         }
-        self.bytes_consumed += retval;
 
-        // If the query has been completed, clean the buffer and reset the direction
-        if self.query_completed {
-            self.clean_buffer(direction);
-            self.reset_direction(direction);
-        }
         self.post_gap_housekeeping(direction);
-        self.prev_dir = direction;
         self.header = None;
         return AppLayerResult::ok();
     }
@@ -1881,7 +1843,6 @@ mod tests {
             0xFF,
         ];
         let mut dcerpc_state = DCERPCState::new();
-        dcerpc_state.data_needed_for_dir = Direction::ToClient;
         assert_eq!(
             AppLayerResult::ok(),
             dcerpc_state.handle_input_data(bind_ack, Direction::ToClient)
@@ -2169,7 +2130,6 @@ mod tests {
         );
         if let Some(ref back) = dcerpc_state.bindack {
             assert_eq!(1, back.accepted_uuid_list.len());
-            dcerpc_state.data_needed_for_dir = Direction::ToServer;
             assert_eq!(11, back.accepted_uuid_list[0].ctxid);
             assert_eq!(expected_uuid3, back.accepted_uuid_list[0].uuid);
         }