mirror of
https://github.com/google/nsjail.git
synced 2026-08-30 18:41:30 -07:00
nstun/tcp: validate guest ACK numbers against the send window
The guest is untrusted and fully controls the acknowledgement number of
every TCP segment it emits. tcp_process_data() accepted any forward ACK
(acked_bytes > 0) without checking that it stayed within the send window,
so the guest could acknowledge data that was never sent.
Each such ACK advances tx_acked_offset by up to 2^31-1. When tx_buffer is
empty the erase step below is a no-op, so tx_acked_offset accumulates and
can be driven past 2^32 with a few pure ACK segments. push_to_guest() then
computes:
int32_t available = tx_buffer.size() - tx_acked_offset;
const uint8_t* data = tx_buffer.data() + tx_acked_offset + in_flight;
available is an int32_t truncation of a size_t subtraction, so an oversized
tx_acked_offset wraps it back positive and defeats the in_flight >= available
guard. data then points at least 2 GB past the heap buffer and
tcp_send_packet() reads up to NSTUN_MTU bytes from it.
The nstun network loop runs in the nsjail parent (supervisor) process, and
SIGSEGV is not handled, so the out of bounds read lets an untrusted jailed
process crash its own supervisor. The minimum out of bounds distance is 2 GB,
so under ASLR the access reliably faults rather than returning data.
Add the RFC 793 receive check (SEG.ACK <= SND.NXT): reject ACKs whose
sequence is beyond seq_to_guest. This keeps tx_acked_offset within
tx_buffer.size(), which the framing in push_to_guest() relies on.
This commit is contained in:
@@ -646,7 +646,14 @@ static void tcp_process_data(Context* ctx, TcpFlow* flow, const tcp_hdr* tcp,
|
||||
flow->syn_acked = true;
|
||||
} else {
|
||||
int32_t acked_bytes = ack - flow->ack_from_guest;
|
||||
if (acked_bytes > 0) {
|
||||
/*
|
||||
* RFC 793: an acceptable ACK must not acknowledge data
|
||||
* we never sent (SEG.ACK <= SND.NXT). Without this the
|
||||
* guest can advance tx_acked_offset past tx_buffer.size(),
|
||||
* which the framing in push_to_guest() relies on staying
|
||||
* within bounds.
|
||||
*/
|
||||
if (acked_bytes > 0 && (int32_t)(ack - flow->seq_to_guest) <= 0) {
|
||||
flow->ack_from_guest = ack;
|
||||
if (!flow->syn_acked) {
|
||||
flow->syn_acked = true;
|
||||
|
||||
Reference in New Issue
Block a user