Memory leak when processing cookies with multiple paths #1932

Closed
cmeister2 opened this Issue Sep 30, 2017 · 0 comments

Comments

Projects
None yet
2 participants
@cmeister2
Contributor

cmeister2 commented Sep 30, 2017

I did this

Added a test case to curl_fuzzer with a line:

Set-Cookie: ckyPersistent=permanent;path=;path=/

I expected the following

Not to leak memory.

root@kali:~/checkouts/curl-fuzzer# ./curl_fuzzer leaktest 
[leaktest] Open succeeded! 
[leaktest] Read 197 bytes, calling fuzzer
* STATE: INIT => CONNECT handle 0x62a000000208; line 1422 (connection #-5000)
* Added connection 0. The cache now contains 1 members
*   Trying 127.0.0.1...
* Could not set TCP_NODELAY: Operation not supported
* STATE: CONNECT => WAITCONNECT handle 0x62a000000208; line 1474 (connection #0)
* Connected to 127.0.0.1 (���) port 8990 (#0)
* STATE: WAITCONNECT => SENDPROTOCONNECT handle 0x62a000000208; line 1591 (connection #0)
* Marked for [keep alive]: HTTP default
* STATE: SENDPROTOCONNECT => DO handle 0x62a000000208; line 1609 (connection #0)
> GET /want/46 HTTP/1.1
Host: 127.0.0.1:8990
Accept: */*

* STATE: DO => DO_DONE handle 0x62a000000208; line 1688 (connection #0)
* STATE: DO_DONE => WAITPERFORM handle 0x62a000000208; line 1813 (connection #0)
* STATE: WAITPERFORM => PERFORM handle 0x62a000000208; line 1823 (connection #0)
* HTTP 1.1 or later with persistent connection, pipelining supported
< HTTP/1.1 200 OK
* Server Microsoft-IIS/4.0 is not blacklisted
< Server: Microsoft-IIS/4.0
< Date: Tue, 25 Sep 2001 19:37:44 GMT
< Content-Type: text/html
* cookie size: name/val 13 + 9 bytes
* cookie size: name/val 4 + 0 bytes
* cookie size: name/val 4 + 1 bytes
* Added cookie ckyPersistent="permanent" for domain 127.0.0.1, path /, expire 0
< Set-Cookie: ckyPersistent=permanent;path=;path=/
* STATE: PERFORM => DONE handle 0x62a000000208; line 1992 (connection #0)
* multi_done
* Connection #0 to host 127.0.0.1 left intact
* Expire cleared
[leaktest] Fuzzing complete

=================================================================
==10525==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 18 byte(s) in 1 object(s) allocated from:
    #0 0x4bcc98  (/root/checkouts/curl-fuzzer/curl_fuzzer+0x4bcc98)
    #1 0x4f9ad3  (/root/checkouts/curl-fuzzer/curl_fuzzer+0x4f9ad3)
    #2 0x4fa331  (/root/checkouts/curl-fuzzer/curl_fuzzer+0x4fa331)
    #3 0x54ceab  (/root/checkouts/curl-fuzzer/curl_fuzzer+0x54ceab)
    #4 0x549ca0  (/root/checkouts/curl-fuzzer/curl_fuzzer+0x549ca0)
    #5 0x56842e  (/root/checkouts/curl-fuzzer/curl_fuzzer+0x56842e)
    #6 0x5d82be  (/root/checkouts/curl-fuzzer/curl_fuzzer+0x5d82be)
    #7 0x5d6181  (/root/checkouts/curl-fuzzer/curl_fuzzer+0x5d6181)
    #8 0x511184  (/root/checkouts/curl-fuzzer/curl_fuzzer+0x511184)
    #9 0x50c6b4  (/root/checkouts/curl-fuzzer/curl_fuzzer+0x50c6b4)
    #10 0x4f8062  (/root/checkouts/curl-fuzzer/curl_fuzzer+0x4f8062)
    #11 0x4f4ede  (/root/checkouts/curl-fuzzer/curl_fuzzer+0x4f4ede)
    #12 0x4f4b16  (/root/checkouts/curl-fuzzer/curl_fuzzer+0x4f4b16)
    #13 0x4f0fcb  (/root/checkouts/curl-fuzzer/curl_fuzzer+0x4f0fcb)
    #14 0x6a6b75  (/root/checkouts/curl-fuzzer/curl_fuzzer+0x6a6b75)
    #15 0x7fdf11cc22b0  (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)

SUMMARY: AddressSanitizer: 18 byte(s) leaked in 1 allocation(s).

Running through asan_sanitizer it looks like the offending code is in cookie.c:502:

          co->spath = sanitize_cookie_path(co->path);

where this variable is overwritten without checking whether co->spath has already been allocated before.

curl/libcurl version

devel

bagder added a commit that referenced this issue Sep 30, 2017

cookie: fix memory leak if path was set twice in header
... this will let the second occurance override the first.

Reported-by: Max Dymond
Fixes #1932

@bagder bagder closed this in 8392a0c Sep 30, 2017

@lock lock bot locked as resolved and limited conversation to collaborators May 6, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.