MH: a Pad1 mobility option crashes parsing with struct.error

[JarryShaw]

Summary

Parsing any Mobility Header whose option area contains a Pad1 option (option type 0) raises struct.error: bad char in struct format. The cause is PadOption.data's length lambda receiving the NoValue sentinel that Schema.unpack stores for a ConditionalField-absent length, which then becomes part of a struct format string. The same pattern affects HOPOPT.

Location

pcapkit/protocols/schema/internet/mh.py:230:

    data: 'bytes' = PaddingField(length=lambda pkt: pkt.get('length', 0))

with length declared at pcapkit/protocols/schema/internet/mh.py:192-195:

    length: 'int' = ConditionalField(
        UInt8Field(default=0),
        lambda pkt: pkt['type'] != Enum_Option.Pad1,
    )

Observed symptom

  File "/pcapkit/protocols/protocol.py", line 280, in unpack
    self.__header__ = cast('_ST', self.__schema__.unpack(self._file, length, packet))
  File "/pcapkit/protocols/schema/schema.py", line 641, in unpack
    value = field.unpack(byte, packet.copy())
  File "/pcapkit/corekit/fields/collections.py", line 270, in unpack
    data = schema.unpack(file, length, packet)
  File "/pcapkit/protocols/schema/schema.py", line 621, in unpack
    byte = data.read(field.length)
  File "/pcapkit/corekit/fields/field.py", line 101, in length
    return struct.calcsize(self.template)
struct.error: bad char in struct format

An option area holding only a PadN parses fine, which isolates it to Pad1:

=== option area with only a PadN (no Pad1): b'\x01\x06\x00\x00\x00\x00\x00\x00'
parsed OK -> BindingRefreshRequestMessage(next=59, length=16, type=0, chksum=b'\x00\x00',
  options=OrderedMultiDict([(<Option.PadN: 1>, <PadOption type=<Option.PadN: 1>, length=8>)]), ...)

pcapkit also cannot read back the Pad1 octet pcapkit itself emits — _make_opt_pad packs a Pad1 to b'\x00' without complaint, and parsing that same octet crashes:

=== packing a Pad1
schema returned: PadOption(type=0, length=0) -> packed b'\x00'   -- packing does NOT crash
=== re-reading pcapkit's own output
option area built from pcapkit output: b'\x00\x01\x05\x00\x00\x00\x00\x00' (8 octets)
re-parse raised struct.error: bad char in struct format

Minimal reproduction

import io

from pcapkit.protocols.internet.mh import MH

# MH Binding Refresh Request whose 8-octet option area is
# Pad1 (0x00) followed by PadN with Option Length = 5.
raw = bytes([59, 1, 0, 0, 0, 0, 0, 0]) + bytes([0x00]) + bytes([0x01, 0x05]) + bytes(5)

MH(io.BytesIO(raw), len(raw))

PadOption.unpack(io.BytesIO(b'\x00'), 1, {}) reproduces it at the schema level with the same final two frames.

Root cause

pkt.get('length', 0) never reaches its 0 default. When a ConditionalField test fails, Schema.unpack at pcapkit/protocols/schema/schema.py:634 stores the sentinel rather than omitting the key:

                if not field.test(packet):
                    self.__buffer__[field.name] = b''
                    setattr(self, field.name, None)
                    packet[field.name] = NoValue
                    continue

So .get finds 'length' present and returns NoValue. That value flows into Field.__call__ (pcapkit/corekit/fields/field.py:267-268, new_self._length = new_self._length_callback(packet)) and then into _TextField.__call__ (pcapkit/corekit/fields/strings.py:64, new_self._template = f'{new_self._length}s'), producing a template built from the sentinel's repr. Field.length (field.py:99-101) calls struct.calcsize on it and raises. Measured directly against the field:

packet={'length': <...NoValueType object at 0x...>}
    -> _length=<...NoValueType object at 0x...>
       template='<pcapkit.corekit.fields.field.NoValueType object at 0x...>s'
       length raised error: bad char in struct format
packet={}             -> _length=0 template='0s' length=0
packet={'length': 5}  -> _length=5 template='5s' length=5

PadOption.post_process (schema/internet/mh.py:207-210), which sets length = 0 for a Pad1, cannot help: it runs after unpack returns (pcapkit/utilities/decorators.py:194), and the crash is inside unpack.

Note that the packing direction works, because Schema.pack seeds the packet dict from self.__dict__ (schema/schema.py:488) where length is a real int. Only the parse direction is affected.

Also affected

pcapkit/protocols/schema/internet/hopopt.py:273 carries the identical pattern under a different field name:

    pad: 'bytes' = PaddingField(length=lambda pkt: pkt.get('len', 0))

and a HOPOPT extension header whose option area contains a Pad1 (b';\x00\x00\x01\x03\x00\x00\x00') raises the same struct.error through the same field.py:101 frame. pcapkit/protocols/schema/internet/ipv6_opts.py:273 has the same line but does not reach it, for an unrelated reason not filed here.

What a fix would need to touch

Narrow fix: the length lambdas — pcapkit/protocols/schema/internet/mh.py:230, pcapkit/protocols/schema/internet/hopopt.py:273, and pcapkit/protocols/schema/internet/ipv6_opts.py:273 — so the NoValue sentinel is coerced to 0 rather than passed through.

Broader fix, covering every pkt.get(...)-style length lambda over a conditional field at once: Schema.unpack at pcapkit/protocols/schema/schema.py:630-635 (do not write the sentinel into the packet dict), or Field.__call__ at pcapkit/corekit/fields/field.py:267-268 (reject or normalise a non-integer callback result).

No existing test covers this: the MH unit tests construct schema.PadOption(type=Pad1, length=0) directly rather than unpacking a Pad1 from bytes, so the whole MH suite passes today (10 passed).

A correction to how this was first reported

It was initially described as "a PadN with length 0 normalises to Pad1 and hits the same crash". That is not what happens: _make_opt_pad(PadN, length=0) normalises to Pad1 and packs fine, returning b'\x00'. The crash is only on the way back in — pcapkit cannot re-parse the octet it just emitted. Recording the correction here so the mechanism is not mis-stated.

Observed vs inferred

• Observed by running code on the current tree: the struct.error traceback above, both via MH(...) and via PadOption.unpack(b'\x00'); that a PadN-only option area parses fine; that the length lambda receives NoValue while an empty packet dict would yield 0; that packing a Pad1 succeeds and re-parsing pcapkit's own output fails; the identical HOPOPT crash; that the MH unit suite is green.
• Inferred by reading code: that post_process runs too late to help; that a fix at schema.py:630-635 or field.py:267-268 would cover all similar lambdas. Neither was verified by patching the source.

[JarryShaw]

Fixed by #398, verified on main at eedebde6d.

The cause was not where it looked. The schema already declared the length octet conditionally, so the ProtocolError path was unreachable — the break was one field further along, in PadOption.pad, which sized itself with pkt.get('length', 0). A skipped ConditionalField is recorded as NoValue rather than omitted, so .get returned the singleton, PaddingField interpolated it into a struct template, and the parse died in the field layer.

>>> PadOption.unpack(io.BytesIO(b'\x00\x2a\x2a'), 3, {})
PadOption(type=0, length=0, data=b'')   # len = 1

One octet consumed, and the following octet left untouched. Previously: struct.error: bad char in struct format.

The same defect class was swept across pcapkit/protocols/schema/ — hopopt.py and ipv6_opts.py had it too and are fixed in the same PR; ipv4.py, httpv2.py, l2tp.py, tcp.py and pcapng.py were checked and are structurally safe.