Skip to content

fix(split): bounds check the source of a peripheral battery event - #3556

Open
akira-toriyama wants to merge 1 commit into
zmkfirmware:mainfrom
akira-toriyama:fix/split-battery-source-bounds
Open

akira-toriyama wants to merge 1 commit into
zmkfirmware:mainfrom
akira-toriyama:fix/split-battery-source-bounds

Conversation

@akira-toriyama

Copy link
Copy Markdown

With CONFIG_ZMK_SPLIT_BLE_CENTRAL_BATTERY_LEVEL_FETCHING=y, split_central_disconnected() in app/src/split/bluetooth/central.c queues a battery event for every disconnected connection, with the source from peripheral_slot_index_for_conn(), which returns -EINVAL for a connection that holds no peripheral slot. The handler in app/src/split/central.c writes peripheral_battery_levels[source] without a range check, so a host's disconnect writes peripheral_battery_levels[234] (#3522).

This adds the range check on the write side, matching zmk_split_central_get_peripheral_battery_level(), and the BLE test app/tests/ble/split/battery-level-host-disconnect: a host (-disconnect_and_reconnect) connects to a split central that fetches and proxies its peripheral's battery level, then disconnects and reconnects. The snapshot keeps the central's connection, battery and slot lines.

Measured in zmkfirmware/zmk-build-arm:4.1 with ./run-ble-test.sh tests/ble/split/battery-level-host-disconnect:

  • on main (4dd95d7) the test fails: after the host's disconnect the proxy logs Got battery level event for an out of range peripheral index;
  • with this change it passes: the central logs Ignoring battery event from out of range peripheral 234 and the proxy gets no event. The peripheral's own level (Peripheral battery level event: 100) still goes through.

Fixes #3522

PR check-list

  • Branch has a clean commit history
  • Additional tests are included, if changing behaviors/core code that is testable.
  • Proper Copyright + License headers added to applicable files (the test files follow the other app/tests/ble/split tests, which carry none)
  • Pre-commit used to check formatting of files, commit messages, etc. (clang-format 18.1.8, gitlint)
  • Includes any necessary documentation changes. (none needed)

🤖 Generated with Claude Code

The BLE transport queues a battery event of level 0 for every
disconnected connection, with the source from
peripheral_slot_index_for_conn(), which returns -EINVAL for a
connection that holds no peripheral slot. A host's disconnect therefore
reaches the central's event handler with source 234, and
peripheral_battery_levels[234] = 0 is written past the array. Check
the source against ZMK_SPLIT_CENTRAL_PERIPHERAL_COUNT before the write,
as zmk_split_central_get_peripheral_battery_level() does on the read
side.

Adds a BLE test in which a host connects to a split central that
fetches and proxies the peripheral's battery level, then disconnects
and reconnects: the central rejects the event with source 234 instead
of passing it to the battery level proxy as an out-of-range event.

Fixes zmkfirmware#3522

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.

Split BLE central: out-of-bounds write to peripheral_battery_levels[] on a non-peripheral disconnect (source = (uint8_t)-EINVAL = 234)

1 participant