prevent multi from crashing with many parallel transfers

Resolves: #1697566
Resolves: #1723242
This commit is contained in:
Kamil Dudka 2019-07-01 10:19:45 +02:00
parent bb93a72533
commit 0c07534eed
2 changed files with 229 additions and 1 deletions

View File

@ -0,0 +1,221 @@
From 42f31adb0386c168682d14bf1e52fbbdfc5fc36d Mon Sep 17 00:00:00 2001
From: Even Rouault <even.rouault@spatialys.com>
Date: Sun, 7 Apr 2019 14:07:35 +0200
Subject: [PATCH 1/2] 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
Upstream-commit: 64cbae31078b2b64818a1d793516fbe73a7e4c45
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
lib/multi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/multi.c b/lib/multi.c
index 856cc22..cad76c1 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -1549,7 +1549,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
if(result)
/* if Curl_once_resolved() returns failure, the connection struct
is already freed and gone */
- Curl_detach_connnection(data); /* no more connection */
+ data->conn = NULL; /* no more connection */
else {
/* call again please so that we get the next socket setup */
rc = CURLM_CALL_MULTI_PERFORM;
--
2.20.1
From 2d79804c0fab1efa594b1739a3e408c4ab031704 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Tue, 28 May 2019 08:23:43 +0200
Subject: [PATCH 2/2] multi: track users of a socket better
They need to be removed from the socket hash linked list with more care.
When sh_delentry() is called to remove a sockethash entry, remove all
individual transfers from the list first. To enable this, each Curl_easy struct
now stores a pointer to the sockethash entry to know how to remove itself.
Reported-by: Tom van der Woerdt and Kunal Ekawde
Fixes #3952
Fixes #3904
Closes #3953
Upstream-commit: 8581e1928ea8e125021408ec071fcedb0276c7fe
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
lib/multi.c | 38 ++++++++++++++++++++++++++------------
lib/urldata.h | 1 +
2 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/lib/multi.c b/lib/multi.c
index cad76c1..467cad8 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -245,8 +245,17 @@ static struct Curl_sh_entry *sh_addentry(struct curl_hash *sh,
/* delete the given socket + handle from the hash */
-static void sh_delentry(struct curl_hash *sh, curl_socket_t s)
+static void sh_delentry(struct Curl_sh_entry *entry,
+ struct curl_hash *sh, curl_socket_t s)
{
+ struct curl_llist *list = &entry->list;
+ struct curl_llist_element *e;
+ /* clear the list of transfers first */
+ for(e = list->head; e; e = list->head) {
+ struct Curl_easy *dta = e->ptr;
+ Curl_llist_remove(&entry->list, e, NULL);
+ dta->sh_entry = NULL;
+ }
/* We remove the hash entry. This will end up in a call to
sh_freeentry(). */
Curl_hash_delete(sh, (char *)&s, sizeof(curl_socket_t));
@@ -806,6 +815,11 @@ bool Curl_pipeline_wanted(const struct Curl_multi *multi, int bits)
occasionally be called with the pointer already cleared. */
void Curl_detach_connnection(struct Curl_easy *data)
{
+ if(data->sh_entry) {
+ /* still listed as a user of a socket hash entry, remove it */
+ Curl_llist_remove(&data->sh_entry->list, &data->sh_queue, NULL);
+ data->sh_entry = NULL;
+ }
data->conn = NULL;
}
@@ -2432,6 +2446,7 @@ static CURLMcode singlesocket(struct Curl_multi *multi,
/* add 'data' to the list of handles using this socket! */
Curl_llist_insert_next(&entry->list, entry->list.tail,
data, &data->sh_queue);
+ data->sh_entry = entry;
}
comboaction = (entry->writers? CURL_POLL_OUT : 0) |
@@ -2491,11 +2506,7 @@ static CURLMcode singlesocket(struct Curl_multi *multi,
multi->socket_cb(data, s, CURL_POLL_REMOVE,
multi->socket_userp,
entry->socketp);
- sh_delentry(&multi->sockhash, s);
- }
- else {
- /* remove this transfer as a user of this socket */
- Curl_llist_remove(&entry->list, &data->sh_queue, NULL);
+ sh_delentry(entry, &multi->sockhash, s);
}
}
} /* for loop over numsocks */
@@ -2539,7 +2550,7 @@ void Curl_multi_closed(struct Curl_easy *data, curl_socket_t s)
entry->socketp);
/* now remove it from the socket hash */
- sh_delentry(&multi->sockhash, s);
+ sh_delentry(entry, &multi->sockhash, s);
}
}
}
@@ -2630,7 +2641,6 @@ static CURLMcode multi_socket(struct Curl_multi *multi,
return result;
}
if(s != CURL_SOCKET_TIMEOUT) {
-
struct Curl_sh_entry *entry = sh_getentry(&multi->sockhash, s);
if(!entry)
@@ -2643,15 +2653,19 @@ static CURLMcode multi_socket(struct Curl_multi *multi,
else {
struct curl_llist *list = &entry->list;
struct curl_llist_element *e;
+ struct curl_llist_element *enext;
SIGPIPE_VARIABLE(pipe_st);
/* the socket can be shared by many transfers, iterate */
- for(e = list->head; e; e = e->next) {
+ for(e = list->head; e; e = enext) {
data = (struct Curl_easy *)e->ptr;
- if(data->magic != CURLEASY_MAGIC_NUMBER)
- /* bad bad bad bad bad bad bad */
- return CURLM_INTERNAL_ERROR;
+ /* assign 'enext' here since the 'e' struct might be cleared
+ further down in the singlesocket() call */
+ enext = e->next;
+
+ DEBUGASSERT(data);
+ DEBUGASSERT(data->magic == CURLEASY_MAGIC_NUMBER);
/* If the pipeline is enabled, take the handle which is in the head of
the pipeline. If we should write into the socket, take the
diff --git a/lib/urldata.h b/lib/urldata.h
index d4a5ad8..c793a6b 100644
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -1797,6 +1797,7 @@ struct Curl_easy {
struct curl_llist_element connect_queue;
struct curl_llist_element pipeline_queue;
struct curl_llist_element sh_queue; /* list per Curl_sh_entry */
+ struct Curl_sh_entry *sh_entry; /* the socket hash this was added to */
CURLMstate mstate; /* the handle's state */
CURLcode result; /* previous result */
--
2.20.1

View File

@ -1,7 +1,7 @@
Summary: A utility for getting files from remote servers (FTP, HTTP, and others)
Name: curl
Version: 7.64.0
Release: 7%{?dist}
Release: 8%{?dist}
License: MIT
Source: https://curl.haxx.se/download/%{name}-%{version}.tar.xz
@ -26,6 +26,9 @@ Patch16: 0016-curl-7.64.0-CVE-2019-5435.patch
# fix TFTP receive buffer overflow (CVE-2019-5436)
Patch17: 0017-curl-7.64.0-CVE-2019-5436.patch
# prevent multi from crashing with many parallel transfers (#1697566, #1723242)
Patch18: 0018-curl-7.64.0-multi-sigsegv.patch
# patch making libcurl multilib ready
Patch101: 0101-curl-7.32.0-multilib.patch
@ -203,6 +206,7 @@ be installed.
# upstream patches
%patch16 -p1
%patch17 -p1
%patch18 -p1
# make tests/*.py use Python 3
sed -e '1 s|^#!/.*python|#!%{__python3}|' -i tests/*.py
@ -363,6 +367,9 @@ rm -f ${RPM_BUILD_ROOT}%{_libdir}/libcurl.la
%{_libdir}/libcurl.so.4.[0-9].[0-9].minimal
%changelog
* Mon Jul 01 2019 Kamil Dudka <kdudka@redhat.com> - 7.64.0-8
- prevent multi from crashing with many parallel transfers (#1697566, #1723242)
* Wed May 22 2019 Kamil Dudka <kdudka@redhat.com> - 7.64.0-7
- fix TFTP receive buffer overflow (CVE-2019-5436)
- fix integer overflows in curl_url_set() (CVE-2019-5435)