tatsuhiro-t/spdylay

Memory Leaks in spdylay 1.3.2

Opened this issue · 9 comments

Deleted user commented

Hi,

there are two possible memory leaks in spdylay 1.3.2, found with afl-clang 1.83b (clang 3.5.2)

./end_to_end.py
md32_common.h:322:2: runtime error: store to misaligned address 0x62a00000029a for type 'unsigned int', which requires 4 byte alignment
0x62a00000029a: note: pointer points here
 2a e2  26 ea 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00
              ^
error:140770FC:SSL routines:SSL23_GET_SERVER_HELLO:unknown protocol
Could not connect to the host: localhost:79

=================================================================
==19917==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 288 byte(s) in 2 object(s) allocated from:
    #0 0x4430fb (/home/daniel/code/spdylay/spdylay-1.3.2.afl/src/.libs/lt-spdycat+0x4430fb)
    #1 0x545891 (/home/daniel/code/spdylay/spdylay-1.3.2.afl/src/.libs/lt-spdycat+0x545891)
    #2 0x539380 (/home/daniel/code/spdylay/spdylay-1.3.2.afl/src/.libs/lt-spdycat+0x539380)
    #3 0x53f585 (/home/daniel/code/spdylay/spdylay-1.3.2.afl/src/.libs/lt-spdycat+0x53f585)
    #4 0x7feb5a76db44 (/lib/x86_64-linux-gnu/libc.so.6+0x21b44)

Indirect leak of 93 byte(s) in 2 object(s) allocated from:
    #0 0x4430fb (/home/daniel/code/spdylay/spdylay-1.3.2.afl/src/.libs/lt-spdycat+0x4430fb)
    #1 0x7feb5adca1e8 (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0xbf1e8)
    #2 0x53f585 (/home/daniel/code/spdylay/spdylay-1.3.2.afl/src/.libs/lt-spdycat+0x53f585)
    #3 0x7feb5a76db44 (/lib/x86_64-linux-gnu/libc.so.6+0x21b44)

Indirect leak of 31 byte(s) in 1 object(s) allocated from:
    #0 0x4430fb (/home/daniel/code/spdylay/spdylay-1.3.2.afl/src/.libs/lt-spdycat+0x4430fb)
    #1 0x7feb5adca1e8 (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0xbf1e8)

SUMMARY: AddressSanitizer: 412 byte(s) leaked in 5 allocation(s).
FCould not connect to the host: localhost:2
.Requests to localhost:9893 timed out.
Some requests were not processed. total=1, processed=0
...md32_common.h:322:2: runtime error: store to misaligned address 0x62a00000029a for type 'unsigned int', which requires 4 byte alignment
0x62a00000029a: note: pointer points here
 8b 56  65 99 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00
              ^
.
======================================================================
FAIL: testFailedRequests (__main__.EndToEndSpdy2Tests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./end_to_end.py", line 82, in testFailedRequests
    2, self.call('/', ['https://localhost:25/', 'http://localhost:79']))
AssertionError: 2 != 23

----------------------------------------------------------------------
Ran 6 tests in 7.134s

FAILED (failures=1)

./failmalloc


     CUnit - A unit testing framework for C - Version 2.1-2
     http://cunit.sourceforge.net/


Suite: libspdylay_TestSuite
  Test: failmalloc_session_send ...=================================================================
==20263==ERROR: AddressSanitizer: attempting free on address which was not malloc()-ed: 0x000002c059e0 in thread T0
    #0 0x49db6b (/home/daniel/code/spdylay/spdylay-1.3.2.afl/tests/failmalloc+0x49db6b)
    #1 0x4f57c4 (/home/daniel/code/spdylay/spdylay-1.3.2.afl/tests/failmalloc+0x4f57c4)
    #2 0x5dbdff (/home/daniel/code/spdylay/spdylay-1.3.2.afl/tests/failmalloc+0x5dbdff)
    #3 0x532bf3 (/home/daniel/code/spdylay/spdylay-1.3.2.afl/tests/failmalloc+0x532bf3)
    #4 0x54dd6a (/home/daniel/code/spdylay/spdylay-1.3.2.afl/tests/failmalloc+0x54dd6a)
    #5 0x4bd360 (/home/daniel/code/spdylay/spdylay-1.3.2.afl/tests/failmalloc+0x4bd360)
    #6 0x4bbf6f (/home/daniel/code/spdylay/spdylay-1.3.2.afl/tests/failmalloc+0x4bbf6f)
    #7 0x7fab68e4d264 (/usr/lib/libcunit.so.1+0x4264)
    #8 0x7fab68e4d592 (/usr/lib/libcunit.so.1+0x4592)
    #9 0x7fab68e4d8a5 (/usr/lib/libcunit.so.1+0x48a5)
    #10 0x4bb74e (/home/daniel/code/spdylay/spdylay-1.3.2.afl/tests/failmalloc+0x4bb74e)
    #11 0x7fab67f6eb44 (/lib/x86_64-linux-gnu/libc.so.6+0x21b44)
    #12 0x4bb45c (/home/daniel/code/spdylay/spdylay-1.3.2.afl/tests/failmalloc+0x4bb45c)

AddressSanitizer can not describe address in more detail (wild memory access suspected).
SUMMARY: AddressSanitizer: bad-free ??:0 ??
==20263==ABORTING

Both do not seem to have impact on security.

tatsuhiro-t commented

Thank you for reporting these issues.
For the first one, I have not paid much attention to the "leak" from short lived command-line client when it failed. If leak comes from libspdylay, then it is more serious, so I need to check more in this case.

The second issue, this is known issue with address sanitizer. I'll look into it.

tatsuhiro-t commented

The second issue is due to that fact that we don't use malloc offered by ASAN. Therefore, ASAN complaints that it is not malloc-ed from their malloc(). This is a limitation of how we memory related tests in spdylay. We fixed this properly in nghttp2.

Deleted user commented

Yes, I noticed that nghttp2 passes all tests nicely, there's not even the need for --disable-failmalloc there, which should instead go to spdylay :-)

tatsuhiro-t commented

I fixed first issue in 6af6c50. It is not libspdylay issue, we have not cleaned up memory in spdycat command line tool.

It would be good to port nghttp2 mechanism to spdylay. But apparently libspdylay has no memory related issues, and the porting requires custom memory allocator support.
Given that SPDY is going to be deprecated in coming years, this is low priority for now.

Deleted user commented

I think backporting nghttp2 mechanisms is not worth the effort, since it's not a security related issue. I'm just using spdylay to provide spdy/3.1 protocol support for clients who don't update their browser to a [r|d]ecent version with h2 protocol support.

tatsuhiro-t commented

Thanks. The browser which supports spdy most likely uses automatic upgrade mechanism, so I hope they are just upgraded to h2.

Deleted user commented

Hi,

I was giving this further testing, and noticed the following two one issues:

1.
Edit: nevermind, that's an libressl issue

2. Running spdycat against localhost (or any gibberish input) w/o further options:

/opt/afl/spdylay/bin > ./spdycat localhost
ASAN:SIGSEGV
=================================================================
==23270==ERROR: AddressSanitizer: SEGV on unknown address 0x604000012ed3 (pc 0x7efcb9b7a318 bp 0x7ffe17fc6bf0 sp 0x7ffe17fc6370 T0)
    #0 0x7efcb9b7a317 (/opt/afl/spdylay/bin/spdycat+0x239317)
    #1 0x7efcb9ab8afa (/opt/afl/spdylay/bin/spdycat+0x177afa)
    #2 0x7efcb9ac0ad9 (/opt/afl/spdylay/bin/spdycat+0x17fad9)
    #3 0x7efcb9ac2306 (/opt/afl/spdylay/bin/spdycat+0x181306)
    #4 0x7efcb504eb44 (/lib/x86_64-linux-gnu/libc.so.6+0x21b44)
    #5 0x7efcb9a4278c (/opt/afl/spdylay/bin/spdycat+0x10178c)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV ??:0 ??
==23270==ABORTING

whoops. Let's try again:

/opt/afl/spdylay/bin > ./spdycat asdasd
=================================================================
==23275==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60400000eed3 at pc 0x7fa4d800a900 bp 0x7ffd9c1dfb20 sp 0x7ffd9c1dfb18
READ of size 1 at 0x60400000eed3 thread T0
    #0 0x7fa4d800a8ff (/opt/afl/spdylay/bin/spdycat+0x23d8ff)
    #1 0x7fa4d7f44afa (/opt/afl/spdylay/bin/spdycat+0x177afa)
    #2 0x7fa4d7f4cad9 (/opt/afl/spdylay/bin/spdycat+0x17fad9)
    #3 0x7fa4d7f4e306 (/opt/afl/spdylay/bin/spdycat+0x181306)
    #4 0x7fa4d34dab44 (/lib/x86_64-linux-gnu/libc.so.6+0x21b44)
    #5 0x7fa4d7ece78c (/opt/afl/spdylay/bin/spdycat+0x10178c)

AddressSanitizer can not describe address in more detail (wild memory access suspected).
SUMMARY: AddressSanitizer: heap-buffer-overflow ??:0 ??
Shadow bytes around the buggy address:
  0x0c087fff9d80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c087fff9d90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c087fff9da0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c087fff9db0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c087fff9dc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c087fff9dd0: fa fa fa fa fa fa fa fa fa fa[fa]fa fa fa fa fa
  0x0c087fff9de0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c087fff9df0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c087fff9e00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c087fff9e10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c087fff9e20: 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
  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
  ASan internal:           fe
==23275==ABORTING

This does not happen when:

  • using anything: as URI (no output)
  • prefixing URI with scheme https:// (this seems to be required to receive output or make getaddrinfo() fail)
tatsuhiro-t commented

Thank you for reporting this issue. It looks like http-parser issue. I updated http-parser, and the bug went away. Check out commit d9fdf16

Deleted user commented

Confirmed!