Four more HIP parameters share the unguarded pkt['len'] - 2 shape that #455 fixed for RegInfoParameter, and they fail the silent way rather than crashing: a malformed parameter parses "successfully" to an empty list with no exception at all.
The four sites, on da2422728
pcapkit/protocols/schema/internet/hip.py:454 NATTraversalModeParameter.modes
pcapkit/protocols/schema/internet/hip.py:808 TransportFormatListParameter.formats
pcapkit/protocols/schema/internet/hip.py:826 ESPTransformParameter.suites
pcapkit/protocols/schema/internet/hip.py:941 HIPTransportModeParameter.mode
All four are the same expression on a wire-controlled UInt16Field len with no lower bound:
length=lambda pkt: pkt['len'] - 2,
Reproduction
A parameter with Length=0 makes the computed length -2:
NATTraversalModeParameter NO EXCEPTION -> lists=[[]]
TransportFormatListParameter NO EXCEPTION -> lists=[[]]
ESPTransformParameter NO EXCEPTION -> lists=[[]]
HIPTransportModeParameter NO EXCEPTION -> lists=[[]]
ListField's loop is while length > 0, so a negative length simply returns an empty list. Nothing signals that the input was malformed — which is arguably worse than the struct.error crash the PaddingField/BytesField sites produce, because a caller gets a plausible-looking parse.
Relationship to #455
#455 fixed exactly this shape at three sites and established the pattern: a named function raising FieldValueError from pcapkit.utilities.exceptions, following the module's own mpl_opt_seed_id_len convention, with duplicated expressions consolidated into a shared helper. PR #460 implemented that and named reg_info_list_len for the RegInfoParameter case. These four want the same treatment — four more named functions, or one shared parametrised helper, since the expression is byte-identical across all four.
Note the offset is genuinely - 2 for these, and it is - 2 for RegInfoParameter for a different reason: RegInfoParameter reads two UInt8Fields (min_lifetime, max_lifetime) before its list, whereas these four read a two-octet reserved or port field. So a shared helper is fine, but it should be named for the shape rather than for either field's meaning.
Two lines that are NOT affected, recorded so nobody re-checks them
pcapkit/protocols/schema/internet/hip.py:420 (HIPTransformParameter.suites) and :635 (HITSuiteListParameter.suites) use a bare pkt['len'] with no subtraction, so they cannot underflow this way. I conflated those two with the four above at one point; they are a different shape and are fine.
Provenance
Flagged out-of-scope by the agent implementing #455 in PR #460, then independently verified empirically by that PR's reviewer, and reproduced a third time here before filing. Correctly excluded from #460, whose scope is the three sites #455 named.
Four more HIP parameters share the unguarded
pkt['len'] - 2shape that #455 fixed forRegInfoParameter, and they fail the silent way rather than crashing: a malformed parameter parses "successfully" to an empty list with no exception at all.The four sites, on
da2422728All four are the same expression on a wire-controlled
UInt16Field lenwith no lower bound:Reproduction
A parameter with
Length=0makes the computed length-2:ListField's loop iswhile length > 0, so a negative length simply returns an empty list. Nothing signals that the input was malformed — which is arguably worse than thestruct.errorcrash thePaddingField/BytesFieldsites produce, because a caller gets a plausible-looking parse.Relationship to #455
#455 fixed exactly this shape at three sites and established the pattern: a named function raising
FieldValueErrorfrompcapkit.utilities.exceptions, following the module's ownmpl_opt_seed_id_lenconvention, with duplicated expressions consolidated into a shared helper. PR #460 implemented that and namedreg_info_list_lenfor theRegInfoParametercase. These four want the same treatment — four more named functions, or one shared parametrised helper, since the expression is byte-identical across all four.Note the offset is genuinely
- 2for these, and it is- 2forRegInfoParameterfor a different reason:RegInfoParameterreads twoUInt8Fields (min_lifetime,max_lifetime) before its list, whereas these four read a two-octetreservedorportfield. So a shared helper is fine, but it should be named for the shape rather than for either field's meaning.Two lines that are NOT affected, recorded so nobody re-checks them
pcapkit/protocols/schema/internet/hip.py:420(HIPTransformParameter.suites) and:635(HITSuiteListParameter.suites) use a barepkt['len']with no subtraction, so they cannot underflow this way. I conflated those two with the four above at one point; they are a different shape and are fine.Provenance
Flagged out-of-scope by the agent implementing #455 in PR #460, then independently verified empirically by that PR's reviewer, and reproduced a third time here before filing. Correctly excluded from #460, whose scope is the three sites #455 named.