]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
nfs3: fix partial write record handling
authorVictor Julien <vjulien@oisf.net>
Mon, 21 Feb 2022 19:30:35 +0000 (20:30 +0100)
committerShivani Bhardwaj <shivanib134@gmail.com>
Fri, 4 Mar 2022 05:38:17 +0000 (11:08 +0530)
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)

rust/src/nfs/nfs.rs
rust/src/nfs/nfs3.rs
rust/src/nfs/nfs3_records.rs
rust/src/nfs/rpc_records.rs

index 487ec9faee71f9720bf666fcdb5a4e6dc33e0826..0a3e03837902cafc7eb9cc27aa7efec5d3be494e 100644 (file)
@@ -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);
index 44701ca13a5ba0f53a89faa44c70db1c6c432590..13406675ecb56f839fe9a1be318cb7263d707c4a 100644 (file)
@@ -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);
index 0df4c0c66f9413e75ebfbedf84c69f43358f980b..6dabe0acb7d61dbf368e0b78ec81730d276f0c98 100644 (file)
 
 //! 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<Nfs3RequestWrite>,
-    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<NfsReplyRead>,
             }
         ))
 );
+
+#[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: <DATA_SYNC> (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());
+            }
+        }
+    }
+}
index 4a9d7fe0901a4b02a75a4f978508b61898362ce6..04349e06810a924b7d5718a863a5b091364ce86d 100644 (file)
@@ -240,7 +240,13 @@ named_args!(pub parse_rpc(start_i: usize, complete: bool) <RpcPacket>,
        >> 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)