From 62bef62c9185d9061a112553ae7d839c526ceec5 Mon Sep 17 00:00:00 2001
From: Harsh23Kashyap <55448981+Harsh23Kashyap@users.noreply.github.com>
Date: Wed, 16 Sep 2026 15:18:53 +0530
Subject: [PATCH] face: validate NDNLPv2 fragmentation fields before reassembly

A single incoming NDNLPv2 frame carries attacker-controlled FragCount
and FragIndex values that were trusted before allocation:

- reassemble() sized its buffer with make(enc.Wire, fragCount), so one
  small fragment with a huge FragCount forces a proportional memory
  allocation (e.g. FragCount=1<<40 would exhaust memory).
- handleIncomingFrame computed baseSequence = Sequence - FragIndex
  before validation, underflowing to a huge sequence number when
  FragIndex > Sequence and polluting a reassembly slot.

Reject frames with FragCount == 0, FragCount above the maximum possible
for a legal packet (MaxNDNPacketSize, since every fragment carries at
least one byte), FragIndex >= FragCount, or FragIndex > Sequence before
any reassembly state is allocated. The ring buffer introduced in #96
bounds the number of concurrent reassemblies but not their size.
---
 fw/face/ndnlp-link-service.go      | 18 ++++++++
 fw/face/ndnlp_link_service_test.go | 67 ++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+)
 create mode 100644 fw/face/ndnlp_link_service_test.go

diff --git a/fw/face/ndnlp-link-service.go b/fw/face/ndnlp-link-service.go
index 9e1ffe66..7e1f260d 100644
--- a/fw/face/ndnlp-link-service.go
+++ b/fw/face/ndnlp-link-service.go
@@ -56,6 +56,11 @@ func MakeNDNLPLinkServiceOptions() NDNLPLinkServiceOptions {
 	}
 }
 
+// maxReassemblyFragments bounds the fragment count of a single reassembled
+// packet. A valid L3 packet is at most MaxNDNPacketSize bytes and every
+// fragment carries at least one byte, so a larger count can never complete.
+const maxReassemblyFragments = defn.MaxNDNPacketSize
+
 // NDNLPLinkService is a link service implementing the NDNLPv2 link protocol
 type NDNLPLinkService struct {
 	linkServiceBase
@@ -317,6 +322,12 @@ func (l *NDNLPLinkService) handleIncomingFrame(frame []byte) {
 			if v, ok := LP.FragCount.Get(); ok {
 				fragCount = v
 			}
+			if fragCount == 0 || fragCount > maxReassemblyFragments ||
+				fragIndex >= fragCount || fragIndex > LP.Sequence.Unwrap() {
+				core.Log.Warn(l, "Received frame with invalid fragmentation fields - DROP",
+					"index", fragIndex, "count", fragCount, "sequence", LP.Sequence.Unwrap())
+				return
+			}
 			baseSequence := LP.Sequence.Unwrap() - fragIndex
 
 			core.Log.Trace(l, "Received fragment", "index", fragIndex, "count", fragCount, "base", baseSequence)
@@ -374,6 +385,13 @@ func (l *NDNLPLinkService) reassemble(
 	fragIndex uint64,
 	fragCount uint64,
 ) enc.Wire {
+	// Validate fragmentation fields before allocating a reassembly buffer
+	if fragCount == 0 || fragCount > maxReassemblyFragments || fragIndex >= fragCount {
+		core.Log.Warn(l, "Invalid fragmentation fields - DROP",
+			"index", fragIndex, "count", fragCount, "base", baseSequence)
+		return nil
+	}
+
 	var buffer enc.Wire = nil
 	var bufIndex int = 0
 
diff --git a/fw/face/ndnlp_link_service_test.go b/fw/face/ndnlp_link_service_test.go
new file mode 100644
index 00000000..adf09b5d
--- /dev/null
+++ b/fw/face/ndnlp_link_service_test.go
@@ -0,0 +1,67 @@
+package face
+
+import (
+	"testing"
+
+	"github.com/named-data/ndnd/fw/defn"
+	enc "github.com/named-data/ndnd/std/encoding"
+	"github.com/named-data/ndnd/std/types/optional"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// A single fragment with an attacker-controlled FragCount must not cause
+// an unbounded reassembly buffer allocation.
+func TestReassembleFragCountBound(t *testing.T) {
+	l := &NDNLPLinkService{}
+	frame := &defn.FwLpPacket{Fragment: enc.Wire{[]byte{0x01}}}
+
+	// 1<<22 fragment slots would allocate a ~96MB slice for one tiny fragment
+	frag := l.reassemble(frame, 1, 0, 1<<22)
+	assert.Nil(t, frag)
+	for i := range l.reassemblyBuffers {
+		assert.Nilf(t, l.reassemblyBuffers[i].buffer,
+			"buffer %d allocated %d fragment slots for an oversized FragCount",
+			i, len(l.reassemblyBuffers[i].buffer))
+	}
+}
+
+// A frame with FragIndex greater than its Sequence must be dropped before
+// baseSequence is computed (uint64 underflow) and no state may be allocated.
+func TestReassemblyFragIndexExceedsSequence(t *testing.T) {
+	l := &NDNLPLinkService{options: MakeNDNLPLinkServiceOptions()}
+	lp := &defn.FwLpPacket{
+		Fragment:  enc.Wire{[]byte{0x01}},
+		Sequence:  optional.Some(uint64(0)),
+		FragIndex: optional.Some(uint64(1)),
+		FragCount: optional.Some(uint64(2)),
+	}
+	pkt := defn.FwPacket{LpPacket: lp}
+	frameWire := pkt.Encode()
+	require.NotNil(t, frameWire)
+
+	l.handleIncomingFrame(frameWire.Join())
+	for i := range l.reassemblyBuffers {
+		assert.Nilf(t, l.reassemblyBuffers[i].buffer,
+			"buffer %d allocated (sequence %d) for an invalid FragIndex/Sequence pair",
+			i, l.reassemblyBuffers[i].sequence)
+	}
+}
+
+// A legitimate fragmented packet must still reassemble: fragments arrive in
+// order, the completed wire is returned, and the buffer is freed.
+func TestReassembleValidSequence(t *testing.T) {
+	l := &NDNLPLinkService{}
+	f0 := &defn.FwLpPacket{Fragment: enc.Wire{[]byte{0x01}}}
+	f1 := &defn.FwLpPacket{Fragment: enc.Wire{[]byte{0x02}}}
+
+	assert.Nil(t, l.reassemble(f0, 100, 0, 2)) // incomplete
+	full := l.reassemble(f1, 100, 1, 2)
+	require.NotNil(t, full)
+	assert.Equal(t, enc.Wire{[]byte{0x01}, []byte{0x02}}, full)
+
+	// buffer freed after completion
+	for i := range l.reassemblyBuffers {
+		assert.Nil(t, l.reassemblyBuffers[i].buffer)
+	}
+}