Skip to content

Three more wire-derived lengths underflow: CALIPSO and MPL padding crash in struct, RegInfoParameter silently returns empty #455

Description

@JarryShaw

Three more wire-derived length expressions share the unguarded-underflow shape that #438 fixes elsewhere: a crafted or truncated option makes the subtraction negative, and the negative length reaches struct as a format like '-1s'.

Reproduction, on e2d8ed6d1

from pcapkit.protocols.internet.hopopt import HOPOPT
HOPOPT(bytes.fromhex('3b0007000000000000000000000000'), 16)
struct.error: bad char in struct format

A bare struct.error on untrusted input, with no in-library exception and nothing naming the option that failed.

The three sites, on main

pcapkit/protocols/schema/internet/hopopt.py:371     CALIPSOOption.pad
pcapkit/protocols/schema/internet/ipv6_opts.py:371  CALIPSOOption.pad   (same expression)
    pad: 'bytes' = PaddingField(length=lambda pkt: pkt['len'] - 8 - pkt['cmpt_len'] * 4)

pcapkit/protocols/schema/internet/hopopt.py:634     MPLOption.pad
pcapkit/protocols/schema/internet/ipv6_opts.py:642  MPLOption.pad       (same expression)
    pad: 'bytes' = PaddingField(length=lambda pkt: pkt['len'] - 2 - (
        0 if pkt['flags']['type'] == 0 else mpl_opt_seed_id_len(pkt)))

pcapkit/protocols/schema/internet/hip.py:680        RegInfoParameter.reg_info
    reg_info: 'list[Enum_Registration]' = ListField(length=lambda pkt: pkt['len'] - 2)

cmpt_len and len are both read from the wire, so nothing bounds len - 8 - cmpt_len * 4 below zero. Note the line numbers differ on PR #449's branch (hopopt.py:392,653 / ipv6_opts.py:392,658 / hip.py:706) because it adds lines above them.

Two different symptoms, same root

Worth knowing before fixing, because a test written for one will not catch the other — this distinction was established while fixing #438:

  • PaddingField/BytesField crash. A negative length reaches struct.calcsize, raising a bare struct.error. That is the CALIPSO and MPL case, reproduced above.
  • ListField fails silently. Its loop is while length > 0, so a negative length simply returns an empty list with no exception at all. That is RegInfoParameter.reg_info — a malformed option parses "successfully" to reg_info=[], which is arguably worse than crashing because nothing signals it.

Fix

Follow what #438 established in PR #449: replace the lambda with a named function that raises FieldValueError from pcapkit.utilities.exceptions when the computed length is negative, matching the existing mpl_opt_seed_id_len and the new smf_i_dpd_id_len / registration_type_list_len helpers in those same modules. House convention is an in-library exception, never a bare struct.error or ValueError.

The CALIPSO and MPL expressions are duplicated verbatim between hopopt.py and ipv6_opts.py, so each wants one shared helper rather than two copies — the same consolidation registration_type_list_len got.

Provenance

Disclosed in PR #449's body as "being filed separately" — this is that issue, and it is filed later than it should have been. Its reviewer independently reproduced the CALIPSOOption crash against d60916b85 and flagged inline that no follow-up existed yet, which is what prompted this. Pre-existing on main, and neither introduced nor worsened by #449, which deliberately scoped itself to the lines #441 and #438 named.

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