Repository navigation
IP address fields silently accept bool and corrupt the packet — unfixed sibling of #469/#481 at the shared root #491
Description
Found by strand 3 of the post-wave-1 consistency sweep (docs/source/pep.rst — unaligned changes). Every claim below was reproduced by me on 5959a437c before filing.
This is the unfixed sibling of #469/#481
#481 fixed bool handling at one call site, MH._make_opt_mn_id, because bool is an int subclass and True was silently becoming ::1. The same mechanism is still live at the shared root: _IPAddressField.pre_process in pcapkit/corekit/fields/ipaddress.py:97-120 calls ipaddress.ip_address(value) with no bool exclusion.
ipaddress.ip_address() treats any int below 2**32 as IPv4, so on an IPv4-typed field a bool is silently accepted and corrupts the packet. On an IPv6-typed field it happens to raise, because the resulting IPv4Address's version mismatches — which is why #481's fix looked sufficient. It was not; it was the IPv6 half masking the IPv4 half.
Reproductions
Silent corruption through the public construction API — no exception, no warning:
proto = object.__new__(IPv4)
raw = proto.make(src=True, dst=False).pack()
info = IPv4(io.BytesIO(raw), len(raw)).infopacked -> 4500001400000000001100000000000100000000
decoded -> src=0.0.0.1 dst=0.0.0.0
At the shared root, showing the IPv4/IPv6 asymmetry that hid it:
IPv4AddressField.pre_process(True) -> b'\x00\x00\x00\x01'
IPv4AddressField.pre_process(False) -> b'\x00\x00\x00\x00'
IPv6AddressField.pre_process(True) -> raises FieldValueError (version mismatch)
Outside the Field abstraction entirely — pcapkit/protocols/internet/esp.py:536,554 calls ipaddress.ip_address() directly with no version check at all:
SecurityAssociation(spi=1, destination=True).destination -> IPv4Address('0.0.0.1')
SCTP._make_param_ipv4 (pcapkit/protocols/transport/sctp.py:2504-2507), address=True packs to 0005000800000001 — type 5, length 8, address 0.0.0.1.
One site the sweep flagged but did not execute — I ran it, and it corrupts too
The interface variant shares the path via ipaddress.ip_interface():
IPv4InterfaceField.pre_process(True) -> b'\x00\x00\x00\x01\xff\xff\xff\xff'
So the defect is wider than the audit established: _IPInterfaceField.pre_process in the same file needs the same guard.
Further sites, confirmed by inspection but not executed
Stated as unverified rather than measured. Each pairs a ... | int | ... annotation with an IPv4AddressField, the identical mechanism: RROption/LSROption/SSROption's route/origin parameters in pcapkit/protocols/internet/ipv4.py, and several dual-stack address options in pcapkit/protocols/internet/mh.py around lines 8529, 8571, 8641, 8759, 8794, 8834 and 9343.
Why this is untested rather than intended
Neither tests/corekit/test_fields_ipaddress.py nor tests/protocols/internet/test_ipv4_unit.py passes a bool anywhere, so there is no test asserting this as an allowance. It is genuinely uncovered ground, not a documented decision.
Suggested shape, matching #481's precedent
#481's guard is placed before the subtype dispatch precisely because a correct check in the wrong position does not fire — that placement error was made twice in this repo's history and a passing test did not reveal it either time. The equivalent here belongs in _IPAddressField.pre_process and _IPInterfaceField.pre_process, ahead of the ipaddress.* call, raising an in-library error from pcapkit.utilities.exceptions and pointing the caller at int(...) as #481's message does.
A fix should also carry a test sweeping {True, False} across the address and interface field types, since the IPv6-raises/IPv4-corrupts asymmetry is exactly what let this survive #481.
What the sweep checked and cleared here
Recorded so this ground is not re-audited:
- Field-annotation-versus-Field-class mismatches across the whole schema tree — an AST scan matching every
Field(...)constructor against its declared annotation found only one, the already-trackedRPL.post_processcase atpcapkit/protocols/schema/internet/ipv6_route.py:165(schema: Schema.pack rejects the tuple its own data models declare, breaking parse-then-reconstruct for 16 HIP parameters #476/schema: let ListField.pack accept the tuple its own data models declare #480).pcapng.py:552'sLiteral[0x1A2B3C4D]againstUInt32Fieldis benign —Literal[int]narrowsintlegitimately. EXPECTED_FAILURESis not stale. Imported the module (it cannot be grepped or ast-parsed —**unpacking) with the tree forced ontosys.path: 59 entries, andipv6-route-type/RPL_Source_Route_Headeris the sole surviving ipv6-route one. Ran the suite:tests/protocols/test_option_roundtrip_unit.py6 passed / 358 subtests, including the two guards that assert no entry names a vanished case and that every entry still fails in its recorded way.- ~20
ListField/OptionFieldlist[T]-versus-tuple[T, ...]pairs acrossipv4.py,hip.py,mh.py,tcp.py,sctp.py— all consistent with schema: let ListField.pack accept the tuple its own data models declare #480'sListField.packwidening. - HIP's ~14 field-name remaps (
groups→group_id,suites→suite_id,reg_info→reg_type,locators→locator_setand so on) — the rename is applied consistently in both directions, read and make. Not drift. sctp.py'slist[Enum_Parameter]versustuple[ParameterType, ...]— both are aliases for the samepcapkit.const.sctp.parameter.Parameter. Benign.- Two promising
mypycomplaints atesp.py:436-445and:739traced toCipher/Integritybeing vendor-generatedaenum-extended enums mypy cannot fully type; the runtime values are correct. Retired.
Not covered
The ~40 OrderedMultiDict-versus-list renamed-field pairs across pcapng.py, mh.py, sctp.py, hopopt.py and ipv6_opts.py were spot-checked for mechanism but not verified individually, so a defect in one specific renamed conversion remains possible. The ~10 further IPv4-typed maker sites above were not executed. The full tests/ suite was not run.
Metadata
Metadata
Assignees
Labels
Projects
- StatusShow more project fieldsDone
Activity
ipaddress/esp: reject a bool address in-library, not as silent corrup…