Stack-buffer-overflow in usbmisc.c:56

I have found stack-buffer-overflow in upstream version usbutils (153d41d) in usbmisc.c file, using lsusb as a argv-based harness for fuzzing. Running lsusb with -D argument and incorrect value, which length is more than 4097 symbols causes stack-buffer-overflow. Here is the stacktrace:

==14951==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffff8286d01 at pc 0x000000434646 bp 0x7ffff8284af0 sp 0x7ffff82842b0
READ of size 4103 at 0x7ffff8286d01 thread T0
    #0 0x434645 in strlen (/home/as/usbutils/lsusb+0x434645)
    #1 0x4f1b04 in readlink_recursive /home/as/usbutils/usbmisc.c:56:10
    #2 0x4f13b5 in get_usb_device /home/as/usbutils/usbmisc.c:122:2
    #3 0x4cf809 in dump_one_device /home/as/usbutils/lsusb.c:3607:8
    #4 0x4cf809 in main /home/as/usbutils/lsusb.c:3820:12
    #5 0x797ba9629d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #6 0x797ba9629e3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #7 0x420534 in _start (/home/as/usbutils/lsusb+0x420534)

Address 0x7ffff8286d01 is located in stack of thread T0 at offset 4161 in frame
    #0 0x4f124f in get_usb_device /home/as/usbutils/usbmisc.c:115

  This frame has 3 object(s):
    [32, 40) 'list' (line 116)
    [64, 4161) 'device_path' (line 119)
    [4432, 8529) 'absolute_path' (line 120) <== Memory access at offset 4161 partially underflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow (/home/as/usbutils/lsusb+0x434645) in strlen
Shadow bytes around the buggy address:
  0x10007f048d50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007f048d60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007f048d70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007f048d80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007f048d90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x10007f048da0:[01]f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2
  0x10007f048db0: f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2
  0x10007f048dc0: f2 f2 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007f048dd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007f048de0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007f048df0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==14951==ABORTING

To reproduce this error, you need to build project with ASAN

export CFLAGS="-fsanitize=address -g"
export CXXFLAGS="-fsanitize=address -g"
export LDFLAGS="-fsanitize=address -g"
./autogen.sh
./lsusb -D testvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluetestvaluete

The problem is in the use of the strncpy() function. https://github.com/gregkh/usbutils/blob/153d41d2d1c05f783918a0a837f4f255c92b8e3a/usbmisc.c#L55 In this case, the path string is copied to the buf string with the bufsize size (equals to PATH_MAX+1 == 4097). In case when path length is greater than bufsize value, bufsize bytes are copied to buf and buf string does not end with null-terminator, which cause stack-overflow when passing buf value to strlen() function. https://github.com/gregkh/usbutils/blob/153d41d2d1c05f783918a0a837f4f255c92b8e3a/usbmisc.c#L56 Possible solution - set last byte in buf string to '\0'.

buf[bufsize - 1] = 0;

gregkh / usbutils

Stack-buffer-overflow in usbmisc.c:56 #190