search

kbandla / dpkt

fast, simple packet creation / parsing, with definitions for the basic TCP/IP protocols
Other
1.08k stars 270 forks source link

IP6.data fragments use type dpkt.udp.UDP when it should be bytes #575

Closed mwander closed 3 years ago

mwander commented 3 years ago

Transport data of IPv4 fragments is set to type(dpkt.udp.UDP) only on the fragment with offset=0. Any other fragment uses type(bytes). I'd expect that IPv6 fragments are handled similarly.

However, data of IPv6 fragments use type(dpkt.udp.UDP) on all UDP fragments, irregardless of offset. This allows to retrieve sport, dport, ulen, sum from fragments, but the data will be arbitrary garbage, except for the first fragment with frag_off=0.

Suggestion: harmonize behavior of IP/IP6 and use type(bytes) for IPv6 data fragments with frag_off > 0.

Attached .py and .pcap reproduce the issue: dpkt-fragment-issue.zip

obormot commented 3 years ago

Thanks for the detailed bug report! Looking into it

obormot commented 3 years ago

Addressed in PR https://github.com/kbandla/dpkt/pull/576 Output of the test snippet after the fix:

$ python dpkt-fragment-issue.py ./frag-ipv4-ipv6.pcap
Frame 1 <class 'dpkt.ip6.IP6'> <class 'dpkt.udp.UDP'> 44 bytes
Frame 2 <class 'dpkt.ip6.IP6'> <class 'dpkt.udp.UDP'> 1232 bytes
Frame 3 <class 'dpkt.ip6.IP6'> <class 'bytes'> 1232 bytes
Frame 4 <class 'dpkt.ip6.IP6'> <class 'bytes'> 300 bytes
Frame 5 <class 'dpkt.ip6.IP6'> <class 'bytes'> 1232 bytes
Frame 6 <class 'dpkt.ip.IP'> <class 'dpkt.udp.UDP'> 44 bytes
Frame 7 <class 'dpkt.ip.IP'> <class 'dpkt.udp.UDP'> 1480 bytes
Frame 8 <class 'dpkt.ip.IP'> <class 'bytes'> 1480 bytes
Frame 9 <class 'dpkt.ip.IP'> <class 'bytes'> 1036 bytes
mwander commented 3 years ago

Thanks for the quick fix. There is a regression issue in IP6.__bytes__() when writing packets. Stacktrace:

  File "anonymizepcap.py", line 148, in main
    pcap_writer.writepkt(frame, ts)
  File "/cygdrive/c/Users/mw/Documents/svn/msmt/anonymizepcap/dpkt/pcap.py", line 229, in writepkt
    self.writepkt_time(bytes(pkt), ts)
  File "/cygdrive/c/Users/mw/Documents/svn/msmt/anonymizepcap/dpkt/ethernet.py", line 268, in __bytes__
    return bytes(dpkt.Packet.__bytes__(self) + tail)
  File "/cygdrive/c/Users/mw/Documents/svn/msmt/anonymizepcap/dpkt/dpkt.py", line 149, in __bytes__
    return self.pack_hdr() + bytes(self.data)
  File "/cygdrive/c/Users/mw/Documents/svn/msmt/anonymizepcap/dpkt/ip6.py", line 125, in __bytes__
    if (self.p == 6 or self.p == 17 or self.p == 58) and not self.data.sum:
AttributeError: 'bytes' object has no attribute 'sum'

Suggestion for patch follows: issue575_regression.txt

obormot commented 3 years ago

Regression addressed in PR https://github.com/kbandla/dpkt/pull/577

obormot commented 3 years ago

@mwander is the latest build working well for you?

mwander commented 3 years ago

Yes, can confirm. Thanks a lot!