From 62169cd2b5da251afdd2c3334b7032de8543141a Mon Sep 17 00:00:00 2001 From: Jonas Malaco Date: Sat, 7 Jan 2023 02:23:34 -0300 Subject: [PATCH] hidraw: fix number of bytes to copy from uevent in parse_uevent_info Commit 5c9f147a07d8 (#432) replaced a call to strdup with an explicit memcpy to a buffer on the stack. However, it incorrectly used the buffer size, instead of the clamped uevent length, as the argument to memcpy, resulting in reads past the end of uevent: $ valgrind -q hidtest/hidtest_hidraw 1>/dev/null ==51900== Invalid read of size 8 ==51900== at 0x48571D5: parse_uevent_info (hid.c:496) ==51900== by 0x48574F2: create_device_info_for_device (hid.c:578) ==51900== by 0x4857ED7: hid_enumerate (hid.c:876) ==51900== by 0x1094CE: main (test.c:105) ==51900== Address 0x4b6c1a0 is 7 bytes after a block of size 185 alloc'd ==51900== at 0x4846CC3: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==51900== by 0x4AA0E45: UnknownInlinedFun (fileio.c:534) ==51900== by 0x4AA0E45: read_virtual_file_at.constprop.0 (fileio.c:572) ==51900== by 0x4A9D37C: UnknownInlinedFun (fileio.h:74) ==51900== by 0x4A9D37C: UnknownInlinedFun (fileio.h:77) ==51900== by 0x4A9D37C: sd_device_get_sysattr_value (sd-device.c:2318) ==51900== by 0x4A8B0D1: udev_device_get_sysattr_value (libudev-device.c:747) ==51900== by 0x48574BE: create_device_info_for_device (hid.c:578) ==51900== by 0x4857ED7: hid_enumerate (hid.c:876) ==51900== by 0x1094CE: main (test.c:105) ==51900== Fix this by using uevent_len as the argument to memcpy. Calling strndupa was considered but abandoned, as it is not standard. Fixes: 5c9f147a07d8 (#432) Fixes: 4779d63d8760 --- linux/hid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux/hid.c b/linux/hid.c index 1866ee4f0..07c04c2ac 100644 --- a/linux/hid.c +++ b/linux/hid.c @@ -493,7 +493,7 @@ static int parse_uevent_info(const char *uevent, unsigned *bus_type, size_t uevent_len = strlen(uevent); if (uevent_len > sizeof(tmp) - 1) uevent_len = sizeof(tmp) - 1; - memcpy(tmp, uevent, sizeof(tmp)); + memcpy(tmp, uevent, uevent_len); tmp[uevent_len] = '\0'; char *saveptr = NULL;