Skip to content

Commit 8f400c6

Browse files
committed
dcerpc: tidy up code
- remove unneeded variables - remove unnecessary tracking of bytes in state - modify calculations as indicated by failing tests
1 parent e2d59d5 commit 8f400c6

File tree

1 file changed

+6
-59
lines changed

1 file changed

+6
-59
lines changed

rust/src/dcerpc/dcerpc.rs

Lines changed: 6 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -312,11 +312,7 @@ pub struct DCERPCState {
312312
tx_index_completed: usize,
313313
pub pad: u8,
314314
pub padleft: u16,
315-
pub bytes_consumed: i32,
316315
pub tx_id: u64,
317-
pub query_completed: bool,
318-
pub data_needed_for_dir: Direction,
319-
pub prev_dir: Direction,
320316
pub ts_gap: bool,
321317
pub tc_gap: bool,
322318
pub ts_ssn_gap: bool,
@@ -338,8 +334,6 @@ impl State<DCERPCTransaction> for DCERPCState {
338334
impl DCERPCState {
339335
pub fn new() -> Self {
340336
return Self {
341-
data_needed_for_dir: Direction::ToServer,
342-
prev_dir: Direction::ToServer,
343337
..Default::default()
344338
}
345339
}
@@ -444,26 +438,6 @@ impl DCERPCState {
444438
None
445439
}
446440

447-
pub fn clean_buffer(&mut self, direction: Direction) {
448-
match direction {
449-
Direction::ToServer => {
450-
self.ts_gap = false;
451-
}
452-
Direction::ToClient => {
453-
self.tc_gap = false;
454-
}
455-
}
456-
self.bytes_consumed = 0;
457-
}
458-
459-
pub fn reset_direction(&mut self, direction: Direction) {
460-
if direction == Direction::ToServer {
461-
self.data_needed_for_dir = Direction::ToClient;
462-
} else {
463-
self.data_needed_for_dir = Direction::ToServer;
464-
}
465-
}
466-
467441
/// Get transaction as per the given transaction ID. Transaction ID with
468442
/// which the lookup is supposed to be done as per the calls from AppLayer
469443
/// parser in C. This requires an internal transaction ID to be maintained.
@@ -901,9 +875,6 @@ impl DCERPCState {
901875
let mut parsed = 0;
902876
let retval;
903877
let mut cur_i = input;
904-
let mut input_len = cur_i.len();
905-
// Set any query's completion status to false in the beginning
906-
self.query_completed = false;
907878

908879
// Skip the record since this means that its in the middle of a known length record
909880
if (self.ts_gap && direction == Direction::ToServer) || (self.tc_gap && direction == Direction::ToClient) {
@@ -936,40 +907,26 @@ impl DCERPCState {
936907
}
937908
}
938909

939-
// Overwrite the dcerpc_state data in case of multiple complete queries in the
940-
// same direction
941-
if self.prev_dir == direction {
942-
self.data_needed_for_dir = direction;
943-
}
944-
945-
if self.data_needed_for_dir != direction && !cur_i.is_empty() {
946-
return AppLayerResult::err();
947-
}
948-
949-
// Set data_needed_for_dir in the same direction in case there is an issue with upcoming parsing
950-
self.data_needed_for_dir = direction;
951-
910+
let mut frag_bytes_consumed: u16 = 0;
952911
// Check if header data was complete. In case of EoF or incomplete data, wait for more
953912
// data else return error
954-
if self.header.is_none() && input_len > 0 {
913+
if self.header.is_none() && !cur_i.is_empty() {
955914
parsed = self.process_header(cur_i);
956915
if parsed == -1 {
957916
return AppLayerResult::incomplete(0, DCERPC_HDR_LEN as u32);
958917
}
959918
if parsed < 0 {
960919
return AppLayerResult::err();
961920
}
962-
self.bytes_consumed += parsed;
963-
input_len -= parsed as usize;
921+
} else {
922+
frag_bytes_consumed = DCERPC_HDR_LEN;
964923
}
965924

966925
let fraglen = self.get_hdr_fraglen().unwrap_or(0);
967926

968-
if (input_len + self.bytes_consumed as usize) < fraglen as usize {
927+
if cur_i.len() < (fraglen - frag_bytes_consumed) as usize {
969928
SCLogDebug!("Possibly fragmented data, waiting for more..");
970-
return AppLayerResult::incomplete(self.bytes_consumed as u32, (fraglen - self.bytes_consumed as u16) as u32);
971-
} else {
972-
self.query_completed = true;
929+
return AppLayerResult::incomplete(parsed as u32, fraglen as u32 - parsed as u32);
973930
}
974931

975932
let current_call_id = self.get_hdr_call_id().unwrap_or(0);
@@ -1033,23 +990,15 @@ impl DCERPCState {
1033990
}
1034991
_ => {
1035992
SCLogDebug!("Unrecognized packet type: {:?}", x);
1036-
self.clean_buffer(direction);
1037993
return AppLayerResult::err();
1038994
}
1039995
},
1040996
None => {
1041997
return AppLayerResult::err();
1042998
}
1043999
}
1044-
self.bytes_consumed += retval;
10451000

1046-
// If the query has been completed, clean the buffer and reset the direction
1047-
if self.query_completed {
1048-
self.clean_buffer(direction);
1049-
self.reset_direction(direction);
1050-
}
10511001
self.post_gap_housekeeping(direction);
1052-
self.prev_dir = direction;
10531002
self.header = None;
10541003
return AppLayerResult::ok();
10551004
}
@@ -1892,7 +1841,6 @@ mod tests {
18921841
0xFF,
18931842
];
18941843
let mut dcerpc_state = DCERPCState::new();
1895-
dcerpc_state.data_needed_for_dir = Direction::ToClient;
18961844
assert_eq!(
18971845
AppLayerResult::ok(),
18981846
dcerpc_state.handle_input_data(bind_ack, Direction::ToClient)
@@ -2180,7 +2128,6 @@ mod tests {
21802128
);
21812129
if let Some(ref back) = dcerpc_state.bindack {
21822130
assert_eq!(1, back.accepted_uuid_list.len());
2183-
dcerpc_state.data_needed_for_dir = Direction::ToServer;
21842131
assert_eq!(11, back.accepted_uuid_list[0].ctxid);
21852132
assert_eq!(expected_uuid3, back.accepted_uuid_list[0].uuid);
21862133
}

0 commit comments

Comments
 (0)