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.
pull/5452/head
Shivani Bhardwaj 5 years ago committed by Victor Julien
parent 4f963717f8
commit 97c67cd5ce

@ -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);
},
}
}

Loading…
Cancel
Save