Skip to content

reassembly: record IP fragment overlap conflicts instead of losing them silently #482

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

Merged
JarryShaw merged 2 commits into from
Sep 18, 2026
Merged

reassembly: record IP fragment overlap conflicts instead of losing them silently #482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6fbfb41
reassembly: record IP fragment overlap conflicts instead of losing th…
JarryShaw Sep 18, 2026
f4ae170
Merge branch 'main' into fix-477-ip-fragment-overlap-conflict
JarryShaw Sep 18, 2026
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
38 changes: 37 additions & 1 deletion docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ Terminology
| |--> 'header' : (bytes) IPv4 header
| |--> 'payload' : (bytes) reassembled IPv4 payload
| |--> 'packet' : (Protocol) parsed reassembled payload
| |--> 'conflict' : (tuple) octet ranges on which two fragments disagreed
| | |--> (tuple) (first, last), absolute and inclusive
| | |--> ...
|--> (Info) data
| |--> 'completed' : (Completion) PARTIAL or TIMEOUT --> incomplete
| |--> 'id' : (Info) original packet identifier
Expand All @@ -82,6 +85,9 @@ Terminology
| | |--> (bytes) IPv4 payload fragment
| | |--> ...
| |--> 'packet' : (None)
| |--> 'conflict' : (tuple) octet ranges on which two fragments disagreed
| | |--> (tuple) (first, last), absolute and inclusive
| | |--> ...
|--> (Info) data ...

.. note::
Expand All @@ -95,6 +101,18 @@ Terminology
``repr()``), runs it and keeps the result; see
:class:`~pcapkit.foundation.reassembly.data.data.Deferred`.

.. note::

``completed`` and ``conflict`` are independent signals: a datagram
can be :attr:`~pcapkit.foundation.reassembly.data.data.Completion.COMPLETE`
and still carry a non-empty ``conflict``. :rfc:`791` resolves an
overlapping fragment's disagreement itself -- "this procedure will
use the more recently arrived copy in the data buffer" -- the
opposite resolution from TCP's first-write-wins
(:rfc:`9293#section-3.10`, fixed for TCP by #443) -- so a contested
range never leaves a hole on its own, and ``conflict`` is what lets
a caller tell a clean datagram from a contested one. See #477.

reasm.ipv4.buffer
Data structure for internal buffering when performing reassembly algorithms
(:attr:`IPv4._buffer <pcapkit.foundation.reassembly.reassembly.Reassembly._buffer>`)
Expand All @@ -118,9 +136,27 @@ Terminology
| |--> 'header' : (bytes) header buffer
| |--> 'datagram' : (bytearray) data buffer, holes set to b'\\x00'
| |--> 'timestamp' : (float) capture timestamp of the
| first-arriving fragment
| | first-arriving fragment
| |--> 'conflict' : (list) octet ranges on which an arriving
| | fragment disagreed with bytes already in
| | 'datagram'
| | |--> (tuple) (first, last), absolute and
| | inclusive
| | |--> ...
|--> (tuple) BUFID ...

.. note::

``conflict`` is only ever appended to, and is checked against
``datagram`` and ``RCVBT`` *before* an arriving fragment's own write
and bookkeeping touch them -- see
:meth:`IP._detect_conflicts <pcapkit.foundation.reassembly.ip.IP._detect_conflicts>`.
``RCVBT`` records receipt in 8-octet blocks, which is coarser than
the octet a conflict needs; ``TDL`` is what recovers the exact
extent for the one block that can be partially real -- the final
fragment's own tail -- so a conflict here is never wider than the
octets that genuinely disagreed, even inside that block.

.. note::

A buffer is abandoned once the reassembly timeout elapses on the
Expand Down
36 changes: 36 additions & 0 deletions docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ Terminology
| |--> 'header' : (bytes) header before IPv6-Frag
| |--> 'payload' : (bytes) reassembled IPv6 payload
| |--> 'packet' : (Protocol) parsed reassembled payload
| |--> 'conflict' : (tuple) octet ranges on which two fragments disagreed
| | |--> (tuple) (first, last), absolute and inclusive
| | |--> ...
|--> (Info) data
| |--> 'completed' : (Completion) PARTIAL or TIMEOUT --> incomplete
| |--> 'id' : (Info) original packet identifier
Expand All @@ -104,6 +107,9 @@ Terminology
| | |--> (bytes) IPv6 payload fragment
| | |--> ...
| |--> 'packet' : (None)
| |--> 'conflict' : (tuple) octet ranges on which two fragments disagreed
| | |--> (tuple) (first, last), absolute and inclusive
| | |--> ...
|--> (Info) data ...

.. note::
Expand All @@ -128,6 +134,18 @@ Terminology
``repr()``), runs it and keeps the result; see
:class:`~pcapkit.foundation.reassembly.data.data.Deferred`.

.. note::

``completed`` and ``conflict`` are independent signals: a datagram
can be :attr:`~pcapkit.foundation.reassembly.data.data.Completion.COMPLETE`
and still carry a non-empty ``conflict``. :rfc:`791` resolves an
overlapping fragment's disagreement itself -- "this procedure will
use the more recently arrived copy in the data buffer" -- the
opposite resolution from TCP's first-write-wins
(:rfc:`9293#section-3.10`, fixed for TCP by #443) -- so a contested
range never leaves a hole on its own, and ``conflict`` is what lets
a caller tell a clean datagram from a contested one. See #477.

reasm.ipv6.buffer
Data structure for internal buffering when performing reassembly algorithms
(:attr:`IPv6._buffer <pcapkit.foundation.reassembly.reassembly.Reassembly._buffer>`)
Expand All @@ -150,4 +168,22 @@ Terminology
| | |--> (int) packet range number
| |--> 'header' : (bytes) header buffer
| |--> 'datagram' : (bytearray) data buffer, holes set to b'\\x00'
| |--> 'conflict' : (list) octet ranges on which an arriving
| | fragment disagreed with bytes already in
| | 'datagram'
| | |--> (tuple) (first, last), absolute and
| | inclusive
| | |--> ...
|--> (tuple) BUFID ...

.. note::

``conflict`` is only ever appended to, and is checked against
``datagram`` and ``RCVBT`` *before* an arriving fragment's own write
and bookkeeping touch them -- see
:meth:`IP._detect_conflicts <pcapkit.foundation.reassembly.ip.IP._detect_conflicts>`.
``RCVBT`` records receipt in 8-octet blocks, which is coarser than
the octet a conflict needs; ``TDL`` is what recovers the exact
extent for the one block that can be partially real -- the final
fragment's own tail -- so a conflict here is never wider than the
octets that genuinely disagreed, even inside that block.
39 changes: 37 additions & 2 deletions pcapkit/foundation/reassembly/data/ip.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,34 @@ class Datagram(DeferredPacket, Info, Generic[_AT]):
#: :class:`Deferred` may be passed in its place, and reading this attribute
#: then runs it and keeps the result.
packet: 'Optional[Protocol]'
#: Octet ranges, absolute into the reassembled payload and both
#: **inclusive** -- the same convention as :attr:`Packet.fo` combined with
#: its length -- on which two fragments disagreed, i.e. an arriving
#: fragment overlapped octets already buffered but did not repeat them.
#: Empty when the datagram never saw a contested octet.
#:
#: :rfc:`791` resolves the disagreement itself: "In the case that two or
#: more fragments contain the same data either identically or through a
#: partial overlap, this procedure will use the more recently arrived copy
#: in the data buffer and datagram delivered." So :attr:`payload` always
#: holds whichever fragment arrived *last* over a contested range -- the
#: opposite resolution from TCP's first-write-wins
#: (:rfc:`9293#section-3.10`) -- and this field is what lets a caller tell
#: a clean datagram from a contested one, since a resolved conflict does
#: not, on its own, leave a hole for ``completed`` to report.
#:
#: :attr:`Buffer.RCVBT <pcapkit.foundation.reassembly.data.ip.Buffer.RCVBT>`
#: only records receipt in 8-octet blocks, coarser than the octet
#: granularity a conflict needs. Every fragment but the last is required
#: to be block-aligned (and :attr:`Packet.fo` is *always* a multiple of 8,
#: being wire-encoded in 8-octet units), so the only block that can be
#: partially real is the one holding the final fragment's own tail -- and
#: a range reported here never extends past
#: :attr:`Buffer.TDL <pcapkit.foundation.reassembly.data.ip.Buffer.TDL>`
#: for exactly that reason, even though a whole ``RCVBT`` block straddling
#: it reads as "received". See
#: :meth:`IP._detect_conflicts <pcapkit.foundation.reassembly.ip.IP._detect_conflicts>`.
conflict: 'tuple[tuple[int, int], ...]'

if TYPE_CHECKING:
# NOTE: one signature, not a pair of ``@overload``\\ s keyed on
Expand All @@ -118,7 +146,7 @@ class Datagram(DeferredPacket, Info, Generic[_AT]):
# from. Overloads keyed on a literal cannot be selected from a ``completed``
# computed at runtime anyway, so they only made the reassemblers' own calls
# untypeable while promising a correlation the code does not keep.
def __init__(self, completed: 'Completion', id: 'DatagramID[_AT]', index: 'tuple[int, ...]', header: 'bytes', payload: 'bytes | tuple[bytes, ...]', packet: 'Optional[Protocol | Deferred]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin
def __init__(self, completed: 'Completion', id: 'DatagramID[_AT]', index: 'tuple[int, ...]', header: 'bytes', payload: 'bytes | tuple[bytes, ...]', packet: 'Optional[Protocol | Deferred]', conflict: 'tuple[tuple[int, int], ...]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin

@info_final
class Buffer(Info, Generic[_AT]):
Expand All @@ -141,6 +169,13 @@ class Buffer(Info, Generic[_AT]):
#: first-arriving fragment", so a later fragment does not extend the
#: deadline and this field is never revised once set.
timestamp: 'float'
#: Octet ranges, absolute into :attr:`datagram` and both **inclusive**, on
#: which an arriving fragment disagreed with bytes already placed there by
#: an earlier one. Accumulated across every fragment merged into this
#: buffer, in the order the conflicts were found; carried onto
#: :attr:`Datagram.conflict <pcapkit.foundation.reassembly.data.ip.Datagram.conflict>`
#: verbatim when the buffer is submitted.
conflict: 'list[tuple[int, int]]'

if TYPE_CHECKING:
def __init__(self, TDL: 'int', RCVBT: 'bytearray', index: 'list[int]', header: 'bytes', datagram: 'bytearray', timestamp: 'float') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin
def __init__(self, TDL: 'int', RCVBT: 'bytearray', index: 'list[int]', header: 'bytes', datagram: 'bytearray', timestamp: 'float', conflict: 'list[tuple[int, int]]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin
101 changes: 96 additions & 5 deletions pcapkit/foundation/reassembly/ip.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,39 +118,127 @@ def reassembly(self, info: 'Packet[_AT]') -> 'None':
header=header, # header buffer
datagram=bytearray(65535), # data buffer
timestamp=TS, # first-arriving fragment's clock reading
conflict=[], # conflicting octet ranges
)
else:
# put header into header buffer
if not FO: # pylint: disable=else-if-used
self._buffer[BUFID].__update__(header=header)

buf = self._buffer[BUFID]

# append packet index
self._buffer[BUFID].index.append(info.num)
buf.index.append(info.num)

# put data into data buffer
start = FO
stop = TL - IHL + FO
self._buffer[BUFID].datagram[start:stop] = info.payload

# Find where this fragment disagrees with what the buffer already
# holds *before* writing it -- ``buf.RCVBT`` and ``buf.TDL`` still
# describe the state as every earlier fragment left it, which is
# exactly what :meth:`_detect_conflicts` needs.
conflicts = self._detect_conflicts(buf.RCVBT, buf.TDL, buf.datagram, info.payload, start, stop)
if conflicts:
buf.conflict.extend(conflicts)

# :rfc:`791` is explicit that an overlapping fragment's data "will use
# the more recently arrived copy in the data buffer" -- the opposite of
# TCP's first-write-wins (:rfc:`9293#section-3.10`) -- so the arriving
# payload always overwrites here; ``conflicts`` above is what records
# that it *disagreed* with what it overwrote, which is the part RFC 791
# leaves unrecorded and this fix adds.
buf.datagram[start:stop] = info.payload

# set RCVBT bits (in 8 octets)
start = FO // 8
stop = FO // 8 + (TL - IHL + 7) // 8
self._buffer[BUFID].RCVBT[start:stop] = b'\x01' * (stop - start)
buf.RCVBT[start:stop] = b'\x01' * (stop - start)

# get total data length (header excludes)
TDL = 0
if not MF:
TDL = TL - IHL + FO
self._buffer[BUFID].__update__(TDL=TDL)
buf.__update__(TDL=TDL)

# when datagram is reassembled in whole
start = 0
stop = (TDL + 7) // 8
if TDL and all(self._buffer[BUFID].RCVBT[start:stop]):
if TDL and all(buf.RCVBT[start:stop]):
self._dtgram.extend(
self.submit(self._buffer.pop(BUFID), bufid=BUFID, checked=True)
)

@staticmethod
def _detect_conflicts(rcvbt: 'bytearray', tdl: 'int', datagram: 'bytearray', payload: 'bytearray',
start: 'int', stop: 'int') -> 'list[tuple[int, int]]':
"""Find where an arriving fragment disagrees with already-received bytes.

Arguments:
rcvbt: this buffer's :attr:`~pcapkit.foundation.reassembly.data.ip.Buffer.RCVBT`
as it stood *before* the arriving fragment's own bits are set,
i.e. what earlier fragments had already claimed, in 8-octet
blocks.
tdl: this buffer's :attr:`~pcapkit.foundation.reassembly.data.ip.Buffer.TDL`
as it stood *before* the arriving fragment's own update --
``-1`` while the final fragment (``MF=0``) has not yet
arrived.
datagram: this buffer's data buffer, read *before* the arriving
fragment's payload is written into it.
payload: the arriving fragment's payload.
start: absolute octet offset of ``payload[0]`` in ``datagram``,
i.e. this fragment's ``FO``.
stop: absolute octet offset one past ``payload[-1]``, i.e.
``start + len(payload)``.

Returns:
``(first, last)`` absolute octet ranges, inclusive, where
``datagram`` and ``payload`` disagree over octets this buffer had
already genuinely received.

:rfc:`791` marks receipt in 8-octet blocks (``RCVBT``), coarser than
the octet granularity a conflict needs: every fragment but the last is
required to be a multiple of 8 octets, and a fragment's ``FO`` is
*always* a multiple of 8 -- it is wire-encoded in 8-octet units -- so a
non-final fragment's range is always exactly block-aligned. The only
block that can be *partially* real is therefore the one holding the
final fragment's own tail: the ``RCVBT`` update in :meth:`reassembly`
sets that block's bit across its full 8 octets even though only the
octets up to ``tdl`` were ever actually written, the rest still being
``datagram``'s zero-fill.

So once ``tdl`` is known, an octet at or past it is excluded here
regardless of its block's bit -- comparing it would manufacture a
conflict against a byte nothing ever really sent, over a distinction
:meth:`~pcapkit.foundation.reassembly.ip.IP.submit` does not need
anyway, since it never reports a payload past ``tdl``. Before ``tdl``
is known (``tdl < 0``), every set ``rcvbt`` bit came from a non-final,
block-aligned fragment and is exact on its own, with nothing to clip.

"""
conflicts = [] # type: list[tuple[int, int]]
length = stop - start
index = 0
while index < length:
pos = start + index
if not (rcvbt[pos // 8] and (tdl < 0 or pos < tdl)):
index += 1
continue
if datagram[pos] == payload[index]:
index += 1
continue
run_stop = index + 1
while run_stop < length:
pos = start + run_stop
if not (rcvbt[pos // 8] and (tdl < 0 or pos < tdl)):
break
if datagram[pos] == payload[run_stop]:
break
run_stop += 1
conflicts.append((start + index, start + run_stop - 1))
index = run_stop
return conflicts

def submit(self, buf: 'Buffer[_AT]', *, bufid: 'tuple[_AT, _AT, int, TransType]', # type: ignore[override] # pylint: disable=arguments-differ
checked: 'bool' = False, timeout: 'bool' = False) -> 'list[Datagram[_AT]]':
"""Submit reassembled payload.
Expand All @@ -176,6 +264,7 @@ def submit(self, buf: 'Buffer[_AT]', *, bufid: 'tuple[_AT, _AT, int, TransType]'
index = buf.index
header = buf.header
datagram = buf.datagram
conflict = tuple(buf.conflict)

start = 0
stop = (TDL + 7) // 8
Expand Down Expand Up @@ -216,6 +305,7 @@ def submit(self, buf: 'Buffer[_AT]', *, bufid: 'tuple[_AT, _AT, int, TransType]'
header=header,
payload=tuple(data),
packet=None,
conflict=conflict,
)
ret.append(packet)
# if datagram is reassembled in whole -- or if it is not, and ``strict``
Expand Down Expand Up @@ -275,6 +365,7 @@ def submit(self, buf: 'Buffer[_AT]', *, bufid: 'tuple[_AT, _AT, int, TransType]'
# result most of them never read. ``Deferred`` postpones it to the
# first read of ``Datagram.packet``.
packet=Deferred(self.protocol.analyze, bufid[3], payload),
conflict=conflict,
)
ret.append(packet)

Expand Down
Loading
Loading