From: Victor Julien Date: Mon, 21 Feb 2022 19:30:35 +0000 (+0100) Subject: nfs3: fix partial write record handling X-Git-Tag: suricata-6.0.5~95 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=29468e60f50925bc33d4b78cbf0595ace949f28a;p=thirdparty%2Fsuricata.git nfs3: fix partial write record handling Comment from Jason Ish: This was a bit more than a cherry pick as some of the modifications weren't directly portable to the nom macros. So instead bring in some of the nom function based parsers as they work fine with nom 5. (cherry picked from commit 4418fc1b02f47533439fe00789d9c850a24271b2) --- diff --git a/rust/src/nfs/nfs.rs b/rust/src/nfs/nfs.rs index 487ec9faee..0a3e038379 100644 --- a/rust/src/nfs/nfs.rs +++ b/rust/src/nfs/nfs.rs @@ -653,15 +653,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); @@ -711,8 +715,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); } @@ -1064,7 +1068,7 @@ impl NFSState { // lets try to parse the RPC record. Might fail with Incomplete. match parse_rpc(cur_i, cur_i.len(), 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 44701ca13a..13406675ec 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 0df4c0c66f..6dabe0acb7 100644 --- a/rust/src/nfs/nfs3_records.rs +++ b/rust/src/nfs/nfs3_records.rs @@ -17,8 +17,10 @@ //! Nom parsers for RPC & NFSv3 +use std::cmp; use nom::IResult; -use nom::combinator::rest; +use nom::combinator::{rest, verify, cond}; +use nom::bytes::complete::take; use nom::number::streaming::{be_u32, be_u64}; use crate::nfs::nfs_records::*; @@ -391,25 +393,51 @@ pub struct Nfs3RequestWrite<'a> { pub file_data: &'a[u8], } -named!(pub parse_nfs3_request_write, - do_parse!( - handle: parse_nfs3_handle - >> offset: be_u64 - >> count: be_u32 - >> stable: verify!(be_u32, |&v| v <= 2) - >> file_len: verify!(be_u32, |&v| v <= count) - >> file_data: rest // likely partial - >> ( - Nfs3RequestWrite { - handle:handle, - offset:offset, - count:count, - stable:stable, - file_len:file_len, - file_data:file_data, - } - )) -); +/// 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 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, + count, + stable, + file_len, + file_data, + }; + Ok((i, req)) +} + /* #[derive(Debug,PartialEq)] pub struct Nfs3ReplyRead<'a> { @@ -443,3 +471,46 @@ named!(pub parse_nfs3_reply_read, } )) ); + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_nfs3_request_write() { + + #[rustfmt::skip] + let buf: &[u8] = &[ + // [handle] + 0x00, 0x00, 0x00, 0x20, /*handle_len: (32)*/ + 0x00, 0x10, 0x10, 0x85, 0x00, 0x00, 0x03, 0xe7, /*handle*/ + 0x00, 0x0a, 0x00, 0x00, 0x00, 0x00, 0xa6, 0x54, + 0x00, 0x00, 0x00, 0x1b, 0x00, 0x0a, 0x00, 0x00, + 0x00, 0x00, 0xb2, 0x5a, 0x00, 0x00, 0x00, 0x29, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*offset: (0)*/ + 0x00, 0x00, 0x00, 0x11, /*count: (17)*/ + 0x00, 0x00, 0x00, 0x01, /*stable: (1)*/ + // [data] + 0x00, 0x00, 0x00, 0x11, /*file_len: (17)*/ + 0x68, 0x61, 0x6c, 0x6c, 0x6f, 0x0a, 0x74, 0x68, /*file_data: ("hallo\nthe b file\n")*/ + 0x65, 0x20, 0x62, 0x20, 0x66, 0x69, 0x6c, 0x65, + 0x0a, + 0x00, 0x00, 0x00, /*_data_padding*/ + ]; + + let (_, expected_handle) = parse_nfs3_handle(&buf[..36]).unwrap(); + + let result = parse_nfs3_request_write(buf, true).unwrap(); + match result { + (r, request) => { + assert_eq!(r.len(), 0); + assert_eq!(request.handle, expected_handle); + assert_eq!(request.offset, 0); + assert_eq!(request.count, 17); + assert_eq!(request.stable, 1); + assert_eq!(request.file_len, 17); + assert_eq!(request.file_data, "hallo\nthe b file\n".as_bytes()); + } + } + } +} diff --git a/rust/src/nfs/rpc_records.rs b/rust/src/nfs/rpc_records.rs index 4a9d7fe090..04349e0681 100644 --- a/rust/src/nfs/rpc_records.rs +++ b/rust/src/nfs/rpc_records.rs @@ -240,7 +240,13 @@ named_args!(pub parse_rpc(start_i: usize, complete: bool) , >> verifier_flavor: be_u32 >> verifier_len: verify!(be_u32, |&size| size < RPC_MAX_VERIFIER_SIZE) >> verifier: take!(verifier_len as usize) - >> rem_len: rest_len + // Gets the value out of the header that we can move into a closure for + // verification. + >> frag_len: value!(hdr.frag_len) + >> rem_len: verify!(rest_len, move |&rem_len| { + let consumed = start_i - rem_len; + consumed <= frag_len as usize + 4 + }) >> prog_data_size: value!(hdr.frag_len + 4 - (start_i - rem_len) as u32) >> prog_data_f: cond!(complete, take!(prog_data_size)) >> prog_data_t: cond!(!complete, rest)