Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak when processing cookies with multiple paths #1932

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

Memory leak when processing cookies with multiple paths #1932

cmeister2 opened this issue Sep 30, 2017 · 0 comments

Comments

@cmeister2
Copy link
Contributor

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
... this will let the second occurance override the first.

Reported-by: Max Dymond
Fixes #1932
@bagder bagder closed this as completed 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.
Development

No branches or pull requests

2 participants