From 97c67cd5ce6d4016f72b3d1f19cadfb410c28b1d Mon Sep 17 00:00:00 2001
From: Shivani Bhardwaj <shivanib134@gmail.com>
Date: Wed, 23 Sep 2020 10:20:56 +0530
Subject: [PATCH] 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.
---
 rust/src/dcerpc/dcerpc.rs | 56 +++++++++++++++++----------------------
 1 file changed, 25 insertions(+), 31 deletions(-)

diff --git a/rust/src/dcerpc/dcerpc.rs b/rust/src/dcerpc/dcerpc.rs
index a08dab6b39..e4fa10e5e7 100644
--- 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);
+                },
             }
         }
 
-- 
2.47.2