smb: checks against nbss records length

When Suricata handles files over SMB, it does not wait for the
NBSS record to be complete, and can stream the payload to the
file... But it did not check the consistency of the SMB record
length being read or written against the NBSS record length.

This could lead to an evasion where an attacker crafts a SMB
write with a too big Length field, and then sends its evil
payload, even if the server returned an error for the write request.

Ticket: #5770
pull/8536/head
Philippe Antoine 2 years ago committed by Victor Julien
parent 8e5b1fe8e6
commit c1b7befb18

@ -26,9 +26,9 @@ alert smb any any -> any any (msg:"SURICATA SMB max response READ size exceeded"
# checks negotiated max-write-size and 'app-layer.protocols.smb.max-write-size`
alert smb any any -> any any (msg:"SURICATA SMB max WRITE size exceeded"; flow:to_server; app-layer-event:smb.write_request_too_large; classtype:protocol-command-decode; sid:2225011; rev:1;)
# checks 'app-layer.protocols.smb.max-read-size` against NEGOTIATE PROTOCOL response
# checks 'app-layer.protocols.smb.max-read-size`, NEGOTIATE PROTOCOL response, and NBSS record length against SMB read data length
alert smb any any -> any any (msg:"SURICATA SMB supported READ size exceeded"; flow:to_client; app-layer-event:smb.negotiate_max_read_size_too_large; classtype:protocol-command-decode; sid:2225012; rev:1;)
# checks 'app-layer.protocols.smb.max-write-size` against NEGOTIATE PROTOCOL response
# checks 'app-layer.protocols.smb.max-write-size`, NEGOTIATE PROTOCOL response, NBSS record length against SMB write data length
alert smb any any -> any any (msg:"SURICATA SMB supported WRITE size exceeded"; flow:to_server; app-layer-event:smb.negotiate_max_write_size_too_large; classtype:protocol-command-decode; sid:2225013; rev:1;)
# checks 'app-layer.protocols.smb.max-write-queue-size` against out of order chunks

@ -1306,8 +1306,12 @@ impl SMBState {
if is_pipe {
return 0;
}
smb1_write_request_record(self, r, SMB1_HEADER_SIZE, SMB1_COMMAND_WRITE_ANDX);
// how many more bytes are expected within this NBSS record
// So that we can check that further parsed offsets and lengths
// stay within the NBSS record.
let nbss_remaining = nbss_part_hdr.length - nbss_part_hdr.data.len() as u32;
smb1_write_request_record(self, r, SMB1_HEADER_SIZE, SMB1_COMMAND_WRITE_ANDX, nbss_remaining);
self.add_nbss_ts_frames(flow, stream_slice, input, nbss_part_hdr.length as i64);
self.add_smb1_ts_pdu_frame(flow, stream_slice, nbss_part_hdr.data, nbss_part_hdr.length as i64);
self.add_smb1_ts_hdr_data_frames(flow, stream_slice, nbss_part_hdr.data, nbss_part_hdr.length as i64);
@ -1322,8 +1326,12 @@ impl SMBState {
SCLogDebug!("SMB2: partial record {}",
&smb2_command_string(smb_record.command));
if smb_record.command == SMB2_COMMAND_WRITE {
smb2_write_request_record(self, smb_record);
// how many more bytes are expected within this NBSS record
// So that we can check that further parsed offsets and lengths
// stay within the NBSS record.
let nbss_remaining = nbss_part_hdr.length - nbss_part_hdr.data.len() as u32;
smb2_write_request_record(self, smb_record, nbss_remaining);
self.add_nbss_ts_frames(flow, stream_slice, input, nbss_part_hdr.length as i64);
self.add_smb2_ts_pdu_frame(flow, stream_slice, nbss_part_hdr.data, nbss_part_hdr.length as i64);
self.add_smb2_ts_hdr_data_frames(flow, stream_slice, nbss_part_hdr.data, nbss_part_hdr.length as i64, smb_record.header_len as i64);
@ -1641,7 +1649,11 @@ impl SMBState {
self.add_smb1_tc_pdu_frame(flow, stream_slice, nbss_part_hdr.data, nbss_part_hdr.length as i64);
self.add_smb1_tc_hdr_data_frames(flow, stream_slice, nbss_part_hdr.data, nbss_part_hdr.length as i64);
smb1_read_response_record(self, r, SMB1_HEADER_SIZE);
// how many more bytes are expected within this NBSS record
// So that we can check that further parsed offsets and lengths
// stay within the NBSS record.
let nbss_remaining = nbss_part_hdr.length - nbss_part_hdr.data.len() as u32;
smb1_read_response_record(self, r, SMB1_HEADER_SIZE, nbss_remaining);
let consumed = input.len() - output.len();
return consumed;
}
@ -1658,7 +1670,11 @@ impl SMBState {
self.add_smb2_tc_pdu_frame(flow, stream_slice, nbss_part_hdr.data, nbss_part_hdr.length as i64);
self.add_smb2_tc_hdr_data_frames(flow, stream_slice, nbss_part_hdr.data, nbss_part_hdr.length as i64, smb_record.header_len as i64);
smb2_read_response_record(self, smb_record);
// how many more bytes are expected within this NBSS record
// So that we can check that further parsed offsets and lengths
// stay within the NBSS record.
let nbss_remaining = nbss_part_hdr.length - nbss_part_hdr.data.len() as u32;
smb2_read_response_record(self, smb_record, nbss_remaining);
let consumed = input.len() - output.len();
return consumed;
}

@ -417,7 +417,7 @@ fn smb1_request_record_one(state: &mut SMBState, r: &SmbRecord, command: u8, and
SMB1_COMMAND_WRITE_ANDX |
SMB1_COMMAND_WRITE |
SMB1_COMMAND_WRITE_AND_CLOSE => {
smb1_write_request_record(state, r, *andx_offset, command);
smb1_write_request_record(state, r, *andx_offset, command, 0);
true // tx handling in func
},
SMB1_COMMAND_TRANS => {
@ -617,7 +617,7 @@ fn smb1_response_record_one(state: &mut SMBState, r: &SmbRecord, command: u8, an
let have_tx = match command {
SMB1_COMMAND_READ_ANDX => {
smb1_read_response_record(state, r, *andx_offset);
smb1_read_response_record(state, r, *andx_offset, 0);
true // tx handling in func
},
SMB1_COMMAND_NEGOTIATE_PROTOCOL => {
@ -932,7 +932,7 @@ pub fn smb1_trans_response_record(state: &mut SMBState, r: &SmbRecord)
}
/// Handle WRITE, WRITE_ANDX, WRITE_AND_CLOSE request records
pub fn smb1_write_request_record(state: &mut SMBState, r: &SmbRecord, andx_offset: usize, command: u8)
pub fn smb1_write_request_record(state: &mut SMBState, r: &SmbRecord, andx_offset: usize, command: u8, nbss_remaining: u32)
{
let mut events : Vec<SMBEvent> = Vec::new();
@ -946,7 +946,13 @@ pub fn smb1_write_request_record(state: &mut SMBState, r: &SmbRecord, andx_offse
match result {
Ok((_, rd)) => {
SCLogDebug!("SMBv1: write andx => {:?}", rd);
if rd.len > rd.data.len() as u32 + nbss_remaining {
// Record claims more bytes than are in NBSS record...
state.set_event(SMBEvent::WriteRequestTooLarge);
// Skip the remaining bytes of the record.
state.set_skip(Direction::ToServer, nbss_remaining, 0);
return;
}
let mut file_fid = rd.fid.to_vec();
file_fid.extend_from_slice(&u32_as_bytes(r.ssn_id));
SCLogDebug!("SMBv1 WRITE: FID {:?} offset {}",
@ -1019,7 +1025,7 @@ pub fn smb1_write_request_record(state: &mut SMBState, r: &SmbRecord, andx_offse
smb1_request_record_generic(state, r, events);
}
pub fn smb1_read_response_record(state: &mut SMBState, r: &SmbRecord, andx_offset: usize)
pub fn smb1_read_response_record(state: &mut SMBState, r: &SmbRecord, andx_offset: usize, nbss_remaining: u32)
{
let mut events : Vec<SMBEvent> = Vec::new();
@ -1027,7 +1033,13 @@ pub fn smb1_read_response_record(state: &mut SMBState, r: &SmbRecord, andx_offse
match parse_smb_read_andx_response_record(&r.data[andx_offset-SMB1_HEADER_SIZE..]) {
Ok((_, rd)) => {
SCLogDebug!("SMBv1: read response => {:?}", rd);
if rd.len > nbss_remaining + rd.data.len() as u32 {
// Record claims more bytes than are in NBSS record...
state.set_event(SMBEvent::ReadResponseTooLarge);
// Skip the remaining bytes of the record.
state.set_skip(Direction::ToClient, nbss_remaining, 0);
return;
}
let fid_key = SMBCommonHdr::from1(r, SMBHDR_TYPE_OFFSET);
let (offset, file_fid) = match state.ssn2vecoffset_map.remove(&fid_key) {
Some(o) => (o.offset, o.guid),

@ -113,7 +113,7 @@ fn smb2_read_response_record_generic(state: &mut SMBState, r: &Smb2Record)
}
}
pub fn smb2_read_response_record(state: &mut SMBState, r: &Smb2Record)
pub fn smb2_read_response_record(state: &mut SMBState, r: &Smb2Record, nbss_remaining: u32)
{
let max_queue_size = unsafe { SMB_CFG_MAX_READ_QUEUE_SIZE };
let max_queue_cnt = unsafe { SMB_CFG_MAX_READ_QUEUE_CNT };
@ -122,6 +122,13 @@ pub fn smb2_read_response_record(state: &mut SMBState, r: &Smb2Record)
match parse_smb2_response_read(r.data) {
Ok((_, rd)) => {
if rd.len - rd.data.len() as u32 > nbss_remaining {
// Record claims more bytes than are in NBSS record...
state.set_event(SMBEvent::ReadResponseTooLarge);
// Skip the remaining bytes of the record.
state.set_skip(Direction::ToClient, nbss_remaining, 0);
return;
}
if r.nt_status == SMB_NTSTATUS_BUFFER_OVERFLOW {
SCLogDebug!("SMBv2/READ: incomplete record, expecting a follow up");
// fall through
@ -264,7 +271,7 @@ pub fn smb2_read_response_record(state: &mut SMBState, r: &Smb2Record)
}
}
pub fn smb2_write_request_record(state: &mut SMBState, r: &Smb2Record)
pub fn smb2_write_request_record(state: &mut SMBState, r: &Smb2Record, nbss_remaining: u32)
{
let max_queue_size = unsafe { SMB_CFG_MAX_WRITE_QUEUE_SIZE };
let max_queue_cnt = unsafe { SMB_CFG_MAX_WRITE_QUEUE_CNT };
@ -277,6 +284,13 @@ pub fn smb2_write_request_record(state: &mut SMBState, r: &Smb2Record)
}
match parse_smb2_request_write(r.data) {
Ok((_, wr)) => {
if wr.wr_len - wr.data.len() as u32 > nbss_remaining {
// Record claims more bytes than are in NBSS record...
state.set_event(SMBEvent::WriteRequestTooLarge);
// Skip the remaining bytes of the record.
state.set_skip(Direction::ToServer, nbss_remaining, 0);
return;
}
if (state.max_write_size != 0 && wr.wr_len > state.max_write_size) ||
(unsafe { SMB_CFG_MAX_WRITE_SIZE != 0 && SMB_CFG_MAX_WRITE_SIZE < wr.wr_len }) {
state.set_event(SMBEvent::WriteRequestTooLarge);
@ -580,7 +594,7 @@ pub fn smb2_request_record(state: &mut SMBState, r: &Smb2Record)
}
},
SMB2_COMMAND_WRITE => {
smb2_write_request_record(state, r);
smb2_write_request_record(state, r, 0);
true // write handling creates both file tx and generic tx
},
SMB2_COMMAND_CLOSE => {
@ -684,7 +698,7 @@ pub fn smb2_response_record(state: &mut SMBState, r: &Smb2Record)
SMB2_COMMAND_READ => {
if r.nt_status == SMB_NTSTATUS_SUCCESS ||
r.nt_status == SMB_NTSTATUS_BUFFER_OVERFLOW {
smb2_read_response_record(state, r);
smb2_read_response_record(state, r, 0);
false
} else if r.nt_status == SMB_NTSTATUS_END_OF_FILE {

Loading…
Cancel
Save