Skip to content

fix(network-stack): Fix TCP sequence number wraparound in qemu network stack #119

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dizk wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(network-stack): Fix TCP sequence number wraparound in qemu network stack #119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 54 additions & 17 deletions host/src/qemu/network-stack.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,41 @@ function makeTcpNatKey(
return `TCP:${srcIP}:${srcPort}:${dstIP}:${dstPort}`;
}

// TCP sequence and acknowledgement numbers are 32 bits wide and wrap at 2^32.
// We keep every stored counter (mySeq/myAck/vmSeq/vmAck) masked into [0, 2^32)
// and compare them with RFC 1982 serial-number arithmetic instead of raw </>,
// which break across the wrap boundary. Skipping either half re-introduces the
// `writeUInt32BE` overflow crash or silent reassembly desync near 0xFFFFFFFF.

/** Fold a value into the 32-bit TCP sequence space [0, 2^32). */
function wrapSeq(value: number): number {
return value >>> 0;
}

/**
* Signed RFC 1982 distance between two sequence numbers: > 0 when `a` is "after"
* `b`, < 0 when "before", 0 when equal. ToInt32 (`| 0`) folds the difference into
* the [-2^31, 2^31) serial window so comparisons stay correct across wrap.
*/
function seqDistance(a: number, b: number): number {
return (a - b) | 0;
}

/** `a` is strictly after `b` in serial-number order. */
function seqGt(a: number, b: number): boolean {
return seqDistance(a, b) > 0;
}

/** `a` is strictly before `b` in serial-number order. */
function seqLt(a: number, b: number): boolean {
return seqDistance(a, b) < 0;
}

/** `a` is at or before `b` in serial-number order. */
function seqLe(a: number, b: number): boolean {
return seqDistance(a, b) <= 0;
}

const HTTP_METHODS = [
"GET",
"POST",
Expand Down Expand Up @@ -753,7 +788,7 @@ export class NetworkStack extends EventEmitter {
vmSeq: seq,
vmAck: ack,
mySeq: Math.floor(Math.random() * 0x0fffffff),
myAck: seq + 1,
myAck: wrapSeq(seq + 1),
peerWindow: window,
pendingOutbound: Buffer.alloc(0),
endPending: false,
Expand Down Expand Up @@ -785,7 +820,7 @@ export class NetworkStack extends EventEmitter {
dstIP,
dstPort,
0,
seq + (payload.length || 1),
wrapSeq(seq + (payload.length || 1)),
0x04,
);
}
Expand All @@ -796,7 +831,7 @@ export class NetworkStack extends EventEmitter {
session.peerWindow = window;

let shouldDrainOutbound = false;
if (ack > session.vmAck && ack <= session.mySeq) {
if (seqGt(ack, session.vmAck) && seqLe(ack, session.mySeq)) {
session.vmAck = ack;
shouldDrainOutbound = true;
}
Expand All @@ -817,7 +852,7 @@ export class NetworkStack extends EventEmitter {
// protocol and trigger errors like "Bad packet length".
const expectedSeq = session.myAck;

if (seq > expectedSeq) {
if (seqGt(seq, expectedSeq)) {
// Out-of-order: re-ACK what we've already seen.
this.sendTCP(
session.srcIP,
Expand All @@ -831,7 +866,7 @@ export class NetworkStack extends EventEmitter {
return;
}

const skip = Math.max(0, expectedSeq - seq);
const skip = Math.max(0, seqDistance(expectedSeq, seq));
if (skip >= payload.length) {
// Pure retransmit (or segment contains only already-acked bytes)
this.sendTCP(
Expand All @@ -850,7 +885,7 @@ export class NetworkStack extends EventEmitter {

const newPayload = payload.subarray(skip);
let sendBuffer: Buffer | null = null;
const nextAck = expectedSeq + newPayload.length;
const nextAck = wrapSeq(expectedSeq + newPayload.length);

if (!session.flowProtocol) {
session.pendingData = Buffer.concat([session.pendingData, newPayload]);
Expand Down Expand Up @@ -930,8 +965,8 @@ export class NetworkStack extends EventEmitter {
if (FIN) {
// FIN consumes one sequence number, so only accept it once the sequence
// space up to the FIN is fully received.
const finSeq = seq + payload.length;
if (finSeq > session.myAck) {
const finSeq = wrapSeq(seq + payload.length);
if (seqGt(finSeq, session.myAck)) {
// Out-of-order FIN: keep ACKing the last in-order byte.
this.sendTCP(
session.srcIP,
Expand All @@ -945,7 +980,7 @@ export class NetworkStack extends EventEmitter {
return;
}

if (finSeq < session.myAck) {
if (seqLt(finSeq, session.myAck)) {
// Duplicate FIN (already acked)
this.sendTCP(
session.srcIP,
Expand All @@ -961,7 +996,7 @@ export class NetworkStack extends EventEmitter {

// finSeq === session.myAck
this.callbacks.onTcpClose({ key, destroy: false });
session.myAck++;
session.myAck = wrapSeq(session.myAck + 1);

this.sendTCP(
session.srcIP,
Expand Down Expand Up @@ -997,8 +1032,10 @@ export class NetworkStack extends EventEmitter {
const header = Buffer.alloc(20);
header.writeUInt16BE(srcPort, 0);
header.writeUInt16BE(dstPort, 2);
header.writeUInt32BE(seq, 4);
header.writeUInt32BE(ack, 8);
// Counters are kept wrapped at their mutation sites; mask again here so the
// serialization boundary can never emit an out-of-range uint32 (the crash).
header.writeUInt32BE(wrapSeq(seq), 4);
header.writeUInt32BE(wrapSeq(ack), 8);
header[12] = 0x50;
header[13] = flags;
header.writeUInt16BE(65535, 14);
Expand Down Expand Up @@ -1325,7 +1362,7 @@ export class NetworkStack extends EventEmitter {
return;
}

const inFlight = Math.max(0, session.mySeq - session.vmAck);
const inFlight = Math.max(0, seqDistance(session.mySeq, session.vmAck));
const maxInFlight = Math.max(
0,
Math.min(session.peerWindow, this.TCP_MAX_IN_FLIGHT_BYTES),
Expand All @@ -1351,7 +1388,7 @@ export class NetworkStack extends EventEmitter {

const MSS = 1460;
let bytesBurstThisTick = 0;
let inFlight = Math.max(0, session.mySeq - session.vmAck);
let inFlight = Math.max(0, seqDistance(session.mySeq, session.vmAck));
const maxInFlight = Math.max(
0,
Math.min(session.peerWindow, this.TCP_MAX_IN_FLIGHT_BYTES),
Expand Down Expand Up @@ -1391,7 +1428,7 @@ export class NetworkStack extends EventEmitter {
return;
}

session.mySeq += chunk.length;
session.mySeq = wrapSeq(session.mySeq + chunk.length);
inFlight += chunk.length;
bytesBurstThisTick += chunk.length;
}
Expand Down Expand Up @@ -1432,7 +1469,7 @@ export class NetworkStack extends EventEmitter {
return;
}

session.mySeq++;
session.mySeq = wrapSeq(session.mySeq + 1);
session.state = "FIN_WAIT";
session.endPending = false;
inFlight += 1;
Expand Down Expand Up @@ -1464,7 +1501,7 @@ export class NetworkStack extends EventEmitter {
session.myAck,
0x12,
);
session.mySeq++;
session.mySeq = wrapSeq(session.mySeq + 1);
}

handleTcpData(message: { key: string; data: Buffer }) {
Expand Down
91 changes: 91 additions & 0 deletions host/test/network-stack.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1759,3 +1759,94 @@ test("network-stack: dropped outbound TCP payload tears down session", () => {
"dead flow must not remain in txFlowPaused",
);
});

test("network-stack: TCP ack past 2^32 must wrap on the wire, not throw", () => {
// Regression: a 32-bit seq/ack overflow crashed a downstream process on 0.12.0.
// RangeError [ERR_OUT_OF_RANGE] ... Received 4_294_967_340
// at NetworkStack.sendTCP (network-stack.ts:1001) writeUInt32BE(ack,8), unmasked
// at NetworkStack.drainOutboundTcp (network-stack.ts:1422)
// at NetworkStack.handleTcpEnd (network-stack.ts:1497)
// session.myAck is seeded from the guest's 32-bit SYN ISN (myAck = seq + 1, :756)
// and only grows (:910 data, :964 FIN), never reduced mod 2^32 -- so a high guest
// ISN overflows the unmasked write at :1001. Deterministic: the ISN is an input.
const gatewayMac = mac([0x5a, 0x94, 0xef, 0xe4, 0x0c, 0xdd]);
const vmMac = mac([0x02, 0x00, 0x00, 0x00, 0x00, 0x01]);

let key = "";
const stack = new NetworkStack({
gatewayMac,
vmMac,
dnsServers: ["8.8.8.8"],
allowTcpFlow: () => true, // raw-tcp bypass: deliver bytes as-is, no deny/teardown
callbacks: {
onUdpSend: () => {},
onTcpConnect: (m) => {
key = m.key;
return { allowRawTcp: true } as any;
},
onTcpSend: () => {},
onTcpClose: () => {},
onTcpPause: () => {},
onTcpResume: () => {},
},
});

const srcIP = ip([192, 168, 127, 3]);
const dstIP = ip([93, 184, 216, 34]);
const srcPort = 40123;
const dstPort = 80;

// ISN+1 (SYN) +60 (data) = 4_294_967_340 = 2^32 + 44; wrapped, the ack must be 44.
const ISN = 0xffffffff - 16;
const WRAPPED_ACK = (ISN + 1 + 60) % 0x1_0000_0000;

stack.handleTCP(
buildTcpSegment({ srcPort, dstPort, seq: ISN, ack: 0, flags: 0x02 }),
srcIP,
dstIP,
); // SYN
stack.handleTcpConnected({ key }); // SYN/ACK; myAck = 2^32 - 16 (still a valid uint32)
drainAllQemuTx(stack);
// mySeq is random; read it back so the guest can ACK the SYN/ACK (opens the window
// so teardown actually sends a FIN). Trigger stays the crafted ISN, not mySeq.
const hostSeq = (stack as any).natTable.get(key).mySeq as number;

// 60 bytes pushes myAck to 2^32 + 44. The data ACK at :918 overflows first, but in
// production receive()'s try/catch (:471/:486) swallows it -- we swallow it here too
// so the test reaches the *unwrapped* teardown path that actually crashes uncaught.
try {
stack.handleTCP(
buildTcpSegment({
srcPort,
dstPort,
seq: ISN + 1,
ack: hostSeq,
flags: 0x18,
payload: Buffer.alloc(60, 0x61),
}),
srcIP,
dstIP,
);
} catch (err) {
// expected only on 0.12.0: the data-ACK overflow that receive() swallows in
// production. Anything else is a real setup failure and must surface here.
assert.equal((err as NodeJS.ErrnoException).code, "ERR_OUT_OF_RANGE");
}

// Upstream close -> drainOutboundTcp FIN with myAck (2^32 + 44). handleTcpEnd has no
// try/catch, so on 0.12.0 this throws uncaught. Correct: wrap to uint32, don't throw.
assert.doesNotThrow(
() => stack.handleTcpEnd({ key }),
"teardown must wrap seq/ack, not throw",
);

const fin = decodeFramesFromQemuData(drainAllQemuTx(stack))
.map(parseEthernet)
.filter((eth) => eth.etherType === 0x0800)
.map((eth) => parseIPv4(eth.payload))
.filter((ipOut) => ipOut.protocol === 6)
.map((ipOut) => ipOut.payload)
.find((tcp) => (tcp[13] & 0x01) !== 0);
assert.ok(fin, "expected an outbound FIN segment during teardown");
assert.equal(fin!.readUInt32BE(8), WRAPPED_ACK, "ack must wrap mod 2^32");
});