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.
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
structas a format like'-1s'.Reproduction, on
e2d8ed6d1A bare
struct.erroron untrusted input, with no in-library exception and nothing naming the option that failed.The three sites, on
maincmpt_lenandlenare both read from the wire, so nothing boundslen - 8 - cmpt_len * 4below 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/BytesFieldcrash. A negative length reachesstruct.calcsize, raising a barestruct.error. That is the CALIPSO and MPL case, reproduced above.ListFieldfails silently. Its loop iswhile length > 0, so a negative length simply returns an empty list with no exception at all. That isRegInfoParameter.reg_info— a malformed option parses "successfully" toreg_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
FieldValueErrorfrompcapkit.utilities.exceptionswhen the computed length is negative, matching the existingmpl_opt_seed_id_lenand the newsmf_i_dpd_id_len/registration_type_list_lenhelpers in those same modules. House convention is an in-library exception, never a barestruct.errororValueError.The CALIPSO and MPL expressions are duplicated verbatim between
hopopt.pyandipv6_opts.py, so each wants one shared helper rather than two copies — the same consolidationregistration_type_list_lengot.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
CALIPSOOptioncrash againstd60916b85and flagged inline that no follow-up existed yet, which is what prompted this. Pre-existing onmain, and neither introduced nor worsened by #449, which deliberately scoped itself to the lines #441 and #438 named.