From e407d02ccf2a8dfb7afa0ad20f5e65c4325f0d3f370edca787291d7b796f1024 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Chv=C3=A1tal?= Date: Sat, 9 Mar 2019 07:51:32 +0000 Subject: [PATCH] Accepting request 682917 from home:favogt:zyppcrash - Add patches to fix use-after-free (boo#1127849): * 0001-connection_check-set-data-to-the-transfer-doing-the-.patch * 0002-connection_check-restore-original-conn-data-after-th.patch OBS-URL: https://build.opensuse.org/request/show/682917 OBS-URL: https://build.opensuse.org/package/show/devel:libraries:c_c++/curl?expand=0&rev=247 --- ...-set-data-to-the-transfer-doing-the-.patch | 31 ++++++++ ...-restore-original-conn-data-after-th.patch | 77 +++++++++++++++++++ curl-mini.changes | 7 ++ curl-mini.spec | 5 ++ curl.changes | 7 ++ curl.spec | 5 ++ 6 files changed, 132 insertions(+) create mode 100644 0001-connection_check-set-data-to-the-transfer-doing-the-.patch create mode 100644 0002-connection_check-restore-original-conn-data-after-th.patch diff --git a/0001-connection_check-set-data-to-the-transfer-doing-the-.patch b/0001-connection_check-set-data-to-the-transfer-doing-the-.patch new file mode 100644 index 0000000..7dbc9a6 --- /dev/null +++ b/0001-connection_check-set-data-to-the-transfer-doing-the-.patch @@ -0,0 +1,31 @@ +From c34b576805318aa4896caf1d9b806a5bb89ca456 Mon Sep 17 00:00:00 2001 +From: Daniel Stenberg +Date: Mon, 11 Feb 2019 07:56:00 +0100 +Subject: [PATCH 1/2] connection_check: set ->data to the transfer doing the + check + +The http2 code for connection checking needs a transfer to use. Make +sure a working one is set before handler->connection_check() is called. + +Reported-by: jnbr on github +Fixes #3541 +Closes #3547 +--- + lib/url.c | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/lib/url.c b/lib/url.c +index d5a982008..229c655da 100644 +--- a/lib/url.c ++++ b/lib/url.c +@@ -965,6 +965,7 @@ static bool extract_if_dead(struct connectdata *conn, + /* The protocol has a special method for checking the state of the + connection. Use it to check if the connection is dead. */ + unsigned int state; ++ conn->data = data; /* use this transfer for now */ + state = conn->handler->connection_check(conn, CONNCHECK_ISDEAD); + dead = (state & CONNRESULT_DEAD); + } +-- +2.20.1 + diff --git a/0002-connection_check-restore-original-conn-data-after-th.patch b/0002-connection_check-restore-original-conn-data-after-th.patch new file mode 100644 index 0000000..956b37e --- /dev/null +++ b/0002-connection_check-restore-original-conn-data-after-th.patch @@ -0,0 +1,77 @@ +From f992905ab8a242934dba114103c730117a1d25a3 Mon Sep 17 00:00:00 2001 +From: Jay Satiro +Date: Mon, 11 Feb 2019 23:00:00 -0500 +Subject: [PATCH 2/2] connection_check: restore original conn->data after the + check + +- Save the original conn->data before it's changed to the specified + data transfer for the connection check and then restore it afterwards. + +This is a follow-up to 38d8e1b 2019-02-11. + +History: + +It was discovered a month ago that before checking whether to extract a +dead connection that that connection should be associated with a "live" +transfer for the check (ie original conn->data ignored and set to the +passed in data). A fix was landed in 54b201b which did that and also +cleared conn->data after the check. The original conn->data was not +restored, so presumably it was thought that a valid conn->data was no +longer needed. + +Several days later it was discovered that a valid conn->data was needed +after the check and follow-up fix was landed in bbae24c which partially +reverted the original fix and attempted to limit the scope of when +conn->data was changed to only when pruning dead connections. In that +case conn->data was not cleared and the original conn->data not +restored. + +A month later it was discovered that the original fix was somewhat +correct; a "live" transfer is needed for the check in all cases +because original conn->data could be null which could cause a bad deref +at arbitrary points in the check. A fix was landed in 38d8e1b which +expanded the scope to all cases. conn->data was not cleared and the +original conn->data not restored. + +A day later it was discovered that not restoring the original conn->data +may lead to busy loops in applications that use the event interface, and +given this observation it's a pretty safe assumption that there is some +code path that still needs the original conn->data. This commit is the +follow-up fix for that, it restores the original conn->data after the +connection check. + +Assisted-by: tholin@users.noreply.github.com +Reported-by: tholin@users.noreply.github.com + +Fixes https://github.com/curl/curl/issues/3542 +Closes #3559 +--- + lib/url.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/lib/url.c b/lib/url.c +index 229c655da..a77e92dfe 100644 +--- a/lib/url.c ++++ b/lib/url.c +@@ -965,8 +965,10 @@ static bool extract_if_dead(struct connectdata *conn, + /* The protocol has a special method for checking the state of the + connection. Use it to check if the connection is dead. */ + unsigned int state; ++ struct Curl_easy *olddata = conn->data; + conn->data = data; /* use this transfer for now */ + state = conn->handler->connection_check(conn, CONNCHECK_ISDEAD); ++ conn->data = olddata; + dead = (state & CONNRESULT_DEAD); + } + else { +@@ -995,7 +997,6 @@ struct prunedead { + static int call_extract_if_dead(struct connectdata *conn, void *param) + { + struct prunedead *p = (struct prunedead *)param; +- conn->data = p->data; /* transfer to use for this check */ + if(extract_if_dead(conn, p->data)) { + /* stop the iteration here, pass back the connection that was extracted */ + p->extracted = conn; +-- +2.20.1 + diff --git a/curl-mini.changes b/curl-mini.changes index 496f12c..33402e0 100644 --- a/curl-mini.changes +++ b/curl-mini.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Fri Mar 8 16:10:39 UTC 2019 - Fabian Vogt + +- Add patches to fix use-after-free (boo#1127849): + * 0001-connection_check-set-data-to-the-transfer-doing-the-.patch + * 0002-connection_check-restore-original-conn-data-after-th.patch + ------------------------------------------------------------------- Wed Feb 27 08:53:31 UTC 2019 - Stephan Kulow diff --git a/curl-mini.spec b/curl-mini.spec index f464bf3..e693582 100644 --- a/curl-mini.spec +++ b/curl-mini.spec @@ -46,6 +46,9 @@ Patch3: ignore_runtests_failure.patch # PATCH-FIX-OPENSUSE bsc#1076446 protocol redirection not supported or disabled Patch4: curl-disabled-redirect-protocol-message.patch Patch5: curl-use_OPENSSL_config.patch +# PATCH-FIX-UPSTREAM boo#1127849 fix a crash in libcurl +Patch6: 0001-connection_check-set-data-to-the-transfer-doing-the-.patch +Patch7: 0002-connection_check-restore-original-conn-data-after-th.patch BuildRequires: libtool BuildRequires: pkgconfig Requires: libcurl4%{?mini} = %{version} @@ -129,6 +132,8 @@ user interaction or any kind of interactivity. %endif %patch4 -p1 %patch5 -p1 +%patch6 -p1 +%patch7 -p1 %build # curl complains if macro definition is contained in CFLAGS diff --git a/curl.changes b/curl.changes index 496f12c..33402e0 100644 --- a/curl.changes +++ b/curl.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Fri Mar 8 16:10:39 UTC 2019 - Fabian Vogt + +- Add patches to fix use-after-free (boo#1127849): + * 0001-connection_check-set-data-to-the-transfer-doing-the-.patch + * 0002-connection_check-restore-original-conn-data-after-th.patch + ------------------------------------------------------------------- Wed Feb 27 08:53:31 UTC 2019 - Stephan Kulow diff --git a/curl.spec b/curl.spec index 769cac4..eb5b412 100644 --- a/curl.spec +++ b/curl.spec @@ -44,6 +44,9 @@ Patch3: ignore_runtests_failure.patch # PATCH-FIX-OPENSUSE bsc#1076446 protocol redirection not supported or disabled Patch4: curl-disabled-redirect-protocol-message.patch Patch5: curl-use_OPENSSL_config.patch +# PATCH-FIX-UPSTREAM boo#1127849 fix a crash in libcurl +Patch6: 0001-connection_check-set-data-to-the-transfer-doing-the-.patch +Patch7: 0002-connection_check-restore-original-conn-data-after-th.patch BuildRequires: libtool BuildRequires: pkgconfig Requires: libcurl4%{?mini} = %{version} @@ -127,6 +130,8 @@ user interaction or any kind of interactivity. %endif %patch4 -p1 %patch5 -p1 +%patch6 -p1 +%patch7 -p1 %build # curl complains if macro definition is contained in CFLAGS