Skip to content

engines(pypcapfile): _decode() feeds hexlified frame bytes straight to pypcapfile's decoders #746

Description

@JarryShaw

pcapkit/foundation/engines/pypcapfile.py loads each savefile packet with layers=0:

sfile = pcapfile.savefile.load_savefile(
    _NamedStream(ext._ifile, ext._ifnm), layers=0, lazy=True,
)

and later decodes it in _decode():

decoded = self._declf(packet.packet, layers=self.LAYERS - 1)   # Ethernet(packet.packet, layers=1)

pcapfile.savefile._read_a_packet (pypcapfile 0.12.0) is not "give me the raw frame" when layers=0 — it is "hexlify the whole frame and stop":

if layers > 0:
    layers -= 1
    raw_packet = linklayer.clookup(hdrp[0].ll_type)(raw_packet_data, layers=layers)
else:
    raw_packet = binascii.hexlify(raw_packet_data)

So packet.packet handed to _decode() is already binascii.hexlify(raw_packet_data) — hex-ASCII text, not the raw frame. _decode() passes it straight to Ethernet(packet.packet, layers=1) without un-hexlifying first, so Ethernet.__init__ unpacks its 14-byte header from hex-characters, not raw bytes. The result decodes without raising (so no AttributeWarning fires and nothing looks wrong), but every field is garbage — reproduced on examples/captures/ipv4.pcap:

ethertype: 0x3030            # should be 0x0800
dst: <double-hex-encoded garbage>   # should be 01:00:5e:01:03:03

Confirmed the fix: Ethernet(binascii.unhexlify(packet.packet), layers=1) decodes correctly (ethertype=0x800, dst=b'01:00:5e:01:03:03', nested IP object with the right src/dst/flags).

Impact

Every real-capture (non-synthetic) use of engine='pypcapfile' gets systematically wrong Ethernet/IP/TCP field values — not just addresses. This is separate from #743 (which is about pcapkit/toolkit/pypcapfile.py reading IP.src/.dst as packed bytes instead of dotted-decimal ASCII): #743's fix corrects the toolkit-level address/opt/payload parsing and is confirmed against pypcapfile's real decoders called directly (bypassing this engine), but does not touch this file and therefore cannot fix frames that go through the engine's run()/_decode().

Concretely, of the 6 HAS_PYPCAPFILE-gated tests in tests/foundation/engines/test_new_engine_parity_runtime.py, 3 remain red after #743's fix, for this reason and not for #743's:

  • test_pypcapfile_agrees_with_the_default_engine (fails at the Ethernet layer, before any IPv4 parsing)
  • test_pypcapfile_ipv4_reassembly_agrees_with_the_default_engine (no IP layer is ever found, since the ethertype is garbage, so ipv4.reassembly stays empty)
  • test_pypcapfile_traces_only_the_ipv4_half_of_a_mixed_capture (same cause, empty trace)

Suggested fix

In _decode() (pcapkit/foundation/engines/pypcapfile.py:338), un-hexlify packet.packet before handing it to self._declf, e.g. self._declf(binascii.unhexlify(packet.packet), layers=self.LAYERS - 1).

Repro

from pcapkit.interface import extract
e = extract(fin='examples/captures/ipv4.pcap', fout='/tmp/out', format='tree',
            store=True, nofile=True, engine='pypcapfile', reassembly=True, ipv4=True)
print(hex(e.frame[0].packet.type))   # 0x3030, not 0x800
print(len(e.reassembly.ipv4))        # 0, though the default engine reassembles 4 fragments

Found while fixing #743 on Python 3.10 with pypcapfile 0.12.0 installed; out of scope for that PR (which owns pcapkit/toolkit/pypcapfile.py only, not this engine file).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugIssues reporting a defect (set by the bug report template; a default, not an assessment)

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions