From cd96968aee63ca02bb88682766113e206f9b9bc9 Mon Sep 17 00:00:00 2001 From: Guillaume GONNET Date: Tue, 27 Oct 2026 14:59:24 +0100 Subject: [PATCH] Fix IP defragmentation regression introduced in commit 219f2febd The cleanup commit 219f2febd broke IP defragmentation functions in the following ways: - Disordered fragments cause failure when last fragment was not the last packet in the list. - Bad fragmented packets were not returned when last fragment packet was not present. - "badfrag" was a list of tuple, instead of a list of packets AI-Assisted: no --- scapy/layers/inet.py | 41 ++++++++++++++++++++++++-------------- test/scapy/layers/inet.uts | 19 ++++++++++++++++++ 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/scapy/layers/inet.py b/scapy/layers/inet.py index 9691a2ff9a7..3e13c1d2161 100644 --- a/scapy/layers/inet.py +++ b/scapy/layers/inet.py @@ -1444,6 +1444,10 @@ def overlap_frag(p, overlap, fragsize=8, overlap_fragsize=None): return qfrag + fragment(p, fragsize) +class MissingFragments(Exception): + pass + + class BadFragments(ValueError): def __init__(self, *args, **kwargs): self.frags = kwargs.pop("frags", None) @@ -1455,13 +1459,11 @@ def _defrag_iter_and_check_offsets(frags): Internal generator used in _defrag_ip_pkt """ offset = 0 - for pkt, o, length in frags: - if offset != o: - if offset > o: - op = ">" - else: - op = "<" - warning("Fragment overlap (%i %s %i) on %r" % (offset, op, o, pkt)) + for pkt, o, length, mf in frags: + if offset < o: + raise MissingFragments + elif offset > o: + warning("Fragment overlap (%i > %i) on %r" % (offset, o, pkt)) raise BadFragments offset += length yield bytes(pkt[IP].payload) @@ -1483,17 +1485,20 @@ def _defrag_ip_pkt(pkt, frags): fraglen = len(ip.payload) else: fraglen = ip.len - (ip.ihl << 2) - # (pkt, frag offset, frag len) - frags[uid].append((pkt, ip.frag << 3, fraglen)) - if not ip.flags.MF: # no more fragments = last fragment - curfrags = sorted(frags[uid], key=lambda x: x[1]) # sort by offset + # (pkt, frag offset, frag len, LF) + curfrags = frags[uid] + lf = (curfrags and curfrags[-1][3]) or not ip.flags.MF + curfrags.append((pkt, ip.frag << 3, fraglen, not ip.flags.MF)) + if lf: # last fragment + curfrags.sort(key=lambda x: x[1]) # sort by offset try: data = b"".join(_defrag_iter_and_check_offsets(curfrags)) + except MissingFragments: + return True, None except ValueError: # bad fragment - badfrags = frags[uid] del frags[uid] - raise BadFragments(frags=badfrags) + raise BadFragments(frags=curfrags) # re-build initial packet without fragmentation p = curfrags[0][0].copy() pay_class = p[IP].payload.__class__ @@ -1533,9 +1538,9 @@ def _defrag_logic(plist, complete=False): ) except BadFragments as ex: if complete: - final.extend(ex.frags) + final.extend(t[0] for t in ex.frags) else: - badfrag.extend(ex.frags) + badfrag.extend(t[0] for t in ex.frags) continue if complete and defragmented_value: final.append(defragmented_value) @@ -1544,6 +1549,12 @@ def _defrag_logic(plist, complete=False): final.append(defragmented_value) else: notfrag.append(defragmented_value) + # Missing fragments + for lst in frags.values(): + if complete: + final.extend(t[0] for t in lst) + else: + badfrag.extend(t[0] for t in lst) # Return if complete: if hasattr(plist, "listname"): diff --git a/test/scapy/layers/inet.uts b/test/scapy/layers/inet.uts index 1032777c4ea..41ed1dc6ed0 100644 --- a/test/scapy/layers/inet.uts +++ b/test/scapy/layers/inet.uts @@ -274,6 +274,25 @@ assert defrags[0].time == last_time pkts = fragment(IP(dst="10.0.0.5")/ICMP()/("X"*1500)) assert len(defragment(pkts[1:])) == 1 += defrag() / defragment() - Disordered fragments + +from random import shuffle + +frags1 = fragment(IP(id=1) / ("A" * payloadlen), fragsize) +frags2 = fragment(IP(id=2) / ("B" * payloadlen), fragsize) +frags3 = fragment(IP(id=3) / ("C" * payloadlen), fragsize) + +allfrags = frags1 + frags2 + frags3[:3] +shuffle(allfrags) + +nonfrag, unfrag, badfrag = defrag(allfrags) +assert len(nonfrag) == 0 +assert len(unfrag) == 2 +assert len(badfrag) == 3 + +defrags = defragment(allfrags) +assert len(defrags) == 5 + = defrag() / defragment() - Real DNS packets import base64