Skip to content

Commit

Permalink
fix(behaviors): Fix use after free in sticky key
Browse files Browse the repository at this point in the history
Fixed an issue where the sticky key behavior would call
ZMK_EVENT_RAISE_AFTER(), which would free the provided event, but then
it would keep using that now-freed event data.
  • Loading branch information
joelspadin committed Apr 8, 2023
1 parent 7434a6b commit 338d232
Showing 1 changed file with 18 additions and 12 deletions.
30 changes: 18 additions & 12 deletions app/src/behaviors/behavior_sticky_key.c
Expand Up @@ -191,30 +191,36 @@ static int sticky_key_keycode_state_changed_listener(const zmk_event_t *eh) {

// keep track whether the event has been reraised, so we only reraise it once
bool event_reraised = false;

// reraising the event frees it, so make a copy of any event data we might
// need after it's been freed.
const struct zmk_keycode_state_changed ev_copy = *ev;

for (int i = 0; i < ZMK_BHV_STICKY_KEY_MAX_HELD; i++) {
struct active_sticky_key *sticky_key = &active_sticky_keys[i];
if (sticky_key->position == ZMK_BHV_STICKY_KEY_POSITION_FREE) {
continue;
}

if (strcmp(sticky_key->config->behavior.behavior_dev, "KEY_PRESS") == 0 &&
ZMK_HID_USAGE_ID(sticky_key->param1) == ev->keycode &&
ZMK_HID_USAGE_PAGE(sticky_key->param1) == ev->usage_page &&
SELECT_MODS(sticky_key->param1) == ev->implicit_modifiers) {
ZMK_HID_USAGE_ID(sticky_key->param1) == ev_copy.keycode &&
ZMK_HID_USAGE_PAGE(sticky_key->param1) == ev_copy.usage_page &&
SELECT_MODS(sticky_key->param1) == ev_copy.implicit_modifiers) {
// don't catch key down events generated by the sticky key behavior itself
continue;
}

// If this event was queued, the timer may be triggered late or not at all.
// Release the sticky key if the timer should've run out in the meantime.
if (sticky_key->release_at != 0 && ev->timestamp > sticky_key->release_at) {
if (sticky_key->release_at != 0 && ev_copy.timestamp > sticky_key->release_at) {
stop_timer(sticky_key);
release_sticky_key_behavior(sticky_key, sticky_key->release_at);
continue;
}

if (ev->state) { // key down
if (sticky_key->config->ignore_modifiers && is_mod(ev->usage_page, ev->keycode)) {
if (ev_copy.state) { // key down
if (sticky_key->config->ignore_modifiers &&
is_mod(ev_copy.usage_page, ev_copy.keycode)) {
// ignore modifier key press so we can stack sticky keys and combine with other
// modifiers
continue;
Expand All @@ -231,17 +237,17 @@ static int sticky_key_keycode_state_changed_listener(const zmk_event_t *eh) {
ZMK_EVENT_RAISE_AFTER(eh, behavior_sticky_key);
event_reraised = true;
}
release_sticky_key_behavior(sticky_key, ev->timestamp);
release_sticky_key_behavior(sticky_key, ev_copy.timestamp);
}
}
sticky_key->modified_key_usage_page = ev->usage_page;
sticky_key->modified_key_keycode = ev->keycode;
sticky_key->modified_key_usage_page = ev_copy.usage_page;
sticky_key->modified_key_keycode = ev_copy.keycode;
} else { // key up
if (sticky_key->timer_started &&
sticky_key->modified_key_usage_page == ev->usage_page &&
sticky_key->modified_key_keycode == ev->keycode) {
sticky_key->modified_key_usage_page == ev_copy.usage_page &&
sticky_key->modified_key_keycode == ev_copy.keycode) {
stop_timer(sticky_key);
release_sticky_key_behavior(sticky_key, ev->timestamp);
release_sticky_key_behavior(sticky_key, ev_copy.timestamp);
}
}
}
Expand Down

0 comments on commit 338d232

Please sign in to comment.