Fix nested ListFields

Author: gpotter2

scapy/packet.py

@@ -638,6 +638,22 @@ def copy_fields_dict(self, fields):
         return {fname: self.copy_field_value(fname, fval)
                 for fname, fval in six.iteritems(fields)}
 
+    def _raw_packet_cache_field_value(self, fld, val, copy=False):
+        # type: (AnyField, Any, bool) -> Optional[Any]
+        """Get a value representative of a mutable field to detect changes"""
+        _cpy = lambda x: fld.do_copy(x) if copy else x  # type: Callable[[Any], Any]
+        if fld.holds_packets:
+            # avoid copying whole packets (perf: #GH3894)
+            if fld.islist:
+                return [
+                    _cpy(x.fields) for x in val
+                ]
+            else:
+                return _cpy(val.fields)
+        elif fld.islist or fld.ismutable:
+            return _cpy(val)
+        return None
+
     def clear_cache(self):
         # type: () -> None
         """Clear the raw packet cache for the field and all its subfields"""
@@ -661,7 +677,8 @@ def self_build(self):
         """
         if self.raw_packet_cache is not None:
             for fname, fval in six.iteritems(self.raw_packet_cache_fields):
-                if self.getfieldval(fname) != fval:
+                fld, val = self.getfield_and_val(fname)
+                if self._raw_packet_cache_field_value(fld, val) != fval:
                     self.raw_packet_cache = None
                     self.raw_packet_cache_fields = None
                     self.wirelen = None
@@ -987,8 +1004,9 @@ def do_dissect(self, s):
                 continue
             # We need to track fields with mutable values to discard
             # .raw_packet_cache when needed.
-            if f.islist or f.holds_packets or f.ismutable:
-                self.raw_packet_cache_fields[f.name] = f.do_copy(fval)
+            if (f.islist or f.holds_packets or f.ismutable) and fval is not None:
+                self.raw_packet_cache_fields[f.name] = \
+                    self._raw_packet_cache_field_value(f, fval, copy=True)
             self.fields[f.name] = fval
         self.raw_packet_cache = _raw[:-len(s)] if s else _raw
         self.explicit = 1

test/fields.uts

@@ -758,6 +758,34 @@ assert p.data[3].data == 0xc102
 assert isinstance(p.payload, conf.raw_layer)
 assert p.payload.load == b'toto'
 
+= Test nested PacketListFields
+~ field
+# Note: having packets that look like this is a terrible idea, and will perform
+# very badly. However we must ensure we don't freeze because of it.
+
+# https://github.com/secdev/scapy/issues/3894
+
+class GuessPayload(Packet):
+     @classmethod
+     def dispatch_hook(cls, *args, **kargs):
+         return TestNestedPLF
+
+class TestNestedPLF(Packet):
+     fields_desc = [
+         ByteField('b', 0),
+         PacketListField('pl', [], GuessPayload)
+     ]
+
+p = TestNestedPLF(b'\x01' * 100)
+
+# check
+i = 1
+while p.pl:
+    p = p.pl[0]
+    p.show()
+    i += 1
+
+assert i == 100
 
 ############
 ############

test/scapy/layers/dhcp6.uts

@@ -1167,6 +1167,11 @@ raw(DHCP6OptRelaySuppliedOpt(relaysupplied=DHCP6OptERPDomain(erpdomain=["toto.ex
 a = DHCP6OptRelaySuppliedOpt(b'\x00B\x00\x16\x00A\x00\x12\x04toto\x07example\x03com\x00')
 a.optcode == 66 and a.optlen == 22 and len(a.relaysupplied) == 1 and isinstance(a.relaysupplied[0], DHCP6OptERPDomain) and a.relaysupplied[0].erpdomain[0] == "toto.example.com."
 
+= DHCP6OptRelaySuppliedOpt - deeply nested DHCP6OptRelaySuppliedOpt
+# https://github.com/secdev/scapy/issues/3894
+
+p = DHCP6(b'\x01\x00\x00\x00' + b'\x00B\x0f\x0f' * 100)
+assert p.getlayer(DHCP6OptRelaySuppliedOpt, 100)
 
 ############
 ############