`raw_packet_cache_fields` slow down `do_dissect` significantly if ListPacketFields are relatively deeply nested

[evverx]

Brief description

If layers can contain nested ListPacketFields it can take a lot of time (and RAM) to dissect relatively short messages.

With Python3 it regressed in aabf4b56f01fa22e039f6cd69e8058e0648b38e0 in the sense that before that commit was merged scapy didn't actually traverse those lists all the way down so it was possible to dissect those layers somewhat linearly in terms of time and RAM.

Scapy version

e834fa305fba6700171483c51faca5a3163a2d41

Python version

3.10.9

Operating system

Linux version 6.1.9-100.fc36.x86_64

Additional environment information

No response

How to reproduce

class GuessPayload(Packet):
     @classmethod
     def dispatch_hook(cls, payload=None, *args, **kargs):
         return Test

class Test(Packet):
     fields_desc = [
         ByteField('b', 0),
         PacketListField('pl', [], GuessPayload)
     ]

Test(b'\x01' * 100)

Even though it's just an example this pattern (where deeply nested PacketListFields can be produced) is used in actual layers like DHCP6 for example.

Actual result

Depending on the machine where it's run it can either take forever to finish or get killed by the OOM killer eventually.

Expected result

No response

Related resources

No response

[guedou]

Thanks for the report. What will be you suggested fix?

[evverx]

I have to admit I'm not sure how to fix it properly. I think one option would probably be to "invalidate" raw_packet_cache_fields and avoid copying those deeply nested lists (for example with https://github.com/secdev/scapy/commit/aabf4b56f01fa22e039f6cd69e8058e0648b38e0 reverted it takes Test(b'\x01' * 100) 10-20 ms to finish on my machine). On the other hand I'm not sure whether it should be even possible to nest PacketListFields so deep. Currently they can be nested until RecursionError is caught by do_dissect_payload with the Raw layer at the bottom.

[evverx]

one option would probably be to "invalidate" raw_packet_cache_fields and avoid copying those deeply nested lists

I quickly put it together and it doesn't fix the issue in general because it just gets it around in one particular place. p.copy() still has to traverse all the lists. It seems the right fix would be to avoid this pattern by making sure that those lists are either flat or reasonably deep (depending on the layer).

[evverx]

Interestingly looks like v2.4.4 was affected by this issue as well. v2.4.5 was released with https://github.com/secdev/scapy/commit/f04b0ae1acf0cb72ce71103c52abbbc020405c68#diff-330de0527046a0fc8af1b24fbea040a50e66ca5fcf47b1c9296e48591b383074L1250 where PacketListField.do_copy (which actually traversed lists) was removed.

Anyway I haven't figured out how to limit the recursion yet.

[evverx]

One idea I had was to go over the "parent" chain and bail out early if this chain is deeper than a certain default value (that can probably be specified explicitly if layers need to nest more than, say, 2 lists). This way those deeply nested lists can't be produced in the first place and the existing layers shouldn't be edited. I'll try to figure out whether this approach actually leads anywhere in practice.

[evverx]

I think I got it to work

diff --git a/scapy/fields.py b/scapy/fields.py
index f820e183..0740c27a 100644
--- a/scapy/fields.py
+++ b/scapy/fields.py
@@ -1743,6 +1743,14 @@ class PacketListField(_PacketField[List[BasePacket]]):
         remain = s
         if len_pkt is not None:
             remain, ret = s[:len_pkt], s[len_pkt:]
+
+        current, depth = pkt, 0
+        while current:
+            depth += 1
+            if depth > 4:
+                return ret, [conf.raw_layer(load=remain)]
+            current = current.parent
+
         while remain:
             if c is not None:
                 if c <= 0:

I'll add some tests and open a PR. I'm not sure what it should default to but it seems there are no layers where the depth should be larger than 4.