Repository: secdev/scapy
Pull request: Fix FCSField
- fixes https://github.com/secdev/scapy/issues/1969

Problem statement:
bytes(packet) modifies packet
#### Brief description
Running `bytes(packet)` causes `packet` to be extended (by padding?) each time it's called.

#### Environment

-   Scapy version: `scapy===git-archive.dev5c9a7789d`
-   Python version: `3.6.7`
-   Operating System: `Ubuntu Bionic`

#### How to reproduce
Simply capture some packets (in this case, i've seen with dot11 and dns, but likely affects them all), and simply call `bytes(packet)` multiple times.

#### Actual result
```
In [2]: bytes(pkt)                                                                                                                                                                                
Out[2]: b'\x00\x00&\x00/@\x00\xa0 \x08\x00\xa0 \x08\x00\x00\xe2\x07\xec\x88\x00\x00\x00\x00\x10\x02\x99\t\xa0\x00\xbe\x00\x00\x00\xbe\x00\xb5\x01\xd4\x00\x00\x00\xe4\x95nF-\xf2,\x0e4\xfd'

In [3]: bytes(pkt)                                                                                                                                                                                
Out[3]: b'\x00\x00&\x00/@\x00\xa0 \x08\x00\xa0 \x08\x00\x00\xe2\x07\xec\x88\x00\x00\x00\x00\x10\x02\x99\t\xa0\x00\xbe\x00\x00\x00\xbe\x00\xb5\x01\xd4\x00\x00\x00\xe4\x95nF-\xf2,\x0e4\xfd,\x0e4\xfd'

In [4]: bytes(pkt)                                                                                                                                                                                
Out[4]: b'\x00\x00&\x00/@\x00\xa0 \x08\x00\xa0 \x08\x00\x00\xe2\x07\xec\x88\x00\x00\x00\x00\x10\x02\x99\t\xa0\x00\xbe\x00\x00\x00\xbe\x00\xb5\x01\xd4\x00\x00\x00\xe4\x95nF-\xf2,\x0e4\xfd,\x0e4\xfd,\x0e4\xfd'

In [5]: bytes(pkt)                                                                                                                                                                                
Out[5]: b'\x00\x00&\x00/@\x00\xa0 \x08\x00\xa0 \x08\x00\x00\xe2\x07\xec\x88\x00\x00\x00\x00\x10\x02\x99\t\xa0\x00\xbe\x00\x00\x00\xbe\x00\xb5\x01\xd4\x00\x00\x00\xe4\x95nF-\xf2,\x0e4\xfd,\x0e4\xfd,\x0e4\xfd,\x0e4\xfd'

In [6]: bytes(pkt)                                                                                                                                                                                
Out[6]: b'\x00\x00&\x00/@\x00\xa0 \x08\x00\xa0 \x08\x00\x00\xe2\x07\xec\x88\x00\x00\x00\x00\x10\x02\x99\t\xa0\x00\xbe\x00\x00\x00\xbe\x00\xb5\x01\xd4\x00\x00\x00\xe4\x95nF-\xf2,\x0e4\xfd,\x0e4\xfd,\x0e4\xfd,\x0e4\xfd,\x0e4\xfd'
```

#### Expected result
I would expect to simply be provided the bytes for the given packet. I would expect that those bytes not change since i'm asking for the same packet over and over, and that packet has not changed.

#### Related resources

Hints from issue discussion:
Please share the original packet/way you built it/pcap file (or in last resort, `pkt.command()`) so that we can reproduce
```python
import scapy.all

sniff = scapy.all.sniff(offline="./dot11example.pcap")
bytes(sniff[0])
bytes(sniff[0])
bytes(sniff[0])
```
[example.zip](https://github.com/secdev/scapy/files/3035144/example.zip)

Commit message: Fix FCSField consistency & add tests

Patch to review:
diff --git a/scapy/fields.py b/scapy/fields.py
index b982d316124..78b0ff71660 100644
--- a/scapy/fields.py
+++ b/scapy/fields.py
@@ -412,7 +412,9 @@ def getfield(self, pkt, s):
         val = self.m2i(pkt, struct.unpack(self.fmt, s[-self.sz:])[0])
 
         def _post_dissect(self, s):
-            self.raw_packet_cache = None  # Reset packet to allow post_build
+            # Reset packet to allow post_build
+            self.raw_packet_cache = None
+            self.post_dissect = previous_post_dissect
             return previous_post_dissect(s)
         pkt.post_dissect = MethodType(_post_dissect, pkt)
         return s[:-self.sz], val
@@ -423,6 +425,7 @@ def addfield(self, pkt, s, val):
 
         def _post_build(self, p, pay):
             pay += value
+            self.post_build = previous_post_build
             return previous_post_build(p, pay)
         pkt.post_build = MethodType(_post_build, pkt)
         return s
diff --git a/test/fields.uts b/test/fields.uts
index c208e4dd23d..6620a9eb7b0 100644
--- a/test/fields.uts
+++ b/test/fields.uts
@@ -1684,3 +1684,29 @@ m = MSBExtendedField("foo", None)
 r = m.m2i(None, data_129["str_with_fx"])
 assert(r == data_129["extended"])
 
+
+############
+############
++ FCSField
+
+= FCSField: basic test
+
+class TestPacket(Packet):
+    fields_desc = [
+        ByteField("a", 0),
+        LEShortField("b", 15),
+        LEIntField("c", 7),
+        FCSField("fcs", None),
+        IntField("bottom", 0)
+    ]
+
+bind_layers(TestPacket, Ether)
+
+pkt = TestPacket(a=12, fcs=0xbeef, bottom=123)/Ether(src="aa:aa:aa:aa:aa:aa", dst="bb:bb:bb:bb:bb:bb")/IP(src="127.0.0.1", dst="127.0.0.1")
+
+assert raw(pkt) == b'\x0c\x0f\x00\x07\x00\x00\x00\x00\x00\x00{\xbb\xbb\xbb\xbb\xbb\xbb\xaa\xaa\xaa\xaa\xaa\xaa\x08\x00E\x00\x00\x14\x00\x01\x00\x00@\x00|\xe7\x7f\x00\x00\x01\x7f\x00\x00\x01\xbe\xef'
+# Test that it is consistent
+assert raw(pkt) == b'\x0c\x0f\x00\x07\x00\x00\x00\x00\x00\x00{\xbb\xbb\xbb\xbb\xbb\xbb\xaa\xaa\xaa\xaa\xaa\xaa\x08\x00E\x00\x00\x14\x00\x01\x00\x00@\x00|\xe7\x7f\x00\x00\x01\x7f\x00\x00\x01\xbe\xef'
+
+pkt = TestPacket(raw(pkt))
+assert pkt.fcs = 0xbeef


Code review comments:
diff_hunk: @@ -1684,3 +1684,29 @@ m = MSBExtendedField("foo", None)
 r = m.m2i(None, data_129["str_with_fx"])
 assert(r == data_129["extended"])
 
+
+############
+############
++ FCSField
+
+= FCSField: basic test
+
+class TestPacket(Packet):
+    fields_desc = [
+        ByteField("a", 0),
+        LEShortField("b", 15),
+        LEIntField("c", 7),
+        FCSField("fcs", None),
+        IntField("bottom", 0)
+    ]
+
+bind_layers(TestPacket, Ether)
+
+pkt = TestPacket(a=12, fcs=0xbeef, bottom=123)/Ether(src="aa:aa:aa:aa:aa:aa", dst="bb:bb:bb:bb:bb:bb")/IP(src="127.0.0.1", dst="127.0.0.1")
+
+assert raw(pkt) == b'\x0c\x0f\x00\x07\x00\x00\x00\x00\x00\x00{\xbb\xbb\xbb\xbb\xbb\xbb\xaa\xaa\xaa\xaa\xaa\xaa\x08\x00E\x00\x00\x14\x00\x01\x00\x00@\x00|\xe7\x7f\x00\x00\x01\x7f\x00\x00\x01\xbe\xef'
+# Test that it is consistent
+assert raw(pkt) == b'\x0c\x0f\x00\x07\x00\x00\x00\x00\x00\x00{\xbb\xbb\xbb\xbb\xbb\xbb\xaa\xaa\xaa\xaa\xaa\xaa\x08\x00E\x00\x00\x14\x00\x01\x00\x00@\x00|\xe7\x7f\x00\x00\x01\x7f\x00\x00\x01\xbe\xef'
+
+pkt = TestPacket(raw(pkt))
+assert pkt.fcs = 0xbeef
path: test/fields.uts
text: @user1:
```suggestion
assert pkt.fcs == 0xbeef
```