curl: fix some security problems

This fixes the following security problems:
* CVE-2017-1000254: FTP PWD response parser out of bounds read
* CVE-2017-1000257: IMAP FETCH response out of bounds read
* CVE-2018-1000005: HTTP/2 trailer out-of-bounds read
* CVE-2018-1000007: HTTP authentication leak in redirects
* CVE-2018-1000120: FTP path trickery leads to NIL byte out of bounds write
* CVE-2018-1000121: LDAP NULL pointer dereference
* CVE-2018-1000122: RTSP RTP buffer over-read
* CVE-2018-1000301: RTSP bad headers buffer over-read

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
This commit is contained in:
Hauke Mehrtens 2018-08-10 21:39:06 +02:00
parent b3983323a1
commit 9bc43f3e65
12 changed files with 385 additions and 45 deletions

View File

@ -9,7 +9,7 @@ include $(TOPDIR)/rules.mk
PKG_NAME:=curl
PKG_VERSION:=7.52.1
PKG_RELEASE:=9
PKG_RELEASE:=10
PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.bz2
PKG_SOURCE_URL:=http://curl.haxx.se/download/ \

View File

@ -0,0 +1,49 @@
From 29b251362e1839d7094993edbed8f9467069773f Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Mon, 25 Sep 2017 00:35:22 +0200
Subject: [PATCH] FTP: zero terminate the entry path even on bad input
... a single double quote could leave the entry path buffer without a zero
terminating byte. CVE-2017-1000254
Test 1152 added to verify.
Reported-by: Max Dymond
Bug: https://curl.haxx.se/docs/adv_20171004.html
---
lib/ftp.c | 7 ++++--
tests/data/Makefile.inc | 1 +
tests/data/test1152 | 61 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 67 insertions(+), 2 deletions(-)
create mode 100644 tests/data/test1152
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -2825,6 +2825,7 @@ static CURLcode ftp_statemach_act(struct
char *ptr=&data->state.buffer[4]; /* start on the first letter */
char *dir;
char *store;
+ bool entry_extracted = FALSE;
dir = malloc(nread + 1);
if(!dir)
@@ -2856,7 +2857,7 @@ static CURLcode ftp_statemach_act(struct
}
else {
/* end of path */
- *store = '\0'; /* zero terminate */
+ entry_extracted = TRUE;
break; /* get out of this loop */
}
}
@@ -2865,7 +2866,9 @@ static CURLcode ftp_statemach_act(struct
store++;
ptr++;
}
-
+ *store = '\0'; /* zero terminate */
+ }
+ if(entry_extracted) {
/* If the path name does not look like an absolute path (i.e.: it
does not start with a '/'), we probably need some server-dependent
adjustments. For example, this is the case when connecting to

View File

@ -0,0 +1,28 @@
From 13c9a9ded3ae744a1e11cbc14e9146d9fa427040 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Sat, 7 Oct 2017 00:11:31 +0200
Subject: [PATCH] imap: if a FETCH response has no size, don't call write
callback
CVE-2017-1000257
Reported-by: Brian Carpenter and 0xd34db347
Also detected by OSS-Fuzz: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=3586
---
lib/imap.c | 5 +++++
1 file changed, 5 insertions(+)
--- a/lib/imap.c
+++ b/lib/imap.c
@@ -1140,6 +1140,11 @@ static CURLcode imap_state_fetch_resp(st
/* The conversion from curl_off_t to size_t is always fine here */
chunk = (size_t)size;
+ if(!chunk) {
+ /* no size, we're done with the data */
+ state(conn, IMAP_STOP);
+ return CURLE_OK;
+ }
result = Curl_client_write(conn, CLIENTWRITE_BODY, pp->cache, chunk);
if(result)
return result;

View File

@ -13,13 +13,9 @@ Bug: https://curl.haxx.se/docs/adv_2017-11e7.html
lib/curl_ntlm_core.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/lib/curl_ntlm_core.c b/lib/curl_ntlm_core.c
index 1309bf0d9..e8962769c 100644
--- a/lib/curl_ntlm_core.c
+++ b/lib/curl_ntlm_core.c
@@ -616,23 +616,42 @@ CURLcode Curl_hmac_md5(const unsigned char *key, unsigned int keylen,
Curl_HMAC_final(ctxt, output);
@@ -618,6 +618,15 @@ CURLcode Curl_hmac_md5(const unsigned ch
return CURLE_OK;
}
@ -35,9 +31,7 @@ index 1309bf0d9..e8962769c 100644
/* This creates the NTLMv2 hash by using NTLM hash as the key and Unicode
* (uppercase UserName + Domain) as the data
*/
CURLcode Curl_ntlm_core_mk_ntlmv2_hash(const char *user, size_t userlen,
const char *domain, size_t domlen,
unsigned char *ntlmhash,
@@ -627,10 +636,20 @@ CURLcode Curl_ntlm_core_mk_ntlmv2_hash(c
unsigned char *ntlmv2hash)
{
/* Unicode representation */
@ -60,8 +54,3 @@ index 1309bf0d9..e8962769c 100644
if(!identity)
return CURLE_OUT_OF_MEMORY;
ascii_uppercase_to_unicode_le(identity, user, userlen);
ascii_to_unicode_le(identity + (userlen << 1), domain, domlen);
--
2.15.0

View File

@ -20,13 +20,9 @@ Bug: https://curl.haxx.se/docs/adv_2017-ae72.html
3 files changed, 56 insertions(+), 7 deletions(-)
create mode 100644 tests/data/test1163
diff --git a/lib/curl_fnmatch.c b/lib/curl_fnmatch.c
index da83393b4..8a1e106c4 100644
--- a/lib/curl_fnmatch.c
+++ b/lib/curl_fnmatch.c
@@ -131,10 +131,13 @@ static int setcharset(unsigned char **p, unsigned char *charset)
unsigned char lastchar = 0;
bool something_found = FALSE;
@@ -133,6 +133,9 @@ static int setcharset(unsigned char **p,
unsigned char c;
for(;;) {
c = **p;
@ -36,11 +32,7 @@ index da83393b4..8a1e106c4 100644
switch(state) {
case CURLFNM_SCHS_DEFAULT:
if(ISALNUM(c)) { /* ASCII value */
rangestart = c;
charset[c] = 1;
@@ -195,13 +198,10 @@ static int setcharset(unsigned char **p, unsigned char *charset)
(*p)++;
}
@@ -197,9 +200,6 @@ static int setcharset(unsigned char **p,
else
return SETCHARSET_FAIL;
}
@ -50,11 +42,7 @@ index da83393b4..8a1e106c4 100644
else {
charset[c] = 1;
(*p)++;
something_found = TRUE;
}
@@ -276,13 +276,10 @@ static int setcharset(unsigned char **p, unsigned char *charset)
(*p)++;
}
@@ -278,9 +278,6 @@ static int setcharset(unsigned char **p,
else if(c == ']') {
return SETCHARSET_OK;
}
@ -64,13 +52,9 @@ index da83393b4..8a1e106c4 100644
else if(ISPRINT(c)) {
charset[c] = 1;
(*p)++;
state = CURLFNM_SCHS_DEFAULT;
}
diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc
index dc1cc03bc..6eb37d81d 100644
--- a/tests/data/Makefile.inc.1 2017-11-29 20:00:26.126452486 +0000
+++ b/tests/data/Makefile.inc 2017-11-29 20:01:13.057783732 +0000
@@ -121,6 +121,7 @@
--- a/tests/data/Makefile.inc
+++ b/tests/data/Makefile.inc
@@ -121,6 +121,7 @@ test1120 test1121 test1122 test1123 test
test1128 test1129 test1130 test1131 test1132 test1133 test1134 test1135 \
test1136 test1137 test1138 test1139 test1140 test1141 test1142 test1143 \
test1144 \
@ -78,9 +62,6 @@ index dc1cc03bc..6eb37d81d 100644
test1200 test1201 test1202 test1203 test1204 test1205 test1206 test1207 \
test1208 test1209 test1210 test1211 test1212 test1213 test1214 test1215 \
test1216 test1217 test1218 test1219 \
diff --git a/tests/data/test1163 b/tests/data/test1163
new file mode 100644
index 000000000..a109b511b
--- /dev/null
+++ b/tests/data/test1163
@@ -0,0 +1,52 @@
@ -136,6 +117,3 @@ index 000000000..a109b511b
+</errorcode>
+</verify>
+</testcase>
--
2.15.0

View File

@ -0,0 +1,34 @@
From fa3dbb9a147488a2943bda809c66fc497efe06cb Mon Sep 17 00:00:00 2001
From: Zhouyihai Ding <ddyihai@ddyihai.svl.corp.google.com>
Date: Wed, 10 Jan 2018 10:12:18 -0800
Subject: [PATCH] http2: fix incorrect trailer buffer size
Prior to this change the stored byte count of each trailer was
miscalculated and 1 less than required. It appears any trailer
after the first that was passed to Curl_client_write would be truncated
or corrupted as well as the size. Potentially the size of some
subsequent trailer could be erroneously extracted from the contents of
that trailer, and since that size is used by client write an
out-of-bounds read could occur and cause a crash or be otherwise
processed by client write.
The bug appears to have been born in 0761a51 (precedes 7.49.0).
Closes https://github.com/curl/curl/pull/2231
---
lib/http2.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -864,8 +864,8 @@ static int on_header(nghttp2_session *se
if(stream->bodystarted) {
/* This is trailer fields. */
- /* 3 is for ":" and "\r\n". */
- uint32_t n = (uint32_t)(namelen + valuelen + 3);
+ /* 4 is for ": " and "\r\n". */
+ uint32_t n = (uint32_t)(namelen + valuelen + 4);
DEBUGF(infof(data_s, "h2 trailer: %.*s: %.*s\n", namelen, name, valuelen,
value));

View File

@ -0,0 +1,102 @@
From af32cd3859336ab963591ca0df9b1e33a7ee066b Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Fri, 19 Jan 2018 13:19:25 +0100
Subject: [PATCH] http: prevent custom Authorization headers in redirects
... unless CURLOPT_UNRESTRICTED_AUTH is set to allow them. This matches how
curl already handles Authorization headers created internally.
Note: this changes behavior slightly, for the sake of reducing mistakes.
Added test 317 and 318 to verify.
Reported-by: Craig de Stigter
Bug: https://curl.haxx.se/docs/adv_2018-b3bf.html
---
docs/libcurl/opts/CURLOPT_HTTPHEADER.3 | 12 +++-
lib/http.c | 10 ++-
lib/setopt.c | 2 +-
lib/urldata.h | 2 +-
tests/data/Makefile.inc | 2 +-
tests/data/test317 | 94 +++++++++++++++++++++++++
tests/data/test318 | 95 ++++++++++++++++++++++++++
7 files changed, 212 insertions(+), 5 deletions(-)
create mode 100644 tests/data/test317
create mode 100644 tests/data/test318
--- a/docs/libcurl/opts/CURLOPT_HTTPHEADER.3
+++ b/docs/libcurl/opts/CURLOPT_HTTPHEADER.3
@@ -5,7 +5,7 @@
.\" * | (__| |_| | _ <| |___
.\" * \___|\___/|_| \_\_____|
.\" *
-.\" * Copyright (C) 1998 - 2015, Daniel Stenberg, <daniel@haxx.se>, et al.
+.\" * Copyright (C) 1998 - 2018, Daniel Stenberg, <daniel@haxx.se>, et al.
.\" *
.\" * This software is licensed as described in the file COPYING, which
.\" * you should have received as part of this distribution. The terms
@@ -77,6 +77,16 @@ the headers. They may be private or othe
Use \fICURLOPT_HEADEROPT(3)\fP to make the headers only get sent to where you
intend them to get sent.
+
+Custom headers are sent in all requests done by the easy handles, which
+implies that if you tell libcurl to follow redirects
+(\fBCURLOPT_FOLLOWLOCATION(3)\fP), the same set of custom headers will be sent
+in the subsequent request. Redirects can of course go to other hosts and thus
+those servers will get all the contents of your custom headers too.
+
+Starting in 7.58.0, libcurl will specifically prevent "Authorization:" headers
+from being sent to other hosts than the first used one, unless specifically
+permitted with the \fBCURLOPT_UNRESTRICTED_AUTH(3)\fP option.
.SH DEFAULT
NULL
.SH PROTOCOLS
--- a/lib/http.c
+++ b/lib/http.c
@@ -725,7 +725,7 @@ Curl_http_output_auth(struct connectdata
if(!data->state.this_is_a_follow ||
conn->bits.netrc ||
!data->state.first_host ||
- data->set.http_disable_hostname_check_before_authentication ||
+ data->set.allow_auth_to_other_hosts ||
strcasecompare(data->state.first_host, conn->host.name)) {
result = output_auth_headers(conn, authhost, request, path, FALSE);
}
@@ -1624,6 +1624,14 @@ CURLcode Curl_add_custom_headers(struct
checkprefix("Transfer-Encoding:", headers->data))
/* HTTP/2 doesn't support chunked requests */
;
+ else if(checkprefix("Authorization:", headers->data) &&
+ /* be careful of sending this potentially sensitive header to
+ other hosts */
+ (data->state.this_is_a_follow &&
+ data->state.first_host &&
+ !data->set.allow_auth_to_other_hosts &&
+ !strcasecompare(data->state.first_host, conn->host.name)))
+ ;
else {
CURLcode result = Curl_add_bufferf(req_buffer, "%s\r\n",
headers->data);
--- a/lib/url.c
+++ b/lib/url.c
@@ -972,7 +972,7 @@ CURLcode Curl_setopt(struct Curl_easy *d
* Send authentication (user+password) when following locations, even when
* hostname changed.
*/
- data->set.http_disable_hostname_check_before_authentication =
+ data->set.allow_auth_to_other_hosts =
(0 != va_arg(param, long)) ? TRUE : FALSE;
break;
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -1675,7 +1675,7 @@ struct UserDefined {
bool http_keep_sending_on_error; /* for HTTP status codes >= 300 */
bool http_follow_location; /* follow HTTP redirects */
bool http_transfer_encoding; /* request compressed HTTP transfer-encoding */
- bool http_disable_hostname_check_before_authentication;
+ bool allow_auth_to_other_hosts;
bool include_header; /* include received protocol headers in data output */
bool http_set_referer; /* is a custom referer used */
bool http_auto_referer; /* set "correct" referer when following location: */

View File

@ -0,0 +1,53 @@
From a6ae0fbe9c50733e0f645f5bd16e1db38c592c3d Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Wed, 31 Jan 2018 08:40:11 +0100
Subject: [PATCH] FTP: reject path components with control codes
Refuse to operate when given path components featuring byte values lower
than 32.
Previously, inserting a %00 sequence early in the directory part when
using the 'singlecwd' ftp method could make curl write a zero byte
outside of the allocated buffer.
Test case 340 verifies.
CVE-2018-1000120
Reported-by: Duy Phan Thanh
Bug: https://curl.haxx.se/docs/adv_2018-9cd6.html
---
lib/ftp.c | 8 ++++----
tests/data/Makefile.inc | 3 +++
tests/data/test340 | 40 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 47 insertions(+), 4 deletions(-)
create mode 100644 tests/data/test340
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -3235,7 +3235,7 @@ static CURLcode ftp_done(struct connectd
if(!result)
/* get the "raw" path */
- result = Curl_urldecode(data, path_to_use, 0, &path, NULL, FALSE);
+ result = Curl_urldecode(data, path_to_use, 0, &path, NULL, TRUE);
if(result) {
/* We can limp along anyway (and should try to since we may already be in
* the error path) */
@@ -4241,7 +4241,7 @@ CURLcode ftp_parse_url_path(struct conne
result = Curl_urldecode(conn->data, slash_pos ? cur_pos : "/",
slash_pos ? dirlen : 1,
&ftpc->dirs[0], NULL,
- FALSE);
+ TRUE);
if(result) {
freedirs(ftpc);
return result;
@@ -4349,7 +4349,7 @@ CURLcode ftp_parse_url_path(struct conne
size_t dlen;
char *path;
CURLcode result =
- Curl_urldecode(conn->data, data->state.path, 0, &path, &dlen, FALSE);
+ Curl_urldecode(conn->data, data->state.path, 0, &path, &dlen, TRUE);
if(result) {
freedirs(ftpc);
return result;

View File

@ -0,0 +1,37 @@
From 8f341a5d6f15381492ca2013325d485b6d8d1c13 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Tue, 6 Mar 2018 23:02:16 +0100
Subject: [PATCH] openldap: check ldap_get_attribute_ber() results for NULL
before using
CVE-2018-1000121
Reported-by: Dario Weisser
Bug: https://curl.haxx.se/docs/adv_2018-97a2.html
---
lib/openldap.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
--- a/lib/openldap.c
+++ b/lib/openldap.c
@@ -443,7 +443,7 @@ static ssize_t ldap_recv(struct connectd
for(ent = ldap_first_message(li->ld, msg); ent;
ent = ldap_next_message(li->ld, ent)) {
- struct berval bv, *bvals, **bvp = &bvals;
+ struct berval bv, *bvals;
int binary = 0, msgtype;
CURLcode writeerr;
@@ -505,9 +505,9 @@ static ssize_t ldap_recv(struct connectd
}
data->req.bytecount += bv.bv_len + 5;
- for(rc = ldap_get_attribute_ber(li->ld, ent, ber, &bv, bvp);
- rc == LDAP_SUCCESS;
- rc = ldap_get_attribute_ber(li->ld, ent, ber, &bv, bvp)) {
+ for(rc = ldap_get_attribute_ber(li->ld, ent, ber, &bv, &bvals);
+ (rc == LDAP_SUCCESS) && bvals;
+ rc = ldap_get_attribute_ber(li->ld, ent, ber, &bv, &bvals)) {
int i;
if(bv.bv_val == NULL) break;

View File

@ -0,0 +1,33 @@
From d70b74d6f893947aa22d3f14df10f92a8c349388 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Thu, 8 Mar 2018 10:33:16 +0100
Subject: [PATCH] readwrite: make sure excess reads don't go beyond buffer end
CVE-2018-1000122
Bug: https://curl.haxx.se/docs/adv_2018-b047.html
Detected by OSS-fuzz
---
lib/transfer.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
--- a/lib/transfer.c
+++ b/lib/transfer.c
@@ -791,10 +791,15 @@ static CURLcode readwrite_data(struct Cu
} /* if(!header and data to read) */
- if(conn->handler->readwrite &&
- (excess > 0 && !conn->bits.stream_was_rewound)) {
+ if(conn->handler->readwrite && excess && !conn->bits.stream_was_rewound) {
/* Parse the excess data */
k->str += nread;
+
+ if(&k->str[excess] > &k->buf[data->set.buffer_size]) {
+ /* the excess amount was too excessive(!), make sure
+ it doesn't read out of buffer */
+ excess = &k->buf[data->set.buffer_size] - k->str;
+ }
nread = (ssize_t)excess;
result = conn->handler->readwrite(data, conn, &nread, &readmore);

View File

@ -0,0 +1,39 @@
From 8c7b3737d29ed5c0575bf592063de8a51450812d Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Sat, 24 Mar 2018 23:47:41 +0100
Subject: [PATCH] http: restore buffer pointer when bad response-line is parsed
... leaving the k->str could lead to buffer over-reads later on.
CVE: CVE-2018-1000301
Assisted-by: Max Dymond
Detected by OSS-Fuzz.
Bug: https://curl.haxx.se/docs/adv_2018-b138.html
Bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=7105
---
lib/http.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
--- a/lib/http.c
+++ b/lib/http.c
@@ -2924,6 +2924,8 @@ CURLcode Curl_http_readwrite_headers(str
{
CURLcode result;
struct SingleRequest *k = &data->req;
+ ssize_t onread = *nread;
+ char *ostr = k->str;
/* header line within buffer loop */
do {
@@ -2988,7 +2990,9 @@ CURLcode Curl_http_readwrite_headers(str
else {
/* this was all we read so it's all a bad header */
k->badheader = HEADER_ALLBAD;
- *nread = (ssize_t)rest_length;
+ *nread = onread;
+ k->str = ostr;
+ return CURLE_OK;
}
break;
}

View File

@ -9,11 +9,9 @@ vtls must set wait for read/write flags for the socket.
lib/vtls/vtls.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/vtls/vtls.c b/lib/vtls/vtls.c
index fad9335bbf..871622fef1 100644
--- a/lib/vtls/vtls.c
+++ b/lib/vtls/vtls.c
@@ -485,8 +485,9 @@ void Curl_ssl_close_all(struct Curl_easy *data)
@@ -488,8 +488,9 @@ void Curl_ssl_close_all(struct Curl_easy
}
#if defined(USE_OPENSSL) || defined(USE_GNUTLS) || defined(USE_SCHANNEL) || \