aiden
/
suricata
mirror of https://github.com/OISF/suricata
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

nfs: check a v3 WRITE and READ claim against the record

Browse Source 
The args of a WRITE request and of a READ reply were trusted to fit the
record that carries them. Where the claim fell below the extent the parse
covers, the file data ran past the record boundary into the next RPC,
which was then read as a record. A max-tx of 0 was accepted and evicted
every transaction on insert.

Compare the claim against that extent: for a WRITE the handle, offset,
count, stable flag and its data, for a READ the status, the optional
attributes, the count, the eof flag and its data. A record whose claim is
below the extent gets a transaction with the malformed data event and a
skip to the record boundary, as an oversized claim already does. Only the
v3 records arrive in pieces, so process_read_record now learns which
reply it is given and the check is v3's. Reject a max-tx of 0 at config
load.

Ticket: #9263.
pull/16529/head
Victor Julien 1 week ago
parent 4b22de33d8
commit e08474a283
4 changed files with 90 additions and 12 deletions
Show Stats Download Patch File Download Diff File

94
rust/src/nfs/nfs.rs
Unescape Escape View File

@ -1188,6 +1188,12 @@ impl NFSState {
pub fn process_write_record<'b>(
&mut self, flow: *mut Flow, r: &RpcPacket<'b>, w: &Nfs3RequestWrite<'b>,
) -> u32 {
// the parsed extent within prog_data: fh len + fh + offset + count
// + stable + file_len + data; the XDR fill follows it when unaligned
let extent = 24u32
.saturating_add(w.handle.len)
.saturating_add(w.file_len);
// for now assume that stable FILE_SYNC flags means a single chunk
let is_last = w.stable == 2;
// a partial record's file tx must stay open (found by handle) until
@ -1202,8 +1208,42 @@ impl NFSState {
Vec::new()
};
// The record must cover the parsed extent: a claim shorter than the
// args would run the file data past the record boundary into the next
// RPC. A claim larger than the extent is not an error, it is the
// record tail (the XDR pad, or bytes the sender put in the record
// after the args): the chunk promise covers it so it is never parsed.
// No alignment is assumed, some senders do not pad the handle.
// unbuffered record tail (file data + XDR fill) left in the stream
let tail = record_tail(r.prog_data_size, r.prog_data.len());
if !self.is_udp && extent > r.prog_data_size {
SCLogDebug!(
"WRITE claim below extent (claim {} extent {}): skipping {} stream bytes",
r.prog_data_size,
extent,
tail
);
// rejected tx + event, same pattern as the too-large arm
let tx = self.new_file_tx(&file_handle, &file_name, Direction::ToServer);
tx.procedure = NFSPROC3_WRITE;
tx.xid = r.hdr.xid;
tx.is_first = true;
tx.nfs_version = r.progver as u16;
tx.request_done = true;
tx.response_done = true;
tx.is_file_closed = true;
tx.tx_data.set_event(NFSEvent::MalformedData as u8);
tx.tx_data.0.updated_ts = true;
tx.tx_data.0.updated_tc = true;
tx.tx_data.0.files_opened = 0;
self.set_skip(Direction::ToServer, tail);
self.ts_chunk_xid = 0;
self.ts_chunk_left = 0;
self.ts_chunk_tail = 0;
self.ts_chunk_fh.clear();
return 0;
}
if w.count == 0 {
// No file data, but the rest of a partial record must not be
// parsed as the next RPC record: skip to the record boundary.
@ -1894,13 +1934,14 @@ impl NFSState {
/// have a complete record. Otherwise we do a lookup ourselves.
pub fn process_read_record<'b>(
&mut self, flow: *mut Flow, r: &RpcReplyPacket<'b>, reply: &NfsReplyRead<'b>,
xidmapr: Option<&NFSRequestXidMap>,
xidmapr: Option<&NFSRequestXidMap>, v3: bool,
) -> u32 {
let file_name;
let file_handle;
let chunk_offset;
let nfs_version;
// The record must cover the parsed extent (status + attr_follows +
// [post_op_attr] + count + eof + data_len + data); a larger claim is
// the record tail, which the chunk promise consumes
let extent = 20u32
.saturating_add(if reply.attr_follows != 0 { 84 } else { 0 })
.saturating_add(reply.data_len);
// unbuffered record tail (reply data + XDR fill) left in the stream
let tail = record_tail(r.prog_data_size, r.prog_data.len());
@ -1918,6 +1959,38 @@ impl NFSState {
return 0;
}
if v3 && !self.is_udp && extent > r.prog_data_size {
SCLogDebug!(
"READ claim below extent (claim {} extent {}): skipping {} stream bytes",
r.prog_data_size,
extent,
tail
);
// rejected tx + event, same pattern as the v4 malformed-response arm
let mut tx = self.new_tx();
tx.procedure = NFSPROC3_READ;
tx.xid = r.hdr.xid;
tx.nfs_version = self.nfs_version;
tx.request_done = true;
tx.response_done = true;
tx.is_file_closed = true;
tx.tx_data.set_event(NFSEvent::MalformedData as u8);
tx.tx_data.0.updated_ts = true;
tx.tx_data.0.updated_tc = true;
self.transactions.push(tx);
self.set_skip(Direction::ToClient, tail);
self.tc_chunk_xid = 0;
self.tc_chunk_left = 0;
self.tc_chunk_tail = 0;
self.tc_chunk_fh.clear();
return 0;
}
let file_name;
let file_handle;
let chunk_offset;
let nfs_version;
match xidmapr {
Some(xidmap) => {
file_name = xidmap.file_name.to_vec();
@ -2217,7 +2290,7 @@ impl NFSState {
reply.count
);
return self.process_read_record(flow, r, reply, None);
return self.process_read_record(flow, r, reply, None, true);
}
fn peek_reply_record(&mut self, r: &RpcPacketHeader) -> u32 {
@ -2701,7 +2774,7 @@ impl NFSState {
data_len: 0,
data: &[],
};
self.process_read_record(flow, hdr, &reply, None);
self.process_read_record(flow, hdr, &reply, None, false);
return AppLayerResult::ok();
}
// Definitive structural error: skip the record (bounded
@ -3613,7 +3686,12 @@ pub unsafe extern "C" fn SCRegisterNfsUdpParser() {
}
if let Some(val) = conf_get("app-layer.protocols.nfs.max-tx") {
if let Ok(v) = val.parse::<usize>() {
// 0 would evict every transaction on insert
if v == 0 {
SCLogError!("Invalid value for nfs.max-tx");
} else {
NFS_MAX_TX = v;
}
} else {
SCLogError!("Invalid value for nfs.max-tx");
}

2
rust/src/nfs/nfs2.rs
Unescape Escape View File

@ -113,7 +113,7 @@ impl NFSState {
match parse_nfs2_reply_read(r.prog_data) {
Ok((_, ref reply)) => {
SCLogDebug!("NFSv2: READ reply record");
self.process_read_record(flow, r, reply, Some(xidmap));
self.process_read_record(flow, r, reply, Some(xidmap), false);
nfs_status = reply.status;
}
_ => {

2
rust/src/nfs/nfs3.rs
Unescape Escape View File

@ -237,7 +237,7 @@ impl NFSState {
};
} else if xidmap.procedure == NFSPROC3_READ {
if let Ok((_, rd)) = parse_nfs3_reply_read(r.prog_data, true) {
self.process_read_record(flow, r, &rd, Some(xidmap));
self.process_read_record(flow, r, &rd, Some(xidmap), true);
nfs_status = rd.status;
} else {
self.set_event(NFSEvent::MalformedData);

2
rust/src/nfs/nfs4.rs
Unescape Escape View File

@ -438,7 +438,7 @@ impl NFSState {
data_len: rd.data.len() as u32,
data: rd.data,
};
self.process_read_record(flow, r, &reply, Some(xidmap));
self.process_read_record(flow, r, &reply, Some(xidmap), false);
}
Nfs4ResponseContent::Open(_s, Some(ref _rd)) => {
SCLogDebug!("OPENv4: status {} opendata {:?}", _s, _rd);

Write Preview
Loading…
Cancel
Save