fix(behaviors): Fix use after free in sticky key

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.
This commit is contained in:
Joel Spadin 2023-04-08 16:32:29 -05:00 committed by Pete Johanson
parent 374104dec6
commit 064aff6bc0

View file

@ -191,6 +191,11 @@ 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 // keep track whether the event has been reraised, so we only reraise it once
bool event_reraised = false; 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++) { for (int i = 0; i < ZMK_BHV_STICKY_KEY_MAX_HELD; i++) {
struct active_sticky_key *sticky_key = &active_sticky_keys[i]; struct active_sticky_key *sticky_key = &active_sticky_keys[i];
if (sticky_key->position == ZMK_BHV_STICKY_KEY_POSITION_FREE) { if (sticky_key->position == ZMK_BHV_STICKY_KEY_POSITION_FREE) {
@ -198,23 +203,24 @@ static int sticky_key_keycode_state_changed_listener(const zmk_event_t *eh) {
} }
if (strcmp(sticky_key->config->behavior.behavior_dev, "KEY_PRESS") == 0 && if (strcmp(sticky_key->config->behavior.behavior_dev, "KEY_PRESS") == 0 &&
ZMK_HID_USAGE_ID(sticky_key->param1) == ev->keycode && ZMK_HID_USAGE_ID(sticky_key->param1) == ev_copy.keycode &&
ZMK_HID_USAGE_PAGE(sticky_key->param1) == ev->usage_page && ZMK_HID_USAGE_PAGE(sticky_key->param1) == ev_copy.usage_page &&
SELECT_MODS(sticky_key->param1) == ev->implicit_modifiers) { SELECT_MODS(sticky_key->param1) == ev_copy.implicit_modifiers) {
// don't catch key down events generated by the sticky key behavior itself // don't catch key down events generated by the sticky key behavior itself
continue; continue;
} }
// If this event was queued, the timer may be triggered late or not at all. // 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. // 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); stop_timer(sticky_key);
release_sticky_key_behavior(sticky_key, sticky_key->release_at); release_sticky_key_behavior(sticky_key, sticky_key->release_at);
continue; continue;
} }
if (ev->state) { // key down if (ev_copy.state) { // key down
if (sticky_key->config->ignore_modifiers && is_mod(ev->usage_page, ev->keycode)) { 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 // ignore modifier key press so we can stack sticky keys and combine with other
// modifiers // modifiers
continue; continue;
@ -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); ZMK_EVENT_RAISE_AFTER(eh, behavior_sticky_key);
event_reraised = true; 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_usage_page = ev_copy.usage_page;
sticky_key->modified_key_keycode = ev->keycode; sticky_key->modified_key_keycode = ev_copy.keycode;
} else { // key up } else { // key up
if (sticky_key->timer_started && if (sticky_key->timer_started &&
sticky_key->modified_key_usage_page == ev->usage_page && sticky_key->modified_key_usage_page == ev_copy.usage_page &&
sticky_key->modified_key_keycode == ev->keycode) { sticky_key->modified_key_keycode == ev_copy.keycode) {
stop_timer(sticky_key); stop_timer(sticky_key);
release_sticky_key_behavior(sticky_key, ev->timestamp); release_sticky_key_behavior(sticky_key, ev_copy.timestamp);
} }
} }
} }