Skip to content

face: validate NDNLPv2 fragmentation fields before reassembly - #216

Open
Harsh23Kashyap wants to merge 1 commit into
named-data:mainfrom
Harsh23Kashyap:fix/ndnlp-reassembly-fragcount
Open

Harsh23Kashyap wants to merge 1 commit into
named-data:mainfrom
Harsh23Kashyap:fix/ndnlp-reassembly-fragcount

Conversation

@Harsh23Kashyap

Copy link
Copy Markdown

Fixes #215

What

  • Drop NDNLPv2 frames with invalid fragmentation fields (FragCount == 0, FragCount > MaxNDNPacketSize, FragIndex >= FragCount, FragIndex > Sequence) in handleIncomingFrame before baseSequence is computed, and re-check the bounds in reassemble before allocating.
  • Regression tests: oversized FragCount no longer allocates; FragIndex > Sequence no longer underflows or allocates; a normal 2-fragment packet still reassembles and frees its buffer.

Evidence

Before (on main):

--- FAIL: TestReassembleFragCountBound (0.67s)   -> allocated 4194304 fragment slots for one fragment
--- FAIL: TestReassemblyFragIndexExceedsSequence (0.00s)   -> buffer at sequence 18446744073709551615

After (this branch):

--- PASS: TestReassembleFragCountBound (0.00s)
--- PASS: TestReassemblyFragIndexExceedsSequence (0.00s)
--- PASS: TestReassembleValidSequence (0.00s)
ok  	github.com/named-data/ndnd/fw/face

Full go test ./... green.

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 named-data#96
bounds the number of concurrent reassemblies but not their size.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fw/face: NDNLPv2 reassembly trusts attacker-controlled FragCount/FragIndex (unbounded allocation + sequence underflow)

1 participant