From: Victor Julien Date: Mon, 21 Feb 2022 19:30:35 +0000 (+0100) Subject: nfs3: fix partial write record handling X-Git-Tag: suricata-7.0.0-beta1~868 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4418fc1b02f47533439fe00789d9c850a24271b2;p=thirdparty%2Fsuricata.git nfs3: fix partial write record handling --- diff --git a/rust/src/nfs/nfs.rs b/rust/src/nfs/nfs.rs index c9b0cd6128..b4e72c72ba 100644 --- a/rust/src/nfs/nfs.rs +++ b/rust/src/nfs/nfs.rs @@ -589,15 +589,19 @@ impl NFSState { } pub fn process_write_record<'b>(&mut self, r: &RpcPacket<'b>, w: &Nfs3RequestWrite<'b>) -> u32 { - // for now assume that stable FILE_SYNC flags means a single chunk - let is_last = if w.stable == 2 { true } else { false }; - let mut fill_bytes = 0; - let pad = w.file_len % 4; + let pad = w.count % 4; if pad != 0 { fill_bytes = 4 - pad; } + // linux defines a max of 1mb. Allow several multiples. + if w.count == 0 || w.count > 16777216 { + return 0; + } + + // for now assume that stable FILE_SYNC flags means a single chunk + let is_last = if w.stable == 2 { true } else { false }; let file_handle = w.handle.value.to_vec(); let file_name = if let Some(name) = self.namemap.get(w.handle.value) { SCLogDebug!("WRITE name {:?}", name); @@ -647,8 +651,8 @@ impl NFSState { } if !self.is_udp { self.ts_chunk_xid = r.hdr.xid; - let file_data_len = w.file_data.len() as u32 - fill_bytes as u32; - self.ts_chunk_left = w.file_len as u32 - file_data_len as u32; + debug_validate_bug_on!(w.file_data.len() as u32 > w.count); + self.ts_chunk_left = w.count - w.file_data.len() as u32; self.ts_chunk_fh = file_handle; SCLogDebug!("REQUEST chunk_xid {:04X} chunk_left {}", self.ts_chunk_xid, self.ts_chunk_left); } @@ -1000,7 +1004,7 @@ impl NFSState { // lets try to parse the RPC record. Might fail with Incomplete. match parse_rpc(cur_i, false) { Ok((remaining, ref hdr)) => { - match parse_nfs3_request_write(hdr.prog_data) { + match parse_nfs3_request_write(hdr.prog_data, false) { Ok((_, ref w)) => { // deal with the partial nfs write data self.process_partial_write_request_record(hdr, w); diff --git a/rust/src/nfs/nfs3.rs b/rust/src/nfs/nfs3.rs index 67d0b57a41..03acb4e850 100644 --- a/rust/src/nfs/nfs3.rs +++ b/rust/src/nfs/nfs3.rs @@ -73,7 +73,7 @@ impl NFSState { self.set_event(NFSEvent::MalformedData); }; } else if r.procedure == NFSPROC3_WRITE { - if let Ok((_, rd)) = parse_nfs3_request_write(r.prog_data) { + if let Ok((_, rd)) = parse_nfs3_request_write(r.prog_data, true) { self.process_write_record(r, &rd); } else { self.set_event(NFSEvent::MalformedData); diff --git a/rust/src/nfs/nfs3_records.rs b/rust/src/nfs/nfs3_records.rs index c605d08141..3526c65e09 100644 --- a/rust/src/nfs/nfs3_records.rs +++ b/rust/src/nfs/nfs3_records.rs @@ -17,6 +17,7 @@ //! Nom parsers for RPC & NFSv3 +use std::cmp; use crate::nfs::nfs_records::*; use nom7::bytes::streaming::take; use nom7::combinator::{complete, cond, rest, verify}; @@ -338,14 +339,40 @@ pub struct Nfs3RequestWrite<'a> { pub file_data: &'a [u8], } -pub fn parse_nfs3_request_write(i: &[u8]) -> IResult<&[u8], Nfs3RequestWrite> { +/// Complete data expected +fn parse_nfs3_request_write_data_complete(i: &[u8], file_len: usize, fill_bytes: usize) -> IResult<&[u8], &[u8]> { + let (i, file_data) = take(file_len as usize)(i)?; + let (i, _) = cond(fill_bytes > 0, take(fill_bytes))(i)?; + Ok((i, file_data)) +} + +/// Partial data. We have all file_len, but need to consider fill_bytes +fn parse_nfs3_request_write_data_partial(i: &[u8], file_len: usize, fill_bytes: usize) -> IResult<&[u8], &[u8]> { + let (i, file_data) = take(file_len as usize)(i)?; + let fill_bytes = cmp::min(fill_bytes as usize, i.len()); + let (i, _) = cond(fill_bytes > 0, take(fill_bytes))(i)?; + Ok((i, file_data)) +} + +/// Parse WRITE record. Consider 3 cases: +/// 1. we have the complete RPC data +/// 2. we have incomplete data but enough for all file data (partial fill bytes) +/// 3. we have incomplete file data +pub fn parse_nfs3_request_write(i: &[u8], complete: bool) -> IResult<&[u8], Nfs3RequestWrite> { let (i, handle) = parse_nfs3_handle(i)?; let (i, offset) = be_u64(i)?; let (i, count) = be_u32(i)?; let (i, stable) = verify(be_u32, |&v| v <= 2)(i)?; let (i, file_len) = verify(be_u32, |&v| v <= count)(i)?; - let (i, file_data) = take(file_len as usize)(i)?; - let (i, _file_padding) = cond(file_len % 4 !=0, take(4 - (file_len % 4)))(i)?; + let fill_bytes = if file_len % 4 != 0 { 4 - file_len % 4 } else { 0 }; + // Handle the various file data parsing logics + let (i, file_data) = if complete { + parse_nfs3_request_write_data_complete(i, file_len as usize, fill_bytes as usize)? + } else if i.len() >= file_len as usize { + parse_nfs3_request_write_data_partial(i, file_len as usize, fill_bytes as usize)? + } else { + rest(i)? + }; let req = Nfs3RequestWrite { handle, offset, @@ -356,6 +383,7 @@ pub fn parse_nfs3_request_write(i: &[u8]) -> IResult<&[u8], Nfs3RequestWrite> { }; Ok((i, req)) } + /* #[derive(Debug,PartialEq)] pub struct Nfs3ReplyRead<'a> { @@ -919,7 +947,7 @@ mod tests { let (_, expected_handle) = parse_nfs3_handle(&buf[..36]).unwrap(); - let result = parse_nfs3_request_write(buf).unwrap(); + let result = parse_nfs3_request_write(buf, true).unwrap(); match result { (r, request) => { assert_eq!(r.len(), 0); diff --git a/rust/src/nfs/rpc_records.rs b/rust/src/nfs/rpc_records.rs index 17f1a3e9b4..b8e3ed9a45 100644 --- a/rust/src/nfs/rpc_records.rs +++ b/rust/src/nfs/rpc_records.rs @@ -24,7 +24,8 @@ use nom7::combinator::{cond, verify, rest}; use nom7::multi::length_data; use nom7::number::streaming::{be_u32}; use nom7::sequence::tuple; -use nom7::IResult; +use nom7::error::{make_error, ErrorKind}; +use nom7::{IResult, Err}; pub const RPC_MAX_MACHINE_SIZE: u32 = 256; // Linux kernel defines 64. pub const RPC_MAX_CREDS_SIZE: u32 = 4096; // Linux kernel defines 400. @@ -240,6 +241,10 @@ pub fn parse_rpc(start_i: &[u8], complete: bool) -> IResult<&[u8], RpcPacket> { let (i, verifier) = take(verifier_len as usize)(i)?; let consumed = start_i.len() - i.len(); + if consumed > rec_size as usize { + return Err(Err::Error(make_error(i, ErrorKind::LengthValue))); + } + let data_size : u32 = (rec_size as usize - consumed) as u32; let (i, prog_data) = if !complete { rest(i)?