PCAP-NG NRB options field reads past the end of the block

[JarryShaw]

What is wrong

A Name Resolution Block that carries any ns_* option is mis-parsed: the block's trailing Block Total Length is read from inside the option area, and the options themselves are read from past the end of the block.

NameResolutionBlock (pcapkit/protocols/schema/misc/pcapng.py:1168) is the only schema in the file with two consecutive OptionFields:

    #: Name resolution records.
    records: 'list[NameResolutionRecord]' = OptionField(
        length=lambda pkt: pkt['length'] - 12,                                       # line 1176
        ..., eool=Enum_RecordType.nrb_record_end,
    )
    #: Options.
    options: 'list[Option]' = OptionField(
        length=lambda pkt: pkt['__option_padding__'] - 4 if pkt['__option_padding__'] else 0,   # line 1184
        ..., eool=Enum_OptionType.opt_endofopt,
    )
    padding: 'bytes' = PaddingField(length=lambda pkt: pkt['__option_padding__'])
    length2: 'int' = UInt32Field(callback=byteorder_callback)

records is declared pkt['length'] - 12, which spans the record area and the option area — it has to, because the record area's size is only known once the records have been parsed. OptionField.unpack stops at nrb_record_end and reports the unconsumed remainder through __option_padding__ (pcapkit/corekit/fields/collections.py:281), which is how options is meant to learn its size.

But Schema.unpack reads each field's declared length off the file before handing it to the field:

byte = data.read(field.length)          # pcapkit/protocols/schema/schema.py:630

so the file has already advanced past the option area by the time options runs. options then reads a fresh __option_padding__ - 4 octets starting at the block's trailing Block Total Length. The - 4 is not a sizing error that a different constant would fix — the octets it is trying to size have already been consumed.

Symptom

On a 60-octet NRB with a 28-octet record area and a 20-octet option area (one ns_dnsname plus opt_endofopt):

[WARNING] PCAP-NG: [Block 4] block length mismatch: 60 != 314

(the generators' docstring records that figure; on the minimal fixture below it is 60 != 0). In that fixture the trailing length is read from block-relative octets 72–75 — the block is only 60 octets long, so those are the following Enhanced Packet Block's timestamp field, which is zero. An NRB with no ns_* options parses cleanly, because __option_padding__ is then 0 and the if pkt['__option_padding__'] else 0 branch reads nothing.

Reproduction

import struct, ipaddress

def pad4(b): return b + b'\x00' * ((4 - len(b) % 4) % 4)
def block(t, body):
    body = pad4(body); n = 12 + len(body)
    return struct.pack('<II', t, n) + body + struct.pack('<I', n)
def tlv(t, v): return struct.pack('<HH', t, len(v)) + pad4(v)

shb = lambda: block(0x0A0D0D0A, struct.pack('<IHHq', 0x1A2B3C4D, 1, 0, -1))
idb = lambda: block(0x00000001, struct.pack('<HHI', 1, 0, 0x40000))
epb = lambda d: block(0x00000006, struct.pack('<IIIII', 0, 0, 0, len(d), len(d)) + pad4(d))
ETH = bytes.fromhex('ffffffffffff001122334455') + b'\x08\x06' + b'\x00' * 28

records = tlv(1, ipaddress.IPv4Address('192.0.2.1').packed + b'host4.example\x00') + tlv(0, b'')
options = tlv(2, b'dns.example.') + tlv(0, b'')     # ns_dnsname + opt_endofopt, 4-aligned
nrb = block(0x00000004, records + options)          # 60 octets: 28 of records, 20 of options

open('/tmp/nrb.pcapng', 'wb').write(shb() + idb() + nrb + epb(ETH))

import pcapkit
pcapkit.extract(fin='/tmp/nrb.pcapng', store=True, nofile=True)

Evidence for the diagnosis

Runtime-patching the two length callbacks on that fixture:

records length	options length	result
length - 12 (as shipped, 48)	__option_padding__ - 4 (16)	60 != 0 — trailing length read from octets 72–75, i.e. from inside the next block
length - 12 (48)	__option_padding__ (20)	60 != 42 — dropping the - 4 does not fix it
28 (the true record area)	__option_padding__ (0)	60 != 786434 = 02 00 0c 00, i.e. the first four octets of the option area — showing exactly where the file pointer is sitting
28 (the true record area)	20 (the true option area)	clean, no warnings

The last two rows are the point: the trailing length only lands correctly when the two fields between them consume the record area and the option area exactly once.

What a fix would need to touch

Not the constant. Either

• OptionField.unpack needs to rewind the file by its unconsumed remainder so the following field starts where it left off — ForwardMatchField already does exactly this rewind at pcapkit/protocols/schema/schema.py:641-642 — or
• NameResolutionBlock needs to parse its records and its options out of one buffer rather than as two file-consuming fields.

NameResolutionBlock is the only schema in pcapkit/protocols/schema/misc/pcapng.py with two OptionFields, so it is the only block that hits this.

One thing that will confuse anyone testing a fix: NS_DNSNameOption (line 1136) has no PaddingField, unlike its siblings IF_NameOption (line 610) and IF_DescriptionOption (line 623), so an ns_dnsname whose value length is not a multiple of 4 under-reads by its own padding on top of everything above. The fixture here uses a 12-octet value to keep the two effects apart.

How it was found

While building deterministic sample captures for the test suite; the generators' module docstrings record it (as an option-sizing error — the mechanism above is what a closer look turned up). examples/samples/pcapng.py exercises the NRB on purpose, because it warns rather than raising.

Line numbers are from main at 72f950d.

[JarryShaw]

Fixed in #371, merged as 19d7962. (GitHub only auto-linked the first issue in that PR's Closes list, so the rest needed closing by hand.)