nfs3: improve read validation; fix partial handling

rust/src/nfs/nfs.rs

@@ -831,6 +831,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();
@@ -852,11 +863,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());
 
@@ -934,7 +940,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}",
@@ -1161,7 +1168,7 @@ impl NFSState {
                 // we should have enough data to parse the RPC record
                 match parse_rpc_reply(cur_i, 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);

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 {

rust/src/nfs/nfs3_records.rs

@@ -340,14 +340,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)?;
@@ -367,9 +367,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)?
     };
@@ -384,27 +384,22 @@ pub fn parse_nfs3_request_write(i: &[u8], complete: bool) -> IResult<&[u8], Nfs3
     Ok((i, req))
 }
 
-/*
+pub fn parse_nfs3_reply_read(i: &[u8], complete: bool) -> IResult<&[u8], NfsReplyRead> {
-#[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]) -> 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) = be_u32(i)?;
+    let (i, data_len) = verify(be_u32, |&v| v <= count)(i)?;
-    let (i, data) = take(data_len as usize)(i)?;
+    let fill_bytes = if data_len % 4 != 0 { 4 - data_len % 4 } else { 0 };
-    let (i, _data_padding) = cond(data_len % 4 !=0, take(4 - (data_len % 4)))(i)?;
+    // 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,
@@ -988,7 +983,7 @@ mod tests {
             0x00, /*_data_padding*/
         ];
 
-        let result = parse_nfs3_reply_read(buf).unwrap();
+        let result = parse_nfs3_reply_read(buf, true).unwrap();
         match result {
             (r, reply) => {
                 assert_eq!(r.len(), 0);

rust/src/nfs/rpc_records.rs

@@ -297,6 +297,10 @@ pub fn parse_rpc_reply(start_i: &[u8], complete: bool) -> IResult<&[u8], RpcRepl
     let (i, accept_state) = be_u32(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)?