From: Philippe Antoine Date: Wed, 18 Jan 2023 15:47:56 +0000 (+0100) Subject: smb: handles records with trailing nbss data X-Git-Tag: suricata-6.0.11~45 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=95009e441195d2a5c879c965df2cdf7d55fb41e5;p=thirdparty%2Fsuricata.git smb: handles records with trailing nbss data If a file (read/write) SMB record has padding/trailing data after the buffer being read or written, and that Suricata falls in one case where it skips the data, it should skip until the very end of the NBSS record, meaning it should also skip the padding/trailing data. Otherwise, an attacker may smuggle some NBSS/SMB record in this trailing data, that will be interpreted by Suricata, but not by the SMB client/server, leading to evasions. Ticket: #5786 (cherry picked from commit 233ab1114873e632014b58d91c351505417fe8ed) --- diff --git a/rust/src/smb/smb.rs b/rust/src/smb/smb.rs index 2e194e7f33..66e12ba406 100644 --- a/rust/src/smb/smb.rs +++ b/rust/src/smb/smb.rs @@ -1264,13 +1264,12 @@ impl SMBState { } } - pub fn set_skip(&mut self, direction: u8, rec_size: u32, data_size: u32) + pub fn set_skip(&mut self, direction: u8, nbss_remaining: u32) { - let skip = rec_size.saturating_sub(data_size); if direction == STREAM_TOSERVER { - self.skip_ts = skip; + self.skip_ts = nbss_remaining; } else { - self.skip_tc = skip; + self.skip_tc = nbss_remaining; } } diff --git a/rust/src/smb/smb1.rs b/rust/src/smb/smb1.rs index 0de13a5052..85181f71fe 100644 --- a/rust/src/smb/smb1.rs +++ b/rust/src/smb/smb1.rs @@ -890,7 +890,7 @@ pub fn smb1_write_request_record<'b>(state: &mut SMBState, r: &SmbRecord<'b>, nb // Record claims more bytes than are in NBSS record... state.set_event(SMBEvent::WriteRequestTooLarge); // Skip the remaining bytes of the record. - state.set_skip(STREAM_TOSERVER, nbss_remaining, 0); + state.set_skip(STREAM_TOSERVER, nbss_remaining); return; } let mut file_fid = rd.fid.to_vec(); @@ -976,7 +976,7 @@ pub fn smb1_read_response_record<'b>(state: &mut SMBState, r: &SmbRecord<'b>, nb // Record claims more bytes than are in NBSS record... state.set_event(SMBEvent::ReadResponseTooLarge); // Skip the remaining bytes of the record. - state.set_skip(STREAM_TOCLIENT, nbss_remaining, 0); + state.set_skip(STREAM_TOCLIENT, nbss_remaining); return; } let fid_key = SMBCommonHdr::from1(r, SMBHDR_TYPE_OFFSET); @@ -985,7 +985,7 @@ pub fn smb1_read_response_record<'b>(state: &mut SMBState, r: &SmbRecord<'b>, nb None => { SCLogDebug!("SMBv1 READ response: reply to unknown request: left {} {:?}", rd.len - rd.data.len() as u32, rd); - state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32); + state.set_skip(STREAM_TOCLIENT, nbss_remaining); return; }, }; diff --git a/rust/src/smb/smb2.rs b/rust/src/smb/smb2.rs index af3bf174b2..aaa31df077 100644 --- a/rust/src/smb/smb2.rs +++ b/rust/src/smb/smb2.rs @@ -125,7 +125,7 @@ pub fn smb2_read_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n // Record claims more bytes than are in NBSS record... state.set_event(SMBEvent::ReadResponseTooLarge); // Skip the remaining bytes of the record. - state.set_skip(STREAM_TOCLIENT, nbss_remaining, 0); + state.set_skip(STREAM_TOCLIENT, nbss_remaining); return; } if r.nt_status == SMB_NTSTATUS_BUFFER_OVERFLOW { @@ -134,7 +134,7 @@ pub fn smb2_read_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n } else if r.nt_status != SMB_NTSTATUS_SUCCESS { SCLogDebug!("SMBv2: read response error code received: skip record"); - state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32); + state.set_skip(STREAM_TOCLIENT, nbss_remaining); return; } @@ -142,7 +142,7 @@ pub fn smb2_read_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n (unsafe { SMB_CFG_MAX_READ_SIZE != 0 && SMB_CFG_MAX_READ_SIZE < rd.len }) { state.set_event(SMBEvent::ReadResponseTooLarge); - state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32); + state.set_skip(STREAM_TOCLIENT, nbss_remaining); return; } @@ -155,7 +155,7 @@ pub fn smb2_read_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n Some(o) => (o.offset, o.guid), None => { SCLogDebug!("SMBv2 READ response: reply to unknown request {:?}",rd); - state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32); + state.set_skip(STREAM_TOCLIENT, nbss_remaining); return; }, }; @@ -172,10 +172,10 @@ pub fn smb2_read_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n } if max_queue_size != 0 && tdf.file_tracker.get_inflight_size() + rd.len as u64 > max_queue_size.into() { state.set_event(SMBEvent::ReadQueueSizeExceeded); - state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32); + state.set_skip(STREAM_TOCLIENT, nbss_remaining); } else if max_queue_cnt != 0 && tdf.file_tracker.get_inflight_cnt() >= max_queue_cnt as usize { state.set_event(SMBEvent::ReadQueueCntExceeded); - state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32); + state.set_skip(STREAM_TOCLIENT, nbss_remaining); } else { filetracker_newchunk(&mut tdf.file_tracker, files, flags, &tdf.file_name, rd.data, offset, @@ -228,7 +228,7 @@ pub fn smb2_read_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n smb_read_dcerpc_record(state, vercmd, hdr, &file_guid, rd.data); } else if is_pipe { SCLogDebug!("non-DCERPC pipe"); - state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32); + state.set_skip(STREAM_TOCLIENT, nbss_remaining); } else { let file_name = match state.guid2name_map.get(&file_guid) { Some(n) => { n.to_vec() } @@ -248,10 +248,10 @@ pub fn smb2_read_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n } if max_queue_size != 0 && tdf.file_tracker.get_inflight_size() + rd.len as u64 > max_queue_size.into() { state.set_event(SMBEvent::ReadQueueSizeExceeded); - state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32); + state.set_skip(STREAM_TOCLIENT, nbss_remaining); } else if max_queue_cnt != 0 && tdf.file_tracker.get_inflight_cnt() >= max_queue_cnt as usize { state.set_event(SMBEvent::ReadQueueCntExceeded); - state.set_skip(STREAM_TOCLIENT, rd.len, rd.data.len() as u32); + state.set_skip(STREAM_TOCLIENT, nbss_remaining); } else { filetracker_newchunk(&mut tdf.file_tracker, files, flags, &file_name, rd.data, offset, @@ -290,13 +290,13 @@ pub fn smb2_write_request_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n // Record claims more bytes than are in NBSS record... state.set_event(SMBEvent::WriteRequestTooLarge); // Skip the remaining bytes of the record. - state.set_skip(STREAM_TOSERVER, nbss_remaining, 0); + state.set_skip(STREAM_TOSERVER, nbss_remaining); 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); - state.set_skip(STREAM_TOSERVER, wr.wr_len, wr.data.len() as u32); + state.set_skip(STREAM_TOSERVER, nbss_remaining); return; } @@ -320,10 +320,10 @@ pub fn smb2_write_request_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n } if max_queue_size != 0 && tdf.file_tracker.get_inflight_size() + wr.wr_len as u64 > max_queue_size.into() { state.set_event(SMBEvent::WriteQueueSizeExceeded); - state.set_skip(STREAM_TOSERVER, wr.wr_len, wr.data.len() as u32); + state.set_skip(STREAM_TOSERVER, nbss_remaining); } else if max_queue_cnt != 0 && tdf.file_tracker.get_inflight_cnt() >= max_queue_cnt as usize { state.set_event(SMBEvent::WriteQueueCntExceeded); - state.set_skip(STREAM_TOSERVER, wr.wr_len, wr.data.len() as u32); + state.set_skip(STREAM_TOSERVER, nbss_remaining); } else { filetracker_newchunk(&mut tdf.file_tracker, files, flags, &file_name, wr.data, wr.wr_offset, @@ -376,7 +376,7 @@ pub fn smb2_write_request_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n smb_write_dcerpc_record(state, vercmd, hdr, wr.data); } else if is_pipe { SCLogDebug!("non-DCERPC pipe: skip rest of the record"); - state.set_skip(STREAM_TOSERVER, wr.wr_len, wr.data.len() as u32); + state.set_skip(STREAM_TOSERVER, nbss_remaining); } else { let (tx, files, flags) = state.new_file_tx(&file_guid, &file_name, STREAM_TOSERVER); tx.vercmd.set_smb2_cmd(SMB2_COMMAND_WRITE); @@ -390,10 +390,10 @@ pub fn smb2_write_request_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>, n if max_queue_size != 0 && tdf.file_tracker.get_inflight_size() + wr.wr_len as u64 > max_queue_size.into() { state.set_event(SMBEvent::WriteQueueSizeExceeded); - state.set_skip(STREAM_TOSERVER, wr.wr_len, wr.data.len() as u32); + state.set_skip(STREAM_TOSERVER, nbss_remaining); } else if max_queue_cnt != 0 && tdf.file_tracker.get_inflight_cnt() >= max_queue_cnt as usize { state.set_event(SMBEvent::WriteQueueCntExceeded); - state.set_skip(STREAM_TOSERVER, wr.wr_len, wr.data.len() as u32); + state.set_skip(STREAM_TOSERVER, nbss_remaining); } else { filetracker_newchunk(&mut tdf.file_tracker, files, flags, &file_name, wr.data, wr.wr_offset,