joelagnel / bpfd

BPFd (Deprecated, please see README.md) : Berkeley Packet Filter daemon (BPFd). Makes it possible to run BCC tools across systems.
Apache License 2.0
95 stars 23 forks source link

Dynamically allocate memory before processing user-controlled data #1

Closed ghost closed 6 years ago

ghost commented 6 years ago

Several functions, such as cat_file and cat_tracefs_file, are copying user-controlled data into buffers of fixed size. This could let somebody crash the daemon or even execute arbitrary code on the host running bfpd by writing controlled data outside this buffer.

For instance, when invoking GET_AVAIL_FILTER_FUNCS, cat_tracefs_file can get called with a long argument (more than 100 - strlen(fn) - 2 characters). This input will be strcat'd to a buffer of 100 bytes, statically allocated on the current stack frame, without performing any check to make sure that it is long enough. I compiled bpfd with ASAN to demonstrate more easily this behavior:

STARTED_BPFD
GET_AVAIL_FILTER_FUNCS AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
START_BPFD_OUTPUT
=================================================================
==1493==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fffd8a8f8a4 at pc 0x000000434b4b bp 0x7fffd8a8f810 sp 0x7fffd8a8efc0
WRITE of size 449 at 0x7fffd8a8f8a4 thread T0
    #0 0x434b4a  (/data/build/bpfd+0x434b4a)
    #1 0x510f4d  (/data/build/bpfd+0x510f4d)
    #2 0x512fe5  (/data/build/bpfd+0x512fe5)
    #3 0x7f37c55c0560  (/lib/x86_64-linux-gnu/libc.so.6+0x20560)
    #4 0x419a39  (/data/build/bpfd+0x419a39)

Address 0x7fffd8a8f8a4 is located in stack of thread T0 at offset 132 in frame
    #0 0x510def  (/data/build/bpfd+0x510def)

  This frame has 1 object(s):
    [32, 132) 'tracef' <== Memory access at offset 132 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow (/data/build/bpfd+0x434b4a)
Shadow bytes around the buggy address:
  0x10007b149ec0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007b149ed0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007b149ee0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007b149ef0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007b149f00: 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00 00
=>0x10007b149f10: 00 00 00 00[04]f3 f3 f3 f3 f3 f3 f3 00 00 00 00
  0x10007b149f20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007b149f30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007b149f40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007b149f50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007b149f60: 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
==1493==ABORTING

Dynamically allocating this buffer in both functions would prevent it.

joelagnel commented 6 years ago

Thanks for finding this, and get_trace_events and get_trace_events_categories too.