Repository navigation
TCP reassembly silently resolves conflicting retransmissions last-write-wins and still reports COMPLETE #443
Description
When two segments claim the same sequence range but carry different bytes, TCP reassembly silently keeps the later one and still reports the datagram as complete. Nothing in the returned Datagram records that the two disagreed.
Reproduction, on c28ffc287
from pcapkit.foundation.reassembly.tcp import TCP
from pcapkit.foundation.reassembly.data.tcp import Packet
def seg(num, seq, payload, *, ack=1000):
return Packet(bufid=('192.0.2.1', 12345, '198.51.100.2', 443), dsn=seq, ack=ack,
num=num, syn=False, fin=False, rst=False, len=len(payload),
first=seq, last=seq + len(payload) - 1,
header=b'hdr', payload=bytearray(payload))
r = TCP(strict=True)
r(seg(1, 100, b'AAAAAAAA')) # original
r(seg(2, 100, b'BBBBBBBB')) # same range, different bytes
r(seg(3, 108, b'CCCC')) # stream continues
for d in r.fetch():
print(d.completed, bytes(d.payload), d.index)True b'BBBBBBBBCCCC' (1, 2, 3)
b'AAAAAAAA' is gone without a trace, and completed is True.
Mechanism
pcapkit/foundation/reassembly/tcp.py:166:
else: # if fragment partially overlaps existing payload
RAW[PSN - ISN:PSN - ISN + info.len] = info.payloadThe slice assignment is unconditional — the incoming payload is written over whatever occupied that range, with no comparison against the bytes already there. The hole-descriptor list is then updated from the segment's own bounds, so the overwritten range remains hole-free and completion is derived as COMPLETE. The mirrored branch at :176 (RAW = info.payload + RAW[-GAP:]) does the same for a segment that reaches back before the current ISN.
Why it is worth recording
A conforming stack retransmits identical bytes, so a disagreement means one of two things, and both matter to anyone using this for analysis:
- a broken or buggy sender, which the caller would want to know about;
- deliberately overlapping segments, which is the classic TCP-overlap IDS-evasion technique — the whole point of that attack is that different reassemblers resolve the conflict differently, so a tool that silently picks one and reports "complete" gives an answer that looks authoritative and isn't.
Last-write-wins is a defensible choice (it is roughly what Linux does), but making it silently and still claiming completeness is what turns it into a defect: there is no way for a caller to tell a clean stream from a contested one.
Possible directions
Not proposing a specific fix, since the right answer depends on whether the reassembler wants to grow a notion of "contested":
- compare before overwriting, and record the conflicting ranges on the
Datagram(a new field, so additive); - surface it as a
ProtocolWarningwhenstrict=True, leaving the resolution unchanged; - keep first-write-wins instead, which is what some stacks do, and document the choice either way.
Found while reviewing #435, which changes completed to a Completion enum but does not touch this path — git diff origin/main over reassembly/tcp.py shows no change to either overlap branch, and :166/:176 are byte-identical on main. Filed separately rather than folded into that PR.
Activity
traceflow: state what analyse=True actually returns, and pin it
traceflow: state what analyse=True actually returns, and pin it
JarryShaw commentedon Sep 18, 2026
Still reproduces on
fa128959e, but three details in the report above have gonestale and one new measurement bears directly on the design choice.
The reproduction needs updating.
Packetnow takes a requiredtimestampargument, so the snippet as written dies with
TypeError: Packet.__init__() missing 1 required positional argument: 'timestamp'.Adding
timestamp=0.0makes it run, and the behaviour is unchanged:The line numbers have moved. The unconditional slice assignment is now at
pcapkit/foundation/reassembly/tcp.py:194, and the mirrored reach-back branch at:204— not:166and:176.strictmakes no difference, which matters for one of the options on thetable. Measured both ways:
That is because
strictdoes not mean "be strict about anomalies" in thiscodebase. It is documented as "if return all datagrams (including those not
implemented)" and stored as
self._flag_sinpcapkit/foundation/reassembly/reassembly.py:424;tcp.pyrefers to it only incomments about whether an incomplete buffer is still reported. So "warn only
under
strict=True" would be repurposing an existing flag that currentlycontrols something unrelated — worth knowing before choosing it, since it would
change the meaning of a public keyword rather than add a new one.
Nothing else here is a new finding; this comment is only to keep the issue
accurate for whoever picks it up.
JarryShaw commentedon Sep 18, 2026
Design decision recorded, and the resolution question is answered by the spec rather than by preference. Two parts.
1. Surfacing: record the conflict on the
DatagramChosen by the repo owner. Compare before overwriting, and add a field to
Datagramnaming the conflicting sequence ranges. Additive, so existing callers are unaffected, and it is the only option that lets a caller ask a datagram after the fact whether it was contested.The
strict=Truegate is off the table, measured:strictmeans "if return all datagrams (including those not implemented)" —self._flag_satpcapkit/foundation/reassembly/reassembly.py:424— and makes no difference to this path either way. Gating on it would repurpose a public keyword that controls something unrelated.There is direct precedent for surfacing rather than silently resolving: Zeek raises a
rexmit_inconsistencyevent, documented as "Generated when Zeek detects a TCP retransmission inconsistency." So a mainstream analysis tool treats a conflicting retransmission as a reportable event in its own right.2. Resolution: RFC 9293 specifies first-write-wins, so this becomes a conformance fix
RFC 9293 §3.10 (which obsoletes RFC 793) is explicit:
"Just the new data" means the already-received bytes are kept and the overlapping portion of the arriving segment is discarded — first-write-wins. §3.10.7.4 points the same way: trim off any portion outside the window and process further only if the segment then begins at
RCV.NXT, i.e. the duplicate prefix is trimmed from the incoming segment, never written over what is already buffered.pcapkit/foundation/reassembly/tcp.py:194does the opposite. The unconditionalRAW[PSN - ISN:PSN - ISN + info.len] = info.payloadoverwrites already-buffered bytes with the arriving ones, and:204mirrors it for the reach-back case. So the current behaviour is not RFC-conformant, which changes this from "a defensible choice made silently" into a straightforward conformance defect.Correcting my own framing in the issue body
The body above says "Last-write-wins is a defensible choice (it is roughly what Linux does)". I never verified the Linux claim and it should not have been stated. Worse, it pointed a reader toward keeping the current behaviour as the sensible default when the spec says the opposite. Retracted. What I have actually verified is the RFC 9293 text quoted above and the existence of Zeek's event; I have not checked any endpoint stack's source, nor Snort's or Wireshark's reassembly policy, so nothing here rests on those.
What this implies, including the cost
Switching to first-write-wins changes reassembled payload bytes for any capture containing a conflicting overlap — the reproduction in the body would yield
b'AAAAAAAACCCC'instead ofb'BBBBBBBBCCCC'. That is a behaviour change, not an additive one, and it is worth being explicit about rather than burying: it is justified because the current output is non-conformant, and because a capture that hits this path is by definition anomalous. A conforming sender retransmits identical bytes, in which case first and last are the same and nothing changes at all.So the shape of the fix is: compare before writing; keep the already-buffered bytes; discard the conflicting portion of the arriving segment; record the conflicting range on the
Datagram; leavecompletedalone, since with the range recorded a caller can now tell a clean stream from a contested one.reassembly: track a per-fragment received mask, not the shared hole l…
docs, style: unwedge the prose from the ASCII block and align three c…
reassembly: record IP fragment overlap conflicts instead of losing th…
reassembly: resolve conflicting TCP overlaps first-write-wins, per RF…
tests: pin that a TCP conflict in one ACK bucket stays out of the others
release: bump to 1.5.0b1, the wave-1 beta (#488)