From 922330c07a19797a448a50c5f82407d99bbeca37 Mon Sep 17 00:00:00 2001 From: Ievgen Bondarenko Date: Mon, 6 Jul 2026 11:08:43 -0700 Subject: [PATCH] nftables: validate csumOffset bounds in payloadSet evaluate ## Summary `payloadSet.evaluate` in `pkg/tcpip/nftables/nft_payload_set.go` reads and writes the IP checksum field at `payload[op.csumOffset:]` without validating `op.csumOffset` against `len(payload)`. When the rule's `NFTA_PAYLOAD_CSUM_OFFSET` netlink attribute is set so that the checksum field falls outside the packet payload, the indexed slice access panics at runtime with "slice bounds out of range" or "index out of range". ## Background `op.csumOffset` is sourced from the optional netlink attribute `NFTA_PAYLOAD_CSUM_OFFSET` in `initPayloadSet` (line 227). The comment at line 225 verbatim reads "Optional attributes; validation is not required." `newPayloadSet` does not validate it either. The data bounds check at line 105 already covers `op.offset+op.blen` (the data being written), but the checksum reads at lines 154 and 160 use a separate attacker-controlled offset attribute that skips the equivalent check. ## Linux behavior Linux kernel `net/netfilter/nft_payload.c::nft_payload_csum_inet` performs the equivalent bounds check via the `skb_copy_bits` and `skb_ensure_writable` helpers, which return -1 when the requested offset+size exceeds the skb length: ```c if (skb_copy_bits(skb, csum_offset, &sum, sizeof(sum)) < 0) return -1; ... if (skb_ensure_writable(skb, csum_offset + sizeof(sum)) || skb_store_bits(skb, csum_offset, &sum, sizeof(sum)) < 0) return -1; ``` The -1 return propagates to `nft_payload_set_eval` which then `goto err`. Linux gracefully fails the rule evaluation; gVisor's Go port uses raw slice indexing which panics on out-of-bounds instead of returning an error. ## Reachability Reachable from a process holding `CAP_NET_ADMIN` in the network namespace. The nftables netlink handler gates rule installation on `CAP_NET_ADMIN` at `pkg/sentry/socket/netlink/netfilter/protocol.go:76`: ```go if !s.NetworkNamespace().HasCapability(ctx, linux.CAP_NET_ADMIN) { return syserr.ErrNotPermittedNet } ``` Once the rule is installed, the malformed `csumOffset` value plus a sufficiently short packet that hits the rule trigger the panic at line 154. Severity is localized sandbox availability: the sentry panic kills the sandbox. This is not CVE-grade given the privilege requirement, but matches the hardening-class pattern of recent panic fixes such as `1985b831b9`, `25a1144119`, `c6c0e528b9`. ## Change Add an early `NFT_BREAK` return at the entry of the `NFT_PAYLOAD_CSUM_INET` branch when `int(op.csumOffset)+2 > len(payload)`. Returns the same `NFT_BREAK` verdict the existing offset+blen guard uses at line 105. 5 added lines, 0 deletions in `nft_payload_set.go`. Two new test cases in `nftables_test.go` exercise the boundary (`csumOffset == len` and `csumOffset > len`). No behavior change for well-formed rules. ## Test Verified at runtime via a standalone Go reproducer of the line 154 pattern. Output: ``` [csumOffset_eq_len] payloadLen=5 csumOffset=5 -> PANIC: runtime error: index out of range [1] with length 0 [csumOffset_gt_len] payloadLen=5 csumOffset=250 -> PANIC: runtime error: slice bounds out of range [250:5] [csumOffset_eq_len_minus_1] payloadLen=5 csumOffset=4 -> PANIC: runtime error: index out of range [1] with length 1 [csumOffset_safe] payloadLen=10 csumOffset=5 -> OK [empty_payload] payloadLen=0 csumOffset=0 -> PANIC: runtime error: index out of range [1] with length 0 ``` Added two `TestEvaluatePayloadSet` cases that exercise boundary conditions (csumOffset equal to len, csumOffset > len). Pre-fix these panic; post-fix they return `NFT_BREAK`. FUTURE_COPYBARA_INTEGRATE_REVIEW=https://github.com/google/gvisor/pull/13161 from ibondarenko1:hardening/nftables-csum-offset-bounds-check a3ee3ee7992a6c6825afc1c0a42d8b22503ba91b PiperOrigin-RevId: 943390357 --- pkg/tcpip/nftables/nft_payload_set.go | 7 +++++++ pkg/tcpip/nftables/nftables_test.go | 14 ++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/pkg/tcpip/nftables/nft_payload_set.go b/pkg/tcpip/nftables/nft_payload_set.go index e7b90892b1..ae169e4598 100644 --- a/pkg/tcpip/nftables/nft_payload_set.go +++ b/pkg/tcpip/nftables/nft_payload_set.go @@ -157,6 +157,13 @@ func (op payloadSet) evaluate(regs *registerSet, evalCtx opEvalCtx) { // Updates the checksum of the header specified by the payload base. if op.csumType == linux.NFT_PAYLOAD_CSUM_INET { + // Equivalent to Linux skb_copy_bits returning -1 when the + // checksum field is outside the packet; matches the data + // bounds check at line 105. + if int(op.csumOffset)+2 > len(payload) { + regs.verdict = stack.NFVerdict{Code: VC(linux.NFT_BREAK)} + return + } // Reads the old checksum from the packet payload. oldTotalCsum := binary.BigEndian.Uint16(payload[op.csumOffset:]) diff --git a/pkg/tcpip/nftables/nftables_test.go b/pkg/tcpip/nftables/nftables_test.go index bfc9c5483a..44c575b747 100644 --- a/pkg/tcpip/nftables/nftables_test.go +++ b/pkg/tcpip/nftables/nftables_test.go @@ -2087,6 +2087,20 @@ func TestEvaluatePayloadSet(t *testing.T) { op1: mustCreateImmediate(t, linux.NFT_REG_3, arbitraryIPv6AddrB[:], stack.NFVerdict{}), op2: mustCreatePayloadSet(t, linux.NFT_PAYLOAD_NETWORK_HEADER, ipv6DstAddrOffset, ipv6DstAddrLen, linux.NFT_REG_3, linux.NFT_PAYLOAD_CSUM_NONE, 0, linux.NFT_PAYLOAD_L4CSUM_PSEUDOHDR), }, + { + tname: "csumOffset equal to network header length triggers NFT_BREAK", + pkt: makeIPv4Packet(header.IPv4MinimumSize, arbitraryIPv4Fields()), + outPkt: nil, + op1: mustCreateImmediate(t, linux.NFT_REG_1, numToBE(0, ipv4LengthLen), stack.NFVerdict{}), + op2: mustCreatePayloadSet(t, linux.NFT_PAYLOAD_NETWORK_HEADER, ipv4LengthOffset, ipv4LengthLen, linux.NFT_REG_1, linux.NFT_PAYLOAD_CSUM_INET, header.IPv4MinimumSize, 0x0), + }, + { + tname: "csumOffset past network header length triggers NFT_BREAK", + pkt: makeIPv4Packet(header.IPv4MinimumSize, arbitraryIPv4Fields()), + outPkt: nil, + op1: mustCreateImmediate(t, linux.NFT_REG_1, numToBE(0, ipv4LengthLen), stack.NFVerdict{}), + op2: mustCreatePayloadSet(t, linux.NFT_PAYLOAD_NETWORK_HEADER, ipv4LengthOffset, ipv4LengthLen, linux.NFT_REG_1, linux.NFT_PAYLOAD_CSUM_INET, 250, 0x0), + }, } { t.Run(test.tname, func(t *testing.T) { // Sets up an NFTables object with a single table, chain, and rule.