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

Segfault in curl_easy_perform #3548

Closed
renaudallard opened this issue Feb 11, 2019 · 5 comments

Comments

Projects
None yet
3 participants
@renaudallard
Copy link

commented Feb 11, 2019

I did this

Installed esniper on OpenBSD 6.4-current, which is using curl_easy_perform.

I expected the following

No error, but instead I got:
(gdb) run
Starting program: /usr/ports/pobj/esniper-2.35.0/esniper-2-35-0/esniper -m

Program received signal SIGSEGV, Segmentation fault.
strlen () at /usr/src/lib/libc/arch/amd64/string/strlen.S:125
125 /usr/src/lib/libc/arch/amd64/string/strlen.S: No such file or directory.
in /usr/src/lib/libc/arch/amd64/string/strlen.S
Current language: auto; currently asm
(gdb) bt
#0 strlen () at /usr/src/lib/libc/arch/amd64/string/strlen.S:125
#1 0x000004a2edf3e9e5 in Curl_pretransfer (data=0x4a270901008) at transfer.c:1406
#2 0x000004a2edf4eb9a in multi_runsingle (multi=0x4a2b2eb7808, now={tv_sec = 96041, tv_usec = 578763},
data=0x4a270901008) at multi.c:1441
#3 0x000004a2edf4e3ca in curl_multi_perform (multi=0x4a2b2eb7808, running_handles=0x7f7ffffdc48c) at multi.c:2214
#4 0x000004a2edf41d25 in easy_transfer (multi=0x4a2b2eb7808) at easy.c:686
#5 0x000004a2edf4074e in easy_perform (data=0x4a270901008, events=false) at easy.c:780
#6 0x000004a2edf40563 in curl_easy_perform (data=0x4a270901008) at easy.c:799
#7 0x000004a0606dae68 in httpRequest (
url=0x4a343e38880 "https://my.ebay.com/ws/eBayISAPI.dll?MyeBay&CurrentPage=MyeBayWatching", logUrl=0x0,
data=0x4a0606c9a49 "", logData=0x0, rt=GET) at http.c:177
#8 0x000004a0606dab7a in httpGet (
url=0x4a343e38880 "https://my.ebay.com/ws/eBayISAPI.dll?MyeBay&CurrentPage=MyeBayWatching", logUrl=0x0)
at http.c:79
#9 0x000004a0606cfdec in printMyItems () at auction.c:1217
#10 0x000004a0606d4df2 in main (argc=0, argv=0x7f7ffffdc858) at esniper.c:850

This is referenced in https://sourceforge.net/p/esniper/bugs/767

The executed action is a GET, but it segfaults if CURLOPT_POSTFIELDSIZE is not set to 0. To my knowledge, it should not even go there if the request is a GET.

curl/libcurl version

curl 7.63.0 (x86_64-unknown-openbsd6.4) libcurl/7.63.0 LibreSSL/2.9.0 zlib/1.2.3 nghttp2/1.36.0
Release-Date: 2018-12-12
Protocols: dict file ftp ftps gopher http https imap imaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: AsynchDNS Debug TrackMemory IPv6 Largefile NTLM NTLM_WB SSL libz HTTP2 UnixSockets HTTPS-proxy

operating system

OpenBSD 6.4-current (also segfaults in OpenBSD 6.4)

@bagder

This comment has been minimized.

Copy link
Member

commented Feb 11, 2019

If you set an option like CURLOPT_POSTFIELDS to point to something bad, I don't think you should assume that libcurl won't use it simply because you have also set CURLOPT_HTTPGET.

I agree that ideally libcurl should just ignore that pointer in this case, but I don't think that's a good reason for an application to keep a stale pointer set for curl. PR coming up.

bagder added a commit that referenced this issue Feb 11, 2019

pretransfer: don't strlen() POSTFIELDS set for GET requests
... since that data won't be used in the request anyway.

Fixes #3548
Reported-by: Renaud Allard
@VMbonn

This comment has been minimized.

Copy link

commented Feb 11, 2019

Just to clarify: The original code does not contain a POSTFIELDS !

if (rt == GET) {
if ((curlrc = curl_easy_setopt(easyhandle, CURLOPT_HTTPGET, 1)))
return httpRequestFailed(mp);
} else {
log(("%s", logData ? logData : nonNullData));
if ((curlrc = curl_easy_setopt(easyhandle, CURLOPT_POSTFIELDS, nonNullData)))
return httpRequestFailed(mp);
}

This code crashes on BSD.

renaudallard then inserted

if ((curlrc = curl_easy_setopt(easyhandle, CURLOPT_POSTFIELDSIZE, 0)))
return httpRequestFailed(mp);

to solve those crashes (https://sourceforge.net/p/esniper/bugs/767/)

@bagder

This comment has been minimized.

Copy link
Member

commented Feb 11, 2019

The original code does not contain a POSTFIELDS !

That's even more weird. curl allocates the entire handle with calloc so it is cleared from the beginning and thus the pointer is NULLed internally from the beginning.

So if you can provide a smaller libcurl example that can make this happen without setting POSTFIELDS then I'm very curious!

@VMbonn

This comment has been minimized.

Copy link

commented Feb 11, 2019

Maybe the reason for the segmentation fault is, that a previous call to the function 'httpRequest' uses the requerstType = POST. The pointer set by this previous call using CURLOPT_POSTFIELDS is only valid during is call and was been freed by the caller. But from my point of view an invalid pointer to CURLOPT_POSTFIELDS must not have any impact to calls using GET.

@bagder

This comment has been minimized.

Copy link
Member

commented Feb 11, 2019

from my point of view an invalid pointer to CURLOPT_POSTFIELDS must not have any impact to calls using GET.

That's a very slippery slope and I will not make any guarantees that this is safe. #3549 tries to make it better, but passing in a stale pointer to curl is wrong and bad no matter which option combination you're using.

@bagder bagder closed this in a6d134e Feb 12, 2019

@lock lock bot locked as resolved and limited conversation to collaborators May 13, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.