dcerpc: fix gap handling

This patch addresses issues discovered by redmine ticket 3896. With the
approach of finding latest record, there was a chance that no record was
found at all and consumed + needed became input length.

e.g.
input_len = 1000
input = 01 05 00 02 00 03 a5 56 00 00 .....

There exists no |05 00| identifier in the rest of the record. After
having parsed |05 00|, there was a search for another record with the
leftover data. Current data length at this point would be 997. Since the
identifier was not found in the data, we calculate the consumed bytes at
this point i.e. consumed = current_data.len() - 1 which would be 996.
Needed bytes still stay at a constant of 2. So, consumed + needed = 996
+ 2 = 998 which is lesser than initial input length of 1000 and hence
the assertion fails.

There could be two fixes to this problem.
1. Finding the latest record but making use of the last found record in
   case no new record was found.
2. Always use the earliest record.

This patch takes the approach (2). It also makes sure that the gap and
current direction are the same.

diff --git a/rust/src/dcerpc/dcerpc.rs b/rust/src/dcerpc/dcerpc.rs
index a08dab6b39446166c6f3612ede795f3a02b8e1ab..e4fa10e5e7967fcfae850f69f79a957614163da9 100644 (file)
--- a/rust/src/dcerpc/dcerpc.rs
+++ b/rust/src/dcerpc/dcerpc.rs
@@ -913,39 +913,33 @@ impl DCERPCState {
         self.query_completed = false;
 
         // Skip the record since this means that its in the middle of a known length record
-        if self.ts_gap || self.tc_gap {
+        if (self.ts_gap && direction == core::STREAM_TOSERVER) || (self.tc_gap && direction == core::STREAM_TOCLIENT) {
             SCLogDebug!("Trying to catch up after GAP (input {})", cur_i.len());
-            while cur_i.len() > 0 { // min record size
-                match self.search_dcerpc_record(cur_i) {
-                    Ok((_, pg)) => {
-                        SCLogDebug!("DCERPC record found");
-                        let offset = cur_i.len() - pg.len();
-                        if offset == 1 {
-                            cur_i = &cur_i[offset + 2..];
-                            continue; // see if we have another record in our data
+            match self.search_dcerpc_record(cur_i) {
+                Ok((_, pg)) => {
+                    SCLogDebug!("DCERPC record found");
+                    let offset = cur_i.len() - pg.len();
+                    cur_i = &cur_i[offset..];
+                    match direction {
+                        core::STREAM_TOSERVER => {
+                            self.ts_gap = false;
+                        },
+                        _ => {
+                            self.tc_gap = false;
                         }
-                        match direction {
-                            core::STREAM_TOSERVER => {
-                                self.ts_gap = false;
-                                break;
-                            },
-                            _ => {
-                                self.tc_gap = false;
-                                break;
-                            }
-                        }
-                    },
-                    _ => {
-                        let mut consumed = cur_i.len();
-                        if consumed < 2 {
-                            consumed = 0;
-                        } else {
-                            consumed = consumed - 1;
-                        }
-                        SCLogDebug!("DCERPC record NOT found");
-                        return AppLayerResult::incomplete(consumed as u32, 2);
-                    },
-                }
+                    }
+                },
+                _ => {
+                    let mut consumed = cur_i.len();
+                    // At least 2 bytes are required to know if a new record is beginning
+                    if consumed < 2 {
+                        consumed = 0;
+                    } else {
+                        consumed = consumed - 1;
+                    }
+                    SCLogDebug!("DCERPC record NOT found");
+                    return AppLayerResult::incomplete(consumed as u32, 2);
+                },
             }
         }