Skip to content

Commit

Permalink
dcerpc: tidy up code
Browse files Browse the repository at this point in the history
- remove unneeded variables
- remove unnecessary tracking of bytes in state
- modify calculations as indicated by failing tests
  • Loading branch information
inashivb committed Oct 1, 2024
1 parent d69ff3a commit 85378c0
Showing 1 changed file with 6 additions and 59 deletions.
65 changes: 6 additions & 59 deletions rust/src/dcerpc/dcerpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,11 +312,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,
Expand All @@ -338,8 +334,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()
}
}
Expand Down Expand Up @@ -442,26 +436,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.
Expand Down Expand Up @@ -897,9 +871,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) {
Expand Down Expand Up @@ -932,40 +903,26 @@ impl DCERPCState {
}
}

// Overwrite the dcerpc_state data in case of multiple complete queries in the
// same direction
if self.prev_dir == direction {
self.data_needed_for_dir = direction;
}

if self.data_needed_for_dir != direction && !cur_i.is_empty() {
return AppLayerResult::err();
}

// Set data_needed_for_dir in the same direction in case there is an issue with upcoming parsing
self.data_needed_for_dir = direction;

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() && input_len > 0 {
if self.header.is_none() && !cur_i.is_empty() {
parsed = self.process_header(cur_i);
if parsed == -1 {
return AppLayerResult::incomplete(0, DCERPC_HDR_LEN as u32);
}
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);
Expand Down Expand Up @@ -1029,23 +986,15 @@ impl DCERPCState {
}
_ => {
SCLogDebug!("Unrecognized packet type: {:?}", x);
self.clean_buffer(direction);
return AppLayerResult::err();
}
},
None => {
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();
}
Expand Down Expand Up @@ -1888,7 +1837,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)
Expand Down Expand Up @@ -2176,7 +2124,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);
}
Expand Down

0 comments on commit 85378c0

Please sign in to comment.