Skip to content

hsrp, bluetooth, kerberos: fix three dissection crashes found by fuzzing - #5147

Open
nkbeast wants to merge 3 commits into
secdev:masterfrom
nkbeast:fix/dissector-crashes-hsrp-bluetooth-kerberos
Open

hsrp, bluetooth, kerberos: fix three dissection crashes found by fuzzing#5147
nkbeast wants to merge 3 commits into
secdev:masterfrom
nkbeast:fix/dissector-crashes-hsrp-bluetooth-kerberos

Conversation

@nkbeast

@nkbeast nkbeast commented Sep 2, 2026

Copy link
Copy Markdown

Three independent dissection crashes found by running random bytes through the default layers. Each commit is self-contained with its UTScapy test; no existing behavior changes.

1. hsrp

HSRP.guess_payload_class read self.underlayer.len unconditionally:

>>> HSRP(b"\x00\x01" + b"\x00" * 26 + b"A" * 20)
AttributeError: 'NoneType' object has no attribute 'len'

Same class as #5089 (MQTTPublish). The HSRPmd5 selection is unchanged when the underlayer is present.

2. bluetooth

The six FieldListField entries of HCI_Event_Inquiry_Result_With_Rssi were given field classes instead of instances, so the event could never be dissected with num_response >= 1:

>>> HCI_Event_Inquiry_Result_With_Rssi(bytes.fromhex("011122334455660100000000000000c0"))
TypeError: Field.getfield() missing 1 required positional argument: 's'

Fixed by passing instances; the test campaign now round-trips a full inquiry result.

3. kerberos

KRB_InnerToken indexed _InitialContextTokens[_parent.TOK_ID] directly, so any TOK_ID outside the implemented set aborted the dissection with a KeyError. Fall back to conf.raw_layer (consistent with the existing mysummary() fallback), keeping the body as raw bytes. Known TOK_IDs are unaffected.

NK added 3 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)
@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.81%. Comparing base (329ecab) to head (39e9a09).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5147   +/-   ##
=======================================
  Coverage   80.80%   80.81%           
=======================================
  Files         390      390           
  Lines       96931    96931           
=======================================
+ Hits        78329    78335    +6     
+ Misses      18602    18596    -6     
Files with missing lines Coverage Δ
scapy/layers/bluetooth.py 90.96% <ø> (ø)
scapy/layers/hsrp.py 97.53% <100.00%> (+1.23%) ⬆️
scapy/layers/kerberos.py 70.50% <ø> (ø)

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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