stack-buffer-overflow (attr cast to union then pass by value): usbg_f_foo_set_attr_val(..., union usbg_f_foo_attr_val val)

[wdl-crestron]

Hi,
While working with HID gadget I discovered a bug which may be present in multiple places (attr set/get).
Easiest way is to run the code with address sanitizer:
CFLAGS="-O0 -g -fsanitize=address" ./configure; make; make install

HID bug:

==1476==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x007ffffff1d0 at pc 0x007ff74f61a4 bp 0x007fffffdca0 sp 0x007fffffdcb8
READ of size 16 at 0x007ffffff1d0 thread T0
...
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x0000007ff7376aa0 in __GI_abort () at abort.c:79
#2  0x0000007ff75e32a4 in __sanitizer::Abort () at ../../../../src/libsanitizer/sanitizer_common/sanitizer_posix_libcdep.cpp:155
#3  0x0000007ff75edce0 in __sanitizer::Die () at ../../../../src/libsanitizer/sanitizer_common/sanitizer_termination.cpp:58
#4  0x0000007ff75d0de8 in __asan::ScopedInErrorReport::~ScopedInErrorReport (this=0x7fffffd098, __in_chrg=<optimized out>) at ../../../../src/libsanitizer/asan/asan_report.cpp:186
#5  0x0000007ff75d06f0 in __asan::ReportGenericError (pc=549610021284, bp=bp@entry=549755804832, sp=sp@entry=549755804856, addr=549755810256, is_write=is_write@entry=false, 
    access_size=access_size@entry=16, exp=exp@entry=0, fatal=fatal@entry=true) at ../../../../src/libsanitizer/asan/asan_report.cpp:470
#6  0x0000007ff75d1460 in __asan::__asan_report_load16 (addr=<optimized out>) at ../../../../src/libsanitizer/asan/asan_rtl.cpp:121
#7  0x0000007ff74f61a4 in usbg_f_hid_set_attrs (hf=0x7ff3a01910, attrs=0x7ffffff1b0) at function/hid.c:329
#8  0x0000007ff74f596c in hid_set_attrs (f=0x7ff3a01910, f_attrs=0x7ffffff1b0) at function/hid.c:207
#9  0x0000007ff74eb0d4 in usbg_set_function_attrs (f=0x7ff3a01910, f_attrs=0x7ffffff1b0) at usbg.c:2621
#10 0x0000007ff74e78b8 in usbg_create_function (g=0x7ff2801ea0, type=USBG_F_HID, instance=0x5555553f60 "usb0", f_attrs=0x7ffffff1b0, f=0x7fffffef80) at usbg.c:2097
#11 0x00000055555531b4 in main () at gadget_multi.c:224

(gdb) f 7
#7  0x0000007ff74f61a4 in usbg_f_hid_set_attrs (hf=0x7ff3a01910, attrs=0x7ffffff1b0) at function/hid.c:329
329			ret = usbg_f_hid_set_attr_val(hf, i,
(gdb) list
324	
325		for (i = USBG_F_HID_ATTR_MIN; i < USBG_F_HID_ATTR_MAX; ++i) {
326			if (hid_attr[i].ro)
327				continue;
328	
329			ret = usbg_f_hid_set_attr_val(hf, i,
330						       *(union usbg_f_hid_attr_val *)
331						       ((char *)attrs
332							+ hid_attr[i].offset));
333			if (ret)
(gdb) p *attrs
$2 = {dev = 0, protocol = 1, report_desc = {desc = 0x5555565100 <report_desc> "\006", len = 28}, report_length = 8, subclass = 0}
(gdb) p sizeof(union usbg_f_hid_attr_val)
$3 = 16
(gdb) p i
$4 = 3
(gdb) p hid_attr[i].offset
$5 = 32
(gdb) f 11
#11 0x00000055555531b4 in main () at gadget_multi.c:224
224		usbg_ret = usbg_create_function(gadget, USBG_F_HID, "usb0", &hid_attrs, &f_hid);
(gdb) p sizeof(hid_attrs)
$6 = 40

As can be seen from GDB, union (union usbg_f_hid_attr_val) size is 16 byte (aligned to max member - struct in this case).
When user provided attribute address is cast to union: (union usbg_f_hid_attr_val *) ((char *)attrs + hid_attr[i].offset)
and then used to initialize new union object (call to usbg_f_hid_set_attr_val), to be passed by value to setter, we have problem as access is 16byte but only 8 byte belong to user provided attribute struct.