smb: fix file reopening issue

Fuzzing highlighted an issue where a command sequence on the same file
id triggered a logging issue:

file data for id N
close id N
file data for id N

If this happened in a single blob of data passed to the parser, the
existing file tx would be reused, the file "reopened", confusing the
file logging logic. This would trigger a debug assert.

This patch makes sure a new file tx is created for the file data
coming in after the first file tx is closed.

Bug: #5567.
pull/8223/head
Victor Julien 3 years ago
parent 9ed65907a7
commit 45eb038e63

@ -89,6 +89,34 @@ impl SMBState {
return tx_ref.unwrap();
}
/// get file tx for a open file. Returns None if a file for the fuid exists,
/// but has already been closed.
pub fn get_file_tx_by_fuid_with_open_file(&mut self, fuid: &[u8], direction: Direction)
-> Option<&mut SMBTransaction>
{
let f = fuid.to_vec();
for tx in &mut self.transactions {
let found = match tx.type_data {
Some(SMBTransactionTypeData::FILE(ref mut d)) => {
direction == d.direction && f == d.fuid && !d.file_tracker.is_done()
},
_ => { false },
};
if found {
SCLogDebug!("SMB: Found SMB file TX with ID {}", tx.id);
if let Some(SMBTransactionTypeData::FILE(ref mut d)) = tx.type_data {
tx.tx_data.update_file_flags(self.state_data.file_flags);
d.update_file_flags(tx.tx_data.file_flags);
}
return Some(tx);
}
}
SCLogDebug!("SMB: Failed to find SMB TX with FUID {:?}", fuid);
return None;
}
/// get file tx for a fuid. File may already have been closed.
pub fn get_file_tx_by_fuid(&mut self, fuid: &[u8], direction: Direction)
-> Option<&mut SMBTransaction>
{

@ -964,7 +964,7 @@ pub fn smb1_write_request_record<'b>(state: &mut SMBState, r: &SmbRecord<'b>, an
None => b"<unknown>".to_vec(),
};
let mut set_event_fileoverlap = false;
let found = match state.get_file_tx_by_fuid(&file_fid, Direction::ToServer) {
let found = match state.get_file_tx_by_fuid_with_open_file(&file_fid, Direction::ToServer) {
Some(tx) => {
let file_id : u32 = tx.id as u32;
if let Some(SMBTransactionTypeData::FILE(ref mut tdf)) = tx.type_data {
@ -1061,7 +1061,7 @@ pub fn smb1_read_response_record<'b>(state: &mut SMBState, r: &SmbRecord<'b>, an
None => Vec::new(),
};
let mut set_event_fileoverlap = false;
let found = match state.get_file_tx_by_fuid(&file_fid, Direction::ToClient) {
let found = match state.get_file_tx_by_fuid_with_open_file(&file_fid, Direction::ToClient) {
Some(tx) => {
if let Some(SMBTransactionTypeData::FILE(ref mut tdf)) = tx.type_data {
let file_id : u32 = tx.id as u32;

@ -157,7 +157,7 @@ pub fn smb2_read_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>)
let mut set_event_fileoverlap = false;
// look up existing tracker and if we have it update it
let found = match state.get_file_tx_by_fuid(&file_guid, Direction::ToClient) {
let found = match state.get_file_tx_by_fuid_with_open_file(&file_guid, Direction::ToClient) {
Some(tx) => {
if let Some(SMBTransactionTypeData::FILE(ref mut tdf)) = tx.type_data {
let file_id : u32 = tx.id as u32;
@ -297,7 +297,7 @@ pub fn smb2_write_request_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>)
};
let mut set_event_fileoverlap = false;
let found = match state.get_file_tx_by_fuid(&file_guid, Direction::ToServer) {
let found = match state.get_file_tx_by_fuid_with_open_file(&file_guid, Direction::ToServer) {
Some(tx) => {
if let Some(SMBTransactionTypeData::FILE(ref mut tdf)) = tx.type_data {
let file_id : u32 = tx.id as u32;

Loading…
Cancel
Save