[PATCH libevdev 1/6] Cap slot values to the announced maximum
Benjamin Tissoires
benjamin.tissoires at gmail.com
Thu Mar 6 14:57:24 PST 2014
On Wed, Mar 5, 2014 at 11:44 PM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> A malicious device may announce N slots but then send a slot index >= N. The
> slot state is almost always allocated (definitely the case in libevdev and
> true for most callers), so providing a slot number higher than the announced
> maximum is likely to lead to invalid dereferences. Don't allow that.
> Likewise, don't allow negative slot numbers.
>
> Note that the kernel filters these events anyway, the only way to trigger this
> is to change the device fd to something outside the kernel's control.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> libevdev/libevdev.c | 24 ++++++++++++++
> libevdev/libevdev.h | 7 +++-
> test/test-libevdev-events.c | 78 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 108 insertions(+), 1 deletion(-)
>
> diff --git a/libevdev/libevdev.c b/libevdev/libevdev.c
> index a77c466..d9e2517 100644
> --- a/libevdev/libevdev.c
> +++ b/libevdev/libevdev.c
> @@ -809,6 +809,26 @@ read_more_events(struct libevdev *dev)
> return 0;
> }
>
> +/**
> + * Sanitize/modify events where needed.
> + * @return 0 if untouched, 1 if modified.
> + */
> +static inline int
> +sanitize_event(const struct libevdev *dev, struct input_event *ev)
> +{
> + if (unlikely(dev->num_slots > -1 &&
> + libevdev_event_is_code(ev, EV_ABS, ABS_MT_SLOT) &&
> + (ev->value < 0 || ev->value >= dev->num_slots))) {
> + log_bug("Device %s received an invalid slot index %d."
> + "Capping to announced max slot number %d.\n",
> + dev->name, ev->value, dev->num_slots - 1);
> + ev->value = dev->num_slots - 1;
just nitpicking here, if the slot is < 0, I would have used 0 instead
of dev->num_slots - 1. But it does not matters that much.
Reviewed-by: Benjamin Tissoires <benjamin.tissoires at gmail.com>
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> LIBEVDEV_EXPORT int
> libevdev_next_event(struct libevdev *dev, unsigned int flags, struct input_event *ev)
> {
> @@ -875,6 +895,7 @@ libevdev_next_event(struct libevdev *dev, unsigned int flags, struct input_event
> if (queue_shift(dev, ev) != 0)
> return -EAGAIN;
>
> + sanitize_event(dev, ev);
> update_state(dev, ev);
>
> /* if we disabled a code, get the next event instead */
> @@ -1072,6 +1093,9 @@ libevdev_set_event_value(struct libevdev *dev, unsigned int type, unsigned int c
> e.code = code;
> e.value = value;
>
> + if (sanitize_event(dev, &e))
> + return -1;
> +
> switch(type) {
> case EV_ABS: rc = update_abs_state(dev, &e); break;
> case EV_KEY: rc = update_key_state(dev, &e); break;
> diff --git a/libevdev/libevdev.h b/libevdev/libevdev.h
> index eab02ba..1855cd3 100644
> --- a/libevdev/libevdev.h
> +++ b/libevdev/libevdev.h
> @@ -1111,6 +1111,10 @@ int libevdev_get_event_value(const struct libevdev *dev, unsigned int type, unsi
> * event code is the value of the currently active slot. You should use
> * libevdev_set_slot_value() instead.
> *
> + * If the device supports ABS_MT_SLOT and the type is EV_ABS and the code is
> + * ABS_MT_SLOT, the value must be a positive number less then the number of
> + * slots on the device. Otherwise, libevdev_set_event_value() returns -1.
> + *
> * @param dev The evdev device, already initialized with libevdev_set_fd()
> * @param type The event type for the code to query (EV_SYN, EV_REL, etc.)
> * @param code The event code to set the value for, one of ABS_X, LED_NUML, etc.
> @@ -1118,7 +1122,8 @@ int libevdev_get_event_value(const struct libevdev *dev, unsigned int type, unsi
> *
> * @return 0 on success, or -1 on failure.
> * @retval -1 the device does not have the event type or code enabled, or the code is outside the
> - * allowed limits for the given type, or the type cannot be set.
> + * allowed limits for the given type, or the type cannot be set, or the
> + * value is not permitted for the given code.
> *
> * @see libevdev_set_slot_value
> * @see libevdev_get_event_value
> diff --git a/test/test-libevdev-events.c b/test/test-libevdev-events.c
> index c9ceb33..8d1bd8b 100644
> --- a/test/test-libevdev-events.c
> +++ b/test/test-libevdev-events.c
> @@ -1143,6 +1143,83 @@ START_TEST(test_mt_event_values_invalid)
> }
> END_TEST
>
> +START_TEST(test_mt_slot_ranges_invalid)
> +{
> + struct uinput_device* uidev;
> + struct libevdev *dev;
> + struct input_event ev[2];
> + int rc;
> + struct input_absinfo abs[5];
> + int num_slots = 2;
> + int pipefd[2];
> +
> + memset(abs, 0, sizeof(abs));
> + abs[0].value = ABS_X;
> + abs[0].maximum = 1000;
> + abs[1].value = ABS_MT_POSITION_X;
> + abs[1].maximum = 1000;
> +
> + abs[2].value = ABS_Y;
> + abs[2].maximum = 1000;
> + abs[3].value = ABS_MT_POSITION_Y;
> + abs[3].maximum = 1000;
> +
> + abs[4].value = ABS_MT_SLOT;
> + abs[4].maximum = num_slots - 1;
> +
> + rc = test_create_abs_device(&uidev, &dev,
> + 5, abs,
> + EV_SYN, SYN_REPORT,
> + -1);
> + ck_assert_msg(rc == 0, "Failed to create device: %s", strerror(-rc));
> +
> + rc = pipe2(pipefd, O_NONBLOCK);
> + ck_assert_int_eq(rc, 0);
> + libevdev_change_fd(dev, pipefd[0]);
> +
> + ev[0].type = EV_ABS;
> + ev[0].code = ABS_MT_SLOT;
> + ev[0].value = num_slots;
> + ev[1].type = EV_SYN;
> + ev[1].code = SYN_REPORT;
> + ev[1].value = 0;
> + rc = write(pipefd[1], ev, sizeof(ev));
> + ck_assert_int_eq(rc, sizeof(ev));
> +
> + libevdev_set_log_function(test_logfunc_ignore_error, NULL);
> +
> + rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, ev);
> + ck_assert(libevdev_event_is_code(ev, EV_ABS, ABS_MT_SLOT));
> + ck_assert_int_eq(ev[0].value, num_slots - 1);
> +
> + /* drain the EV_SYN */
> + libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, ev);
> +
> + ev[0].type = EV_ABS;
> + ev[0].code = ABS_MT_SLOT;
> + ev[0].value = -1;
> + ev[1].type = EV_SYN;
> + ev[1].code = SYN_REPORT;
> + ev[1].value = 0;
> + rc = write(pipefd[1], ev, sizeof(ev));
> + ck_assert_int_eq(rc, sizeof(ev));
> +
> + rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, ev);
> + ck_assert(libevdev_event_is_code(ev, EV_ABS, ABS_MT_SLOT));
> + ck_assert_int_eq(ev[0].value, num_slots - 1);
> +
> + ck_assert_int_eq(libevdev_get_current_slot(dev), num_slots - 1);
> +
> + ck_assert_int_eq(libevdev_set_event_value(dev, EV_ABS, ABS_MT_SLOT, num_slots), -1);
> + ck_assert_int_eq(libevdev_set_event_value(dev, EV_ABS, ABS_MT_SLOT, -1), -1);
> +
> + libevdev_set_log_function(test_logfunc_abort_on_error, NULL);
> +
> + uinput_device_free(uidev);
> + libevdev_free(dev);
> +}
> +END_TEST
> +
> START_TEST(test_ev_rep_values)
> {
> struct uinput_device* uidev;
> @@ -1447,6 +1524,7 @@ libevdev_events(void)
> tcase_add_test(tc, test_event_values_invalid);
> tcase_add_test(tc, test_mt_event_values);
> tcase_add_test(tc, test_mt_event_values_invalid);
> + tcase_add_test(tc, test_mt_slot_ranges_invalid);
> tcase_add_test(tc, test_ev_rep_values);
> suite_add_tcase(s, tc);
>
> --
> 1.8.5.3
>
> _______________________________________________
> Input-tools mailing list
> Input-tools at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/input-tools
More information about the Input-tools
mailing list