Skip to content

IPv4 option schemas: a fifth bool-laundering site, a hardcoded Quick-Start length, and a dropped ts_data keyword #552

Description

@JarryShaw

Three defects in IPv4 option schemas, grouped because they share pcapkit/protocols/schema/internet/ipv4.py and want one pass. All found by a sweep of deferred work across this release's 68 merged PRs.

1. TSOption.post_process launders a bool into an address — the fifth instance

pcapkit/protocols/schema/internet/ipv4.py:278,293 call bare ipaddress.ip_address(ip). Measured on main at e7e9ba98f:

TSOption(type=TS, length=12, pointer=5, flags={'oflw':0,'flag':1}, ts_data=[True, 5]).pack()
-> 440c050100000001000000050000000000000000

True became IPv4Address('0.0.0.1') with no exception. Reachable through public IPv4.make(options=[...]), since ipv4.py:1229 accepts a caller-built schema and calls .pack().

This is the fifth site of a defect already fixed four times — #481, #500, #539, with #540 filed for four more. #540 lists this site only as an unconfirmed candidate; it is now confirmed. The same sweep established the other three #540 candidates — esp.py:575, hip.py:3072/:3077, ipv4.py:308 — do not reproduce, being already guarded or bytes-only, which closes #540's open question in both directions.

Fix: route through parse_ip_address from pcapkit/corekit/fields/ipaddress.py, as #539 did for its seven sites.

2. Quick-Start returns a hardcoded 5-octet length

pcapkit/protocols/schema/internet/ipv4.py:128:

return SchemaField(length=5, schema=schema)

The length is fixed at 5 regardless of which Quick-Start suboption the selector resolved, but the option is 5 or 8 octets. On reparse the reader consumes 5, resynchronises on the nonce's second byte, and dies with ProtocolError — silent corruption rather than a clean failure.

Logged during review of #430 and again in #434, and no issue was filed either time.

3. _make_opt_ts passes a keyword the schema does not have

pcapkit/protocols/internet/ipv4.py:1593 passes data=ts_list where the schema field is ts_data. The value is silently dropped, which makes the IPv4 Timestamp option unbuildable through make. Also flagged in #430 and re-flagged in #434 as "carried across unchanged", never filed.

Note the schema rejects unknown field names with UnknownFieldWarning rather than an error, which is why this is silent — worth considering whether that warning should be fatal in make.

Coverage

Each needs a test proving it fails without its fix. For 1, assert the rejection and that a real address still round-trips. For 2, a test that a 8-octet Quick-Start suboption survives a make/parse cycle byte-for-byte. For 3, a test that builds the Timestamp option through IPv4.make and checks the timestamps are present rather than dropped.

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

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions