sfeakes / AqualinkD

Daemon to control Jandy Aqualink RS pool equipment from any home automation hub (Alexa, Homekit & Siri, Home Assistant, smartthings, domoticz etc) or web browser.
Other
172 stars 47 forks source link

Buffer overflow in action_web_request #135

Closed ballle98 closed 1 year ago

ballle98 commented 3 years ago

Problem is here, the size of buff is 50 and URI of more than 13 characters will overflow. Need to increase buffer and use snprintf() instead of sprintf().

sprintf(buf, "action_web_request() request '%.*s' took",http_msg->uri.len, http_msg->uri.p);

console output reporting error:

16:34:46.662 Info:    NetService:URI request: '/ '
16:34:46.671 Debug:   NetService:Served WEB request
16:34:46.690 Debug:   NetService:-- Websocket left
16:34:46.707 Info:    NetService:URI request: '/api/dynamicconfig '
16:34:46.709 Debug:   NetService:API: URI Request 'dynamicconfig': value 0.00
=================================================================
==23335==ERROR: AddressSanitizer: stack-buffer-overflow on address 0xb47fceb2 at pc 0x00083abc bp 0xb47fcd84 sp 0xb47fc950
WRITE of size 55 at 0xb47fceb2 thread T1
    #0 0x83abb in vsprintf (/home/pi/remote-debugging/aqualinkd+0x83abb)
    #1 0x83bbb in sprintf (/home/pi/remote-debugging/aqualinkd+0x83bbb)
    #2 0x12715b in action_web_request C:\git\AqualinkD/net_services.c:1163
    #3 0x128933 in ev_handler C:\git\AqualinkD/net_services.c:1297
    #4 0x14f18b in mg_call C:\git\AqualinkD/mongoose.c:2258
    #5 0x1719c7 in mg_http_call_endpoint_handler C:\git\AqualinkD/mongoose.c:8463
    #6 0x166927 in mg_http_handler C:\git\AqualinkD/mongoose.c:6374
    #7 0x14f18b in mg_call C:\git\AqualinkD/mongoose.c:2258
    #8 0x153c5b in mg_recv_common C:\git\AqualinkD/mongoose.c:2706
    #9 0x153cab in mg_if_recv_tcp_cb C:\git\AqualinkD/mongoose.c:2710
    #10 0x159a2f in mg_handle_tcp_read C:\git\AqualinkD/mongoose.c:3608
    #11 0x15a647 in mg_mgr_handle_conn C:\git\AqualinkD/mongoose.c:3733
    #12 0x15c3d7 in mg_socket_if_poll C:\git\AqualinkD/mongoose.c:3925
    #13 0x1513bb in mg_mgr_poll C:\git\AqualinkD/mongoose.c:2424
    #14 0x12a247 in net_services_thread C:\git\AqualinkD/net_services.c:1541

Address 0xb47fceb2 is located in stack of thread T1 at offset 146 in frame
    #0 0x1261c3 in action_web_request C:\git\AqualinkD/net_services.c:1043

  This frame has 3 object(s):
    [32, 36) 'msg'
    [96, 146) 'buf'
    [192, 5312) 'message' <== Memory access at offset 146 partially underflows 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)
Thread T1 created by T0 here:
    #0 0x2b54f in __interceptor_pthread_create (/home/pi/remote-debugging/aqualinkd+0x2b54f)
    #1 0x12a43b in start_net_services C:\git\AqualinkD/net_services.c:1566
    #2 0xfd0bf in main_loop C:\git\AqualinkD/aqualinkd.c:1476
    #3 0xfc39b in main C:\git\AqualinkD/aqualinkd.c:1245
    #4 0xb6cef677 in __libc_start_main (/lib/arm-linux-gnueabihf/libc.so.6+0x16677)

SUMMARY: AddressSanitizer: stack-buffer-overflow (/home/pi/remote-debugging/aqualinkd+0x83abb) in vsprintf
Shadow bytes around the buggy address:
  0x368ff980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x368ff990: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x368ff9a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x368ff9b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x368ff9c0: 00 00 00 00 f1 f1 f1 f1 04 f4 f4 f4 f2 f2 f2 f2
=>0x368ff9d0: 00 00 00 00 00 00[02]f4 f2 f2 f2 f2 00 00 00 00
  0x368ff9e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x368ff9f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x368ffa00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x368ffa10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x368ffa20: 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
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  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
==23335==ABORTING
logout
doubliez commented 1 year ago

I encountered the same issue and fixed it by increasing the size of the buffer from 50 to 200 as suggested in https://github.com/sfeakes/AqualinkD/pull/136

sfeakes commented 1 year ago

This part of the code shouldn't even be compiled. Problem was fixed in later releases.