1
0
mirror of https://github.com/moparisthebest/curl synced 2024-12-25 09:38:54 -05:00
Commit Graph

542 Commits

Author SHA1 Message Date
Even Rouault
64cbae3107
multi_runsingle(): fix use-after-free
Fixes #3745
Closes #3746

The following snippet
```

int main()
{
    CURL* hCurlHandle = curl_easy_init();
    curl_easy_setopt(hCurlHandle, CURLOPT_URL, "http://example.com");
    curl_easy_setopt(hCurlHandle, CURLOPT_PROXY, "1");
    curl_easy_perform(hCurlHandle);
    curl_easy_cleanup(hCurlHandle);
    return 0;
}
```
triggers the following Valgrind warning

```
==4125== Invalid read of size 8
==4125==    at 0x4E7D1EE: Curl_llist_remove (llist.c:97)
==4125==    by 0x4E7EF5C: detach_connnection (multi.c:798)
==4125==    by 0x4E80545: multi_runsingle (multi.c:1451)
==4125==    by 0x4E8197C: curl_multi_perform (multi.c:2072)
==4125==    by 0x4E766A0: easy_transfer (easy.c:625)
==4125==    by 0x4E76915: easy_perform (easy.c:719)
==4125==    by 0x4E7697C: curl_easy_perform (easy.c:738)
==4125==    by 0x4008BE: main (in /home/even/curl/test)
==4125==  Address 0x9b3d1d0 is 1,120 bytes inside a block of size 1,600 free'd
==4125==    at 0x4C2ECF0: free (vg_replace_malloc.c:530)
==4125==    by 0x4E62C36: conn_free (url.c:756)
==4125==    by 0x4E62D34: Curl_disconnect (url.c:818)
==4125==    by 0x4E48DF9: Curl_once_resolved (hostip.c:1097)
==4125==    by 0x4E8052D: multi_runsingle (multi.c:1446)
==4125==    by 0x4E8197C: curl_multi_perform (multi.c:2072)
==4125==    by 0x4E766A0: easy_transfer (easy.c:625)
==4125==    by 0x4E76915: easy_perform (easy.c:719)
==4125==    by 0x4E7697C: curl_easy_perform (easy.c:738)
==4125==    by 0x4008BE: main (in /home/even/curl/test)
==4125==  Block was alloc'd at
==4125==    at 0x4C2F988: calloc (vg_replace_malloc.c:711)
==4125==    by 0x4E6438E: allocate_conn (url.c:1654)
==4125==    by 0x4E685B4: create_conn (url.c:3496)
==4125==    by 0x4E6968F: Curl_connect (url.c:4023)
==4125==    by 0x4E802E7: multi_runsingle (multi.c:1368)
==4125==    by 0x4E8197C: curl_multi_perform (multi.c:2072)
==4125==    by 0x4E766A0: easy_transfer (easy.c:625)
==4125==    by 0x4E76915: easy_perform (easy.c:719)
==4125==    by 0x4E7697C: curl_easy_perform (easy.c:738)
==4125==    by 0x4008BE: main (in /home/even/curl/test)
```

This has been bisected to commit 2f44e94

Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=14109
Credit to OSS Fuzz
2019-04-07 22:55:38 +02:00
Daniel Stenberg
2f44e94efb
pipelining: removed
As previously planned and documented in DEPRECATE.md, all pipelining
code is removed.

Closes #3651
2019-04-06 22:49:50 +02:00
Daniel Stenberg
dbce1bd86f
multi: improved HTTP_1_1_REQUIRED handling
Make sure to downgrade to 1.1 even when we get this HTTP/2 stream error
on first flight.

Reported-by: niner on github
Fixes #3696
Closes #3707
2019-03-27 23:28:14 +01:00
Dominik Hölzl
6c60355323
Negotiate: fix for HTTP POST with Negotiate
* Adjusted unit tests 2056, 2057
* do not generally close connections with CURLAUTH_NEGOTIATE after every request
* moved negotiatedata from UrlState to connectdata
* Added stream rewind logic for CURLAUTH_NEGOTIATE
* introduced negotiatedata::GSS_AUTHDONE and negotiatedata::GSS_AUTHSUCC
* Consider authproblem state for CURLAUTH_NEGOTIATE
* Consider reuse_forbid for CURLAUTH_NEGOTIATE
* moved and adjusted negotiate authentication state handling from
  output_auth_headers into Curl_output_negotiate
* Curl_output_negotiate: ensure auth done is always set
* Curl_output_negotiate: Set auth done also if result code is
  GSS_S_CONTINUE_NEEDED/SEC_I_CONTINUE_NEEDED as this result code may
  also indicate the last challenge request (only works with disabled
  Expect: 100-continue and CURLOPT_KEEP_SENDING_ON_ERROR -> 1)
* Consider "Persistent-Auth" header, detect if not present;
  Reset/Cleanup negotiate after authentication if no persistent
  authentication
* apply changes introduced with #2546 for negotiate rewind logic

Fixes #1261
Closes #1975
2019-03-14 09:26:03 +01:00
Daniel Stenberg
0bb56392d4
multi: removed unused code for request retries
This code was once used for the non multi-interface using code path, but
ever since easy_perform was turned into a wrapper around the multi
interface, this code path never runs.

Closes #3666
2019-03-11 17:29:51 +01:00
Jay Satiro
b0972bc8e1 multi: support verbose conncache closure handle
- Change closure handle to receive verbose setting from the easy handle
  most recently added via curl_multi_add_handle.

The closure handle is a special easy handle used for closing cached
connections. It receives limited settings from the easy handle most
recently added to the multi handle. Prior to this change that did not
include verbose which was a problem because on connection shutdown
verbose mode was not acknowledged.

Ref: https://github.com/curl/curl/pull/3598

Co-authored-by: Daniel Stenberg

Closes https://github.com/curl/curl/pull/3618
2019-03-05 00:00:01 -05:00
Daniel Stenberg
4ff055530d
Curl_easy: remove req.maxfd - never used!
Introduced in 8b6314ccfb, but not used anymore in current code. Unclear
since when.

Closes #3626
2019-02-28 15:27:32 +01:00
Daniel Stenberg
aa7b813ad1
multi: call multi_done on connect timeouts
Failing to do so would make the CURLINFO_TOTAL_TIME timeout to not get
updated correctly and could end up getting reported to the application
completely wrong (way too small).

Reported-by: accountantM on github
Fixes #3602
Closes #3605
2019-02-24 22:56:46 +01:00
Daniel Stenberg
afc00e047c
singlesocket: fix the 'sincebefore' placement
The variable wasn't properly reset within the loop and thus could remain
set for sockets that hadn't been set before and miss notifying the app.

This is a follow-up to 4c35574 (shipped in curl 7.64.0)

Reported-by: buzo-ffm on github
Detected-by: Jan Alexander Steffens
Fixes #3585
Closes #3589
2019-02-20 08:15:32 +01:00
Daniel Stenberg
bb2444b794
multi: Dereference of null pointer
Mostly a false positive, but this makes the code easier to read anyway.

Detected by scan-build.

Closes #3563
2019-02-13 08:06:35 +01:00
Daniel Stenberg
61496154ce
http2: multi_connchanged() moved from multi.c, only used for h2
Closes #3557
2019-02-12 14:40:37 +01:00
Daniel Stenberg
aabc7ae5ec
multi: remove verbose "Expire in" ... messages
Reported-by: James Brown
Bug: https://curl.haxx.se/mail/archive-2019-02/0013.html
Closes #3558
2019-02-12 14:13:15 +01:00
Daniel Stenberg
982c09b95b
multi: (void)-prefix when ignoring return values
... and added braces to two function calls which fixes warnings if they
are replace by empty macros at build-time.
2019-02-11 11:57:02 +01:00
Daniel Stenberg
05b100aee2
cleanup: make local functions static
urlapi: turn three local-only functions into statics

conncache: make conncache_find_first_connection static

multi: make detach_connnection static

connect: make getaddressinfo static

curl_ntlm_core: make hmac_md5 static

http2: make two functions static

http: make http_setup_conn static

connect: make tcpnodelay static

tests: make UNITTEST a thing to mark functions with, so they can be static for
normal builds and non-static for unit test builds

... and mark Curl_shuffle_addr accordingly.

url: make up_free static

setopt: make vsetopt static

curl_endian: make write32_le static

rtsp: make rtsp_connisdead static

warnless: remove unused functions

memdebug: remove one unused function, made another static
2019-02-10 18:38:57 +01:00
Daniel Stenberg
6f61933adf
multi: set the EXPIRE_*TIMEOUT timers at TIMER_STARTSINGLE time
To make sure Curl_timeleft() also thinks the timeout has been reached
when one of the EXPIRE_*TIMEOUTs expires.

Bug: https://curl.haxx.se/mail/lib-2019-01/0073.html
Reported-by: Zhao Yisha
Closes #3501
2019-02-01 08:19:40 +01:00
Daniel Stenberg
fe71b2d928
multi: remove "Dead assignment"
Found by scan-build. Follow-up to 4c35574bb7.

Closes #3471
2019-01-15 08:10:17 +01:00
Daniel Stenberg
ba243235ec
urldata: rename easy_conn to just conn
We use "conn" everywhere to be a pointer to the connection.

Introduces two functions that "attaches" and "detaches" the connection
to and from the transfer.

Going forward, we should favour using "data->conn" (since a transfer
always only has a single connection or none at all) to "conn->data"
(since a connection can have none, one or many transfers associated with
it and updating conn->data to be correct is error prone and a frequent
reason for internal issues).

Closes #3442
2019-01-11 15:35:13 +01:00
Daniel Stenberg
4c35574bb7
multi: multiplexing improvements
Fixes #3436
Closes #3448

 Problem 1

After LOTS of scratching my head, I eventually realized that even when doing
10 uploads in parallel, sometimes the socket callback to the application that
tells it what to wait for on the socket, looked like it would reflect the
status of just the single transfer that just changed state.

Digging into the code revealed that this was indeed the truth. When multiple
transfers are using the same connection, the application did not correctly get
the *combined* flags for all transfers which then could make it switch to READ
(only) when in fact most transfers wanted to get told when the socket was
WRITEABLE.

 Problem 1b

A separate but related regression had also been introduced by me when I
cleared connection/transfer association better a while ago, as now the logic
couldn't find the connection and see if that was marked as used by more
transfers and then it would also prematurely remove the socket from the socket
hash table even in times other transfers were still using it!

 Fix 1

Make sure that each socket stored in the socket hash has a "combined" action
field of what to ask the application to wait for, that is potentially the ORed
action of multiple parallel transfers. And remove that socket hash entry only
if there are no transfers left using it.

 Problem 2

The socket hash entry stored an association to a single transfer using that
socket - and when curl_multi_socket_action() was called to tell libcurl about
activities on that specific socket only that transfer was "handled".

This was WRONG, as a single socket/connection can be used by numerous parallel
transfers and not necessarily a single one.

 Fix 2

We now store a list of handles in the socket hashtable entry and when libcurl
is told there's traffic for a particular socket, it now iterates over all
known transfers using that single socket.
2019-01-10 09:49:09 +01:00
Brad Spencer
84a30d0a41
curl_multi_remove_handle() don't block terminating c-ares requests
Added Curl_resolver_kill() for all three resolver modes, which only
blocks when necessary, along with test 1592 to confirm
curl_multi_remove_handle() doesn't block unless it must.

Closes #3428
Fixes #3371
2019-01-07 10:05:20 +01:00
Rikard Falkeborn
1e4ec09a26
printf: introduce CURL_FORMAT_TIMEDIFF_T 2019-01-04 23:51:13 +01:00
Daniel Stenberg
fb445a1e18
disconnect: separate connections and easy handles better
Do not assume/store assocation between a given easy handle and the
connection if it can be avoided.

Long-term, the 'conn->data' pointer should probably be removed as it is a
little too error-prone. Still used very widely though.

Reported-by: masbug on github
Fixes #3391
Closes #3400
2018-12-22 18:14:50 +01:00
Daniel Stenberg
ecb2e194d4
multi: convert two timeout variables to timediff_t
The time_t type is unsigned on some systems and these variables are used
to hold return values from functions that return timediff_t
already. timediff_t is always a signed type.

Closes #3363
2018-12-11 15:43:20 +01:00
Johannes Schindelin
d997aa0e96
Upon HTTP_1_1_REQUIRED, retry the request with HTTP/1.1
This is a companion patch to cbea2fd2c (NTLM: force the connection to
HTTP/1.1, 2018-12-06): with NTLM, we can switch to HTTP/1.1
preemptively. However, with other (Negotiate) authentication it is not
clear to this developer whether there is a way to make it work with
HTTP/2, so let's try HTTP/2 first and fall back in case we encounter the
error HTTP_1_1_REQUIRED.

Note: we will still keep the NTLM workaround, as it avoids an extra
round trip.

Daniel Stenberg helped a lot with this patch, in particular by
suggesting to introduce the Curl_h2_http_1_1_error() function.

Closes #3349

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2018-12-08 10:59:23 +01:00
Daniel Stenberg
34fe0e1622
curl_easy_perform: fix timeout handling
curl_multi_wait() was erroneously used from within
curl_easy_perform(). It could lead to it believing there was no socket
to wait for and then instead sleep for a while instead of monitoring the
socket and then miss acting on that activity as swiftly as it should
(causing an up to 1000 ms delay).

Reported-by: Antoni Villalonga
Fixes #3305
Closes #3306
Closes #3308
2018-11-25 11:25:43 +01:00
Daniel Stenberg
dcd6f81025
snprintf: renamed and we now only use msnprintf()
The function does not return the same value as snprintf() normally does,
so readers may be mislead into thinking the code works differently than
it actually does. A different function name makes this easier to detect.

Reported-by: Tomas Hoger
Assisted-by: Daniel Gustafsson
Fixes #3296
Closes #3297
2018-11-23 08:26:51 +01:00
Romain Fliedel
6765e6d9e6
ares: remove fd from multi fd set when ares is about to close the fd
When using c-ares for asyn dns, the dns socket fd was silently closed
by c-ares without curl being aware. curl would then 'realize' the fd
has been removed at next call of Curl_resolver_getsock, and only then
notify the CURLMOPT_SOCKETFUNCTION to remove fd from its poll set with
CURL_POLL_REMOVE. At this point the fd is already closed.

By using ares socket state callback (ARES_OPT_SOCK_STATE_CB), this
patch allows curl to be notified that the fd is not longer needed
for neither for write nor read. At this point by calling
Curl_multi_closed we are able to notify multi with CURL_POLL_REMOVE
before the fd is actually closed by ares.

In asyn-ares.c Curl_resolver_duphandle we can't use ares_dup anymore
since it does not allow passing a different sock_state_cb_data

Closes #3238
2018-11-20 19:58:45 +01:00
Michael Kaufmann
3793761a37 multi: Fix error handling in the SENDPROTOCONNECT state
If Curl_protocol_connect() returns an error code,
handle the error instead of switching to the next state.

Closes #3170
2018-10-27 13:03:50 +02:00
Daniel Stenberg
05564e750e
multi: avoid double-free
Curl_follow() no longer frees the string. Make sure it happens in the
caller function, like we normally handle allocations.

This bug was introduced with the use of the URL API internally, it has
never been in a release version

Reported-by: Dario Weißer
Closes #3149
2018-10-19 15:29:31 +02:00
Daniel Stenberg
8a49f91d32
multi: make the closure handle "inherit" CURLOPT_NOSIGNAL
Otherwise, closing that handle can still cause surprises!

Reported-by: Martin Ankerl
Fixes #3138
Closes #3147
2018-10-19 11:03:17 +02:00
Michael Kaufmann
6afe70a00b Curl_follow: Always free the passed new URL
Closes #3124
2018-10-13 13:18:51 +02:00
Daniel Stenberg
2dfc0dd6b5
multi: fix memory leak in content encoding related error path
... a missing multi_done() call.

Credit to OSS-Fuzz
Bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10728
Closes #3063
2018-09-29 15:03:57 +02:00
Daniel Stenberg
454fa3fd7b
multi: fix location URL memleak in error path
Follow-up to #3044 - fix a leak OSS-Fuzz detected
Closes #3057
2018-09-28 17:10:14 +02:00
Daniel Stenberg
46e164069d
url: use the URL API internally as well
... to make it a truly unified URL parser.

Closes #3017
2018-09-22 11:58:10 +02:00
Daniel Stenberg
ec5d0991cc
curl_multi_wait: call getsock before figuring out timeout
.... since getsock may update the expiry timer.

Fixes #2996
Closes #3000
2018-09-18 15:39:28 +02:00
Daniel Stenberg
abff183387
setopt: add CURLOPT_DOH_URL
Closes #2668
2018-09-06 09:17:17 +02:00
Daniel Stenberg
d7c4213bd0
multiplex: enable by default
Starting 7.62.0, multiplexing is enabled by default in multi handles.
2018-09-05 14:35:57 +02:00
Daniel Stenberg
f7208df7d9
pipelining: deprecated
Transparently. The related curl_multi_setopt() options all still returns
OK when pipelining is selected.

To re-enable the support, the single line change in lib/multi.c needs to
be reverted.

See docs/DEPRECATE.md

Closes #2705
2018-09-05 11:34:03 +02:00
Daniel Stenberg
e6e9b006f7
upload: allocate upload buffer on-demand
Saves 16KB on the easy handle for operations that don't need that
buffer.

Part 1 of #2888
2018-08-18 15:48:08 +02:00
Daniel Stenberg
09da53998e
urldata: remove unused pipe_broke struct field
This struct field is never set TRUE in any existing code path. This
change removes the field completely.

Closes #2871
2018-08-16 09:33:12 +02:00
Daniel Jelinski
d6cf93011d
retry: return error if rewind was necessary but didn't happen
Fixes #2801
Closes #2812
2018-07-31 01:26:08 +02:00
Daniel Stenberg
acefdd0cd1
multi: always do the COMPLETED procedure/state
It was previously erroneously skipped in some situations.

libtest/libntlmconnect.c wrongly depended on wrong behavior (that it
would get a zero timeout) when no handles are "running" in a multi
handle. That behavior is no longer present with this fix. Now libcurl
will always return a -1 timeout when all handles are completed.

Closes #2733
2018-07-11 23:41:24 +02:00
Daniel Stenberg
1b76c38904
conn: remove the boolean 'inuse' field
... as the usage needs to be counted.
2018-07-11 23:41:24 +02:00
Javier Blazquez
4c901638b4
multi: fix crash due to dangling entry in connect-pending list
Fixes #2677
Closes #2679
2018-06-23 22:31:52 +02:00
Daniel Stenberg
ac9a179fe9
multi: fix memory leak when stopped during name resolve
When the application just started the transfer and then stops it while
the name resolve in the background thread hasn't completed, we need to
wait for the resolve to complete and then cleanup data accordingly.

Enabled test 1553 again and added test 1590 to also check when the host
name resolves successfully.

Detected by OSS-fuzz.
Closes #1968
2018-06-16 12:01:27 +02:00
Marian Klymov
c45360d463
cppcheck: fix warnings
- Get rid of variable that was generating false positive warning
(unitialized)

- Fix issues in tests

- Reduce scope of several variables all over

etc

Closes #2631
2018-06-11 11:14:48 +02:00
Viktor Szakats
4bd91bc474 spelling fixes
Detected using the `codespell` tool (version 1.13.0).

Also secure and fix an URL.
2018-06-03 12:14:45 +00:00
Daniel Stenberg
923ce4a590
multi: remove a DEBUGF()
... it might call infof() with a NULL first argument that isn't harmful
but makes it not do anything. The infof() line is not very useful
anymore, it has served it purpose. Good riddance!

Fixes #2627
2018-06-01 12:55:41 +02:00
Rikard Falkeborn
8ea5d41fe0
strictness: correct {infof, failf} format specifiers
Closes #2623
2018-05-31 11:34:32 +02:00
Patrick Monnerat
8541d02c96 psl: use latest psl and refresh it periodically
The latest psl is cached in the multi or share handle. It is refreshed
before use after 72 hours.
New share lock CURL_LOCK_DATA_PSL controls the psl cache sharing.
If the latest psl is not available, the builtin psl is used.

Reported-by: Yaakov Selkowitz
Fixes #2553
Closes #2601
2018-05-28 20:37:14 +02:00
Daniel Gustafsson
2f13e3d23d
checksrc: force indentation of lines after an else
This extends the INDENTATION case to also handle 'else' statements
and require proper indentation on the following line. Also fixes the
offending cases found in the codebase.

Closes #2532
2018-04-27 00:51:35 +02:00