diff --git a/rules/smb-events.rules b/rules/smb-events.rules index 248d75e4b5..745c2ea2d2 100644 --- a/rules/smb-events.rules +++ b/rules/smb-events.rules @@ -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 diff --git a/rust/src/smb/smb.rs b/rust/src/smb/smb.rs index b36b465db5..aeb7102a6f 100644 --- a/rust/src/smb/smb.rs +++ b/rust/src/smb/smb.rs @@ -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; } diff --git a/rust/src/smb/smb1.rs b/rust/src/smb/smb1.rs index c664461557..bbe18921ae 100644 --- a/rust/src/smb/smb1.rs +++ b/rust/src/smb/smb1.rs @@ -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 = 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 = 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), diff --git a/rust/src/smb/smb2.rs b/rust/src/smb/smb2.rs index 685aae487b..36c31c577b 100644 --- a/rust/src/smb/smb2.rs +++ b/rust/src/smb/smb2.rs @@ -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 {