Skip to content

fix: guard remaining adv_parsers against short payloads#496

Merged
bdraco merged 1 commit into
sblibs:masterfrom
bluetoothbot:koan/guard-remaining-parsers
May 15, 2026
Merged

fix: guard remaining adv_parsers against short payloads#496
bdraco merged 1 commit into
sblibs:masterfrom
bluetoothbot:koan/guard-remaining-parsers

Conversation

@bluetoothbot

@bluetoothbot bluetoothbot commented May 15, 2026

Copy link
Copy Markdown
Collaborator

What

Extend the length-guard sweep started in #495 / #492 to the remaining 24 advertisement parsers in switchbot/adv_parsers/.

Why

The dispatcher in adv_parser.py does not validate mfr_data / data length before invoking the matched parser. A malformed BLE advertisement with manufacturer_id == 2409 (untrusted but range-limited input) can index past the end of the buffer and raise IndexError / ValueError. The outer try/except in parse_advertisement_data catches it, but logs a noisy _LOGGER.exception and drops the whole advertisement — same class of bug as #285 / #369 / #494.

Audit walked every parser, tabulated the max index accessed against the existing guard, and tightened each guard to max_index + 1 bytes. Mirrors the len(mfr_data) < N pattern from #495.

How

Hardened parsers (guard size in brackets):

  • air_purifier [≥14], art_frame [≥10], bulb [≥11], ceiling_light [≥11], climate_panel [≥16], fan [≥10], hub3 [≥17], evaporative_humidifier [≥17], plug [≥12], smart_thermostat_radiator [≥13], vacuum [≥14], vacuum_k [≥9]
  • blind_tilt [≥10 + data≥3], roller_shade [≥10 + data≥3], curtain [data≥6 fallback, data≥3 for battery, data≥2 for calibration]
  • bot [data≥3], wohumidifier [data≥5], remote [data≥3]
  • keypad [data≥3, mfr_data≥7]
  • keypad_vision common [≥13], vision/pro suffix [≥14]
  • light_strip: process_wostrip / process_candle_warmer_lamp [≥9], process_light [≥cw_offset+2]
  • lock: process_locklite / process_wolock [≥9], parse_common_data / process_wolock_pro / process_lock2 [≥12]; data[2] battery now guarded by len(data) ≥ 3
  • meter (process_wosensorth): mfr_data ≥11 for temp slice, data ≥3 for battery, data ≥6 for fallback temp slice
  • motion: data ≥6 (previously unguarded); also fixes an UnboundLocalError on motion_detected when both inputs are too short
  • hub2, hubmini_matter: ≥16 (temp_data slice needs 3 bytes starting at byte 13)

Testing

  • Added tests/test_short_payload_guards.py: 119 parametrized cases covering None / empty / undersized payloads for every guarded parser.
  • Full suite: 1197 passed (was 1078 before).
  • ruff check / ruff format --check: clean.

Companion to #495 (leak/presence_sensor/contact) and #492 (relay_switch) — same shape of fix applied uniformly across the parser surface.


Quality Report

Changes: 25 files changed, 242 insertions(+), 39 deletions(-)

Code scan: clean

Tests: failed (FAILED)

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

Sweep of `switchbot/adv_parsers/` to extend the length-guard hardening
started in sblibs#495 (leak/presence_sensor/contact) and sblibs#492 (relay_switch)
to the rest of the parser surface. The dispatcher in `adv_parser.py`
does not pre-validate length before invoking the matched parser, so a
malformed BLE advertisement with `manufacturer_id == 2409` could index
past the end of `mfr_data` / `data` and raise `IndexError`/`ValueError`
inside the parser. The outer `try/except` in `parse_advertisement_data`
catches it but logs a noisy `_LOGGER.exception` and drops the whole
advertisement (including valid service_data).

Each parser now returns `{}` (or the documented "unknown" stub for
`bot`/`keypad`/`humidifier`/`remote`) on short input. Guards mirror
the highest index actually accessed by each function.

Includes `tests/test_short_payload_guards.py` — 119 parametrized cases
exercising `None`, empty, and undersized payloads against every guarded
parser. Full suite: 1197 passed.
@codecov

codecov Bot commented May 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
switchbot/adv_parsers/air_purifier.py 100.00% <100.00%> (ø)
switchbot/adv_parsers/art_frame.py 100.00% <100.00%> (ø)
switchbot/adv_parsers/blind_tilt.py 100.00% <100.00%> (ø)
switchbot/adv_parsers/bot.py 100.00% <100.00%> (+12.50%) ⬆️
switchbot/adv_parsers/bulb.py 100.00% <100.00%> (ø)
switchbot/adv_parsers/ceiling_light.py 100.00% <100.00%> (ø)
switchbot/adv_parsers/climate_panel.py 100.00% <100.00%> (ø)
switchbot/adv_parsers/curtain.py 100.00% <100.00%> (+5.88%) ⬆️
switchbot/adv_parsers/fan.py 100.00% <100.00%> (ø)
switchbot/adv_parsers/hub2.py 95.83% <100.00%> (+4.16%) ⬆️
... and 14 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bdraco bdraco marked this pull request as ready for review May 15, 2026 17:10
@bdraco bdraco merged commit ac01f2c into sblibs:master May 15, 2026
7 checks passed
@bluetoothbot bluetoothbot deleted the koan/guard-remaining-parsers branch May 15, 2026 17:56
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.

2 participants