nfs3: improve read validation; fix partial handling

(cherry picked from commit d85b77cad064bd88c921b2f3d520fe526ad8ff82)

diff --git a/rust/src/nfs/nfs.rs b/rust/src/nfs/nfs.rs
index 0a3e03837902cafc7eb9cc27aa7efec5d3be494e..ad942809d1f93e7ef2cd90666d41ac04aa67f9b5 100644 (file)
--- a/rust/src/nfs/nfs.rs
+++ b/rust/src/nfs/nfs.rs
@@ -895,6 +895,17 @@ impl NFSState {
         let chunk_offset;
         let nfs_version;
 
+        let mut fill_bytes = 0;
+        let pad = reply.count % 4;
+        if pad != 0 {
+            fill_bytes = 4 - pad;
+        }
+
+        // linux defines a max of 1mb. Allow several multiples.
+        if reply.count == 0 || reply.count > 16777216 {
+            return 0;
+        }
+
         match xidmapr {
             Some(xidmap) => {
                 file_name = xidmap.file_name.to_vec();
@@ -916,11 +927,6 @@ impl NFSState {
         SCLogDebug!("chunk_offset {}", chunk_offset);
 
         let mut is_last = reply.eof;
-        let mut fill_bytes = 0;
-        let pad = reply.count % 4;
-        if pad != 0 {
-            fill_bytes = 4 - pad;
-        }
         SCLogDebug!("XID {} is_last {} fill_bytes {} reply.count {} reply.data_len {} reply.data.len() {}",
                 r.hdr.xid, is_last, fill_bytes, reply.count, reply.data_len, reply.data.len());
 
@@ -998,7 +1004,8 @@ impl NFSState {
 
         if !self.is_udp {
             self.tc_chunk_xid = r.hdr.xid;
-            self.tc_chunk_left = (reply.count as u32 + fill_bytes) - reply.data.len() as u32;
+            debug_validate_bug_on!(reply.data.len() as u32 > reply.count);
+            self.tc_chunk_left = reply.count - reply.data.len() as u32;
         }
 
         SCLogDebug!("REPLY {} to procedure {} blob size {} / {}: chunk_left {} chunk_xid {:04X}",
@@ -1224,7 +1231,7 @@ impl NFSState {
                 // we should have enough data to parse the RPC record
                 match parse_rpc_reply(cur_i, cur_i.len(), false) {
                     Ok((remaining, ref hdr)) => {
-                        match parse_nfs3_reply_read(hdr.prog_data) {
+                        match parse_nfs3_reply_read(hdr.prog_data, false) {
                             Ok((_, ref r)) => {
                                 // deal with the partial nfs read data
                                 self.process_partial_read_reply_record(hdr, r);

diff --git a/rust/src/nfs/nfs3.rs b/rust/src/nfs/nfs3.rs
index 13406675ecb56f839fe9a1be318cb7263d707c4a..4e99c00954ae453595a0582091db3a62f3553b43 100644 (file)
--- a/rust/src/nfs/nfs3.rs
+++ b/rust/src/nfs/nfs3.rs
@@ -220,7 +220,7 @@ impl NFSState {
                 self.set_event(NFSEvent::MalformedData);
             };
         } else if xidmap.procedure == NFSPROC3_READ {
-            if let Ok((_, rd)) = parse_nfs3_reply_read(r.prog_data) {
+            if let Ok((_, rd)) = parse_nfs3_reply_read(r.prog_data, true) {
                 self.process_read_record(r, &rd, Some(&xidmap));
                 nfs_status = rd.status;
             } else {

diff --git a/rust/src/nfs/nfs3_records.rs b/rust/src/nfs/nfs3_records.rs
index 6dabe0acb7d61dbf368e0b78ec81730d276f0c98..120ac36b0adc313b358b208643560872cecf1159 100644 (file)
--- a/rust/src/nfs/nfs3_records.rs
+++ b/rust/src/nfs/nfs3_records.rs
@@ -394,14 +394,14 @@ pub struct Nfs3RequestWrite<'a> {
 }
 
 /// Complete data expected
-fn parse_nfs3_request_write_data_complete(i: &[u8], file_len: usize, fill_bytes: usize) -> IResult<&[u8], &[u8]> {
+fn parse_nfs3_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]> {
+fn parse_nfs3_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)?;
@@ -421,9 +421,9 @@ pub fn parse_nfs3_request_write(i: &[u8], complete: bool) -> IResult<&[u8], Nfs3
     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)?
+        parse_nfs3_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)?
+        parse_nfs3_data_partial(i, file_len as usize, fill_bytes as usize)?
     } else {
         rest(i)?
     };
@@ -438,39 +438,33 @@ pub fn parse_nfs3_request_write(i: &[u8], complete: bool) -> IResult<&[u8], Nfs3
     Ok((i, req))
 }
 
-/*
-#[derive(Debug,PartialEq)]
-pub struct Nfs3ReplyRead<'a> {
-    pub status: u32,
-    pub attr_follows: u32,
-    pub attr_blob: &'a[u8],
-    pub count: u32,
-    pub eof: bool,
-    pub data_len: u32,
-    pub data: &'a[u8], // likely partial
+pub fn parse_nfs3_reply_read(i: &[u8], complete: bool) -> IResult<&[u8], NfsReplyRead> {
+    let (i, status) = be_u32(i)?;
+    let (i, attr_follows) = verify(be_u32, |&v| v <= 1)(i)?;
+    let (i, attr_blob) = take(84_usize)(i)?; // fixed size?
+    let (i, count) = be_u32(i)?;
+    let (i, eof) = verify(be_u32, |&v| v <= 1)(i)?;
+    let (i, data_len) = verify(be_u32, |&v| v <= count)(i)?;
+    let fill_bytes = if data_len % 4 != 0 { 4 - data_len % 4 } else { 0 };
+    // Handle the various file data parsing logics
+    let (i, data) = if complete {
+        parse_nfs3_data_complete(i, data_len as usize, fill_bytes as usize)?
+    } else if i.len() >= data_len as usize {
+        parse_nfs3_data_partial(i, data_len as usize, fill_bytes as usize)?
+    } else {
+        rest(i)?
+    };
+    let reply = NfsReplyRead {
+        status,
+        attr_follows,
+        attr_blob,
+        count,
+        eof: eof != 0,
+        data_len,
+        data,
+    };
+    Ok((i, reply))
 }
-*/
-named!(pub parse_nfs3_reply_read<NfsReplyRead>,
-    do_parse!(
-            status: be_u32
-        >>  attr_follows: verify!(be_u32, |&v| v <= 1)
-        >>  attr_blob: take!(84) // fixed size?
-        >>  count: be_u32
-        >>  eof: verify!(be_u32, |&v| v <= 1)
-        >>  data_len: be_u32
-        >>  data_contents: rest
-        >> (
-            NfsReplyRead {
-                status:status,
-                attr_follows:attr_follows,
-                attr_blob:attr_blob,
-                count:count,
-                eof:eof != 0,
-                data_len:data_len,
-                data:data_contents,
-            }
-        ))
-);
 
 #[cfg(test)]
 mod tests {
@@ -513,4 +507,46 @@ mod tests {
             }
         }
     }
+
+    #[test]
+    fn test_nfs3_reply_read() {
+
+        #[rustfmt::skip]
+        let buf: &[u8] = &[
+            0x00, 0x00, 0x00, 0x00, /*Status: NFS3_OK (0)*/
+            0x00, 0x00, 0x00, 0x01, /*attributes_follows: (1)*/
+            0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x81, 0xa4, /*attr_blob*/
+            0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00,
+            0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00,
+            0x00, 0x00, 0x00, 0x0b, 0x00, 0x00, 0x00, 0x00,
+            0x00, 0x00, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
+            0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+            0x00, 0x10, 0x10, 0x85, 0x00, 0x00, 0x00, 0x00,
+            0x00, 0x00, 0xb2, 0x5d, 0x38, 0x47, 0x76, 0x25,
+            0x23, 0xc3, 0x46, 0x00, 0x38, 0x47, 0x71, 0xc4,
+            0x21, 0xf9, 0x82, 0x80, 0x38, 0x47, 0x76, 0x25,
+            0x1e, 0x65, 0xfb, 0x81,
+            0x00, 0x00, 0x00, 0x0b, /*count: (11)*/
+            0x00, 0x00, 0x00, 0x01, /*EOF: (true)*/
+        // [data]
+            0x00, 0x00, 0x00, 0x0b, /*data_len: (11)*/
+            0x74, 0x68, 0x65, 0x20, 0x62, 0x20, 0x66, 0x69,
+            0x6c, 0x65, 0x0a, /*data: ("the b file\n")*/
+            0x00, /*_data_padding*/
+        ];
+
+        let result = parse_nfs3_reply_read(buf, true).unwrap();
+        match result {
+            (r, reply) => {
+                assert_eq!(r.len(), 0);
+                assert_eq!(reply.status, 0);
+                assert_eq!(reply.attr_follows, 1);
+                assert_eq!(reply.attr_blob.len(), 84);
+                assert_eq!(reply.count, 11);
+                assert_eq!(reply.eof, true);
+                assert_eq!(reply.data_len, 11);
+                assert_eq!(reply.data, "the b file\n".as_bytes());
+            }
+        }
+    }
 }

diff --git a/rust/src/nfs/rpc_records.rs b/rust/src/nfs/rpc_records.rs
index 04349e06810a924b7d5718a863a5b091364ce86d..d3988a096f5baf095f4f1d840db879c323bd1dc5 100644 (file)
--- a/rust/src/nfs/rpc_records.rs
+++ b/rust/src/nfs/rpc_records.rs
@@ -294,7 +294,11 @@ named_args!(pub parse_rpc_reply(start_i: usize, complete: bool) <RpcReplyPacket>
        >> verifier: cond!(verifier_len > 0, take!(verifier_len as usize))
 
        >> accept_state: be_u32
-       >> rem_len: rest_len
+       >> 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)