Skip to content

zigbee: fix ZCLPricePublishPrice rate_label dissection - #5148

Open
nkbeast wants to merge 4 commits into
secdev:masterfrom
nkbeast:fix/zcl-price-publish-rate-label
Open

zigbee: fix ZCLPricePublishPrice rate_label dissection#5148
nkbeast wants to merge 4 commits into
secdev:masterfrom
nkbeast:fix/zcl-price-publish-rate-label

Conversation

@nkbeast

@nkbeast nkbeast commented Sep 2, 2026

Copy link
Copy Markdown

ZCLPricePublishPrice.rate_label used a self-referential length callback:

StrLenField("rate_label", "", length_from=lambda pkt: int(pkt.rate_label[0]))

length_from reads the field's own value before it is set, so pkt.rate_label is still the empty default and [0] raises IndexError on every dissection — the Publish Price command could never be parsed at all.

Per the wire format noted in the comment ("The first Octet indicates the length"), the fix reads the length octet from the remaining buffer, following the _DiscreteString convention of keeping the length tag inside the value:

>>> p = ZCLPricePublishPrice(bytes.fromhex("1122334404616263645566778899aabbcc01ccdd0203445566778899aabbccdd112233445566778899aabbccddee"))
>>> p.rate_label
b'\x04abcd'
>>> p.issuer_event_id
0x88776655

Round trip is byte-identical; a UTScapy test is included.

NK added 4 commits September 3, 2026 02:06
HSRP.guess_payload_class read self.underlayer.len unconditionally, so
dissecting an HSRP packet that carries a trailing payload without an
underlayer (e.g. HSRP(b'\x00\x01' + ...) or via layers that do not
expose .len) raised AttributeError instead of dissecting the trailing
bytes as the default payload class.

Guard the underlayer access, keeping the HSRPmd5 selection unchanged
when the underlayer is present and long enough.

AI-Assisted: yes (Claude)
The six FieldListField entries were given the field classes (LEMACField,
ByteField, ...) instead of instances, so any dissection of the event
with num_response >= 1 raised 'Field.getfield() missing 1 required
positional argument'. FieldListField expects an instance per its
annotation (field: AnyField).

Pass instances and cover a full inquiry result round trip in the test
campaign.

AI-Assisted: yes (Claude)
_InitialContextTokens[_parent.TOK_ID] raised KeyError for any TOK_ID
outside the implemented set, aborting the dissection of a token with an
attacker-supplied or future token identifier. Fall back to
conf.raw_layer, consistent with the mysummary() fallback, keeping the
body as raw bytes. Known TOK_IDs are unaffected.

AI-Assisted: yes (Claude)
rate_label used a self-referential length callback,
int(pkt.rate_label[0]), which reads the field before it is set, so
dissecting the command always raised IndexError and the packet could
never be parsed. The Rate Label is an octet string whose first octet
carries its length; read that octet from the remaining buffer instead,
keeping the length octet as part of the value (same convention as
_DiscreteString in this module).

AI-Assisted: yes (Claude)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant