From 76f91d8b2da6501558cd92620c9656c283db8adf Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Tue, 4 Oct 2016 18:56:45 +0200
Subject: [PATCH 1/2] unescape: avoid integer overflow
CVE-2016-8622
Bug: https://curl.haxx.se/docs/adv_20161102H.html
Reported-by: Cure53
Upstream-commit: 53e71e47d6b81650d26ec33a58d0dca24c7ffb2c
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
docs/libcurl/curl_easy_unescape.3 | 7 +++++--
lib/dict.c | 10 +++++-----
lib/escape.c | 10 ++++++++--
3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/docs/libcurl/curl_easy_unescape.3 b/docs/libcurl/curl_easy_unescape.3
index 06fd6fc..50ce97d 100644
--- a/docs/libcurl/curl_easy_unescape.3
+++ b/docs/libcurl/curl_easy_unescape.3
@@ -5,7 +5,7 @@
.\" * | (__| |_| | _ <| |___
.\" * \___|\___/|_| \_\_____|
.\" *
-.\" * Copyright (C) 1998 - 2015, Daniel Stenberg, <daniel@haxx.se>, et al.
+.\" * Copyright (C) 1998 - 2016, 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
@@ -40,7 +40,10 @@ will use strlen() on the input \fIurl\fP string to find out the size.
If \fBoutlength\fP is non-NULL, the function will write the length of the
returned string in the integer it points to. This allows an escaped string
-containing %00 to still get used properly after unescaping.
+containing %00 to still get used properly after unescaping. Since this is a
+pointer to an \fIint\fP type, it can only return a value up to INT_MAX so no
+longer string can be unescaped if the string length is returned in this
+parameter.
You must \fIcurl_free(3)\fP the returned string when you're done with it.
.SH AVAILABILITY
diff --git a/lib/dict.c b/lib/dict.c
index 2e7cb47..e1475ca 100644
--- a/lib/dict.c
+++ b/lib/dict.c
@@ -5,7 +5,7 @@
* | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____|
*
- * Copyright (C) 1998 - 2015, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2016, 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
@@ -52,7 +52,7 @@
#include <curl/curl.h>
#include "transfer.h"
#include "sendf.h"
-
+#include "escape.h"
#include "progress.h"
#include "strequal.h"
#include "dict.h"
@@ -96,12 +96,12 @@ static char *unescape_word(struct SessionHandle *data, const char *inputbuff)
char *newp;
char *dictp;
char *ptr;
- int len;
+ size_t len;
char ch;
int olen=0;
- newp = curl_easy_unescape(data, inputbuff, 0, &len);
- if(!newp)
+ CURLcode result = Curl_urldecode(data, inputbuff, 0, &newp, &len, FALSE);
+ if(!newp || result)
return NULL;
dictp = malloc(((size_t)len)*2 + 1); /* add one for terminating zero */
diff --git a/lib/escape.c b/lib/escape.c
index 808ac6c..4091097 100644
--- a/lib/escape.c
+++ b/lib/escape.c
@@ -224,8 +224,14 @@ char *curl_easy_unescape(CURL *handle, const char *string, int length,
FALSE);
if(res)
return NULL;
- if(olen)
- *olen = curlx_uztosi(outputlen);
+
+ if(olen) {
+ if(outputlen <= (size_t) INT_MAX)
+ *olen = curlx_uztosi(outputlen);
+ else
+ /* too large to return in an int, fail! */
+ Curl_safefree(str);
+ }
}
return str;
}
--
2.7.4
From e24b3f19b11fa4bf01ae4bce34d6aa96b27ec6ab Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Sat, 8 Oct 2016 11:21:38 +0200
Subject: [PATCH 2/2] escape: avoid using curl_easy_unescape() internally
Since the internal Curl_urldecode() function has a better API.
Upstream-commit: 46133aa536f7f5bf552b83369e3851b6f811299e
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
lib/file.c | 9 +++++----
lib/ftp.c | 60 +++++++++++++++++++++++++++++-------------------------------
lib/gopher.c | 5 +++--
lib/ldap.c | 26 +++++++++++++++-----------
lib/ssh.c | 12 ++++++------
lib/tftp.c | 9 +++++----
lib/url.c | 55 +++++++++++++++++++++++++++----------------------------
7 files changed, 90 insertions(+), 86 deletions(-)
diff --git a/lib/file.c b/lib/file.c
index d47bda1..1afdb7b 100644
--- a/lib/file.c
+++ b/lib/file.c
@@ -194,11 +194,12 @@ static CURLcode file_connect(struct connectdata *conn, bool *done)
int i;
char *actual_path;
#endif
- int real_path_len;
+ size_t real_path_len;
- real_path = curl_easy_unescape(data, data->state.path, 0, &real_path_len);
- if(!real_path)
- return CURLE_OUT_OF_MEMORY;
+ CURLcode result = Curl_urldecode(data, data->state.path, 0, &real_path,
+ &real_path_len, FALSE);
+ if(result)
+ return result;
#ifdef DOS_FILESYSTEM
/* If the first character is a slash, and there's
diff --git a/lib/ftp.c b/lib/ftp.c
index b5a7f55..d7e3dbb 100644
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -3298,8 +3298,8 @@ static CURLcode ftp_done(struct connectdata *conn, CURLcode status,
}
/* get the "raw" path */
- path = curl_easy_unescape(data, path_to_use, 0, NULL);
- if(!path) {
+ result = Curl_urldecode(data, path_to_use, 0, &path, NULL, FALSE);
+ if(result) {
/* out of memory, but we can limp along anyway (and should try to
* since we may already be in the out of memory cleanup path) */
if(!result)
@@ -4291,6 +4291,7 @@ CURLcode ftp_parse_url_path(struct connectdata *conn)
slash_pos=strrchr(cur_pos, '/');
if(slash_pos || !*cur_pos) {
size_t dirlen = slash_pos-cur_pos;
+ CURLcode result;
ftpc->dirs = calloc(1, sizeof(ftpc->dirs[0]));
if(!ftpc->dirs)
@@ -4299,12 +4300,13 @@ CURLcode ftp_parse_url_path(struct connectdata *conn)
if(!dirlen)
dirlen++;
- ftpc->dirs[0] = curl_easy_unescape(conn->data, slash_pos ? cur_pos : "/",
- slash_pos ? curlx_uztosi(dirlen) : 1,
- NULL);
- if(!ftpc->dirs[0]) {
+ result = Curl_urldecode(conn->data, slash_pos ? cur_pos : "/",
+ slash_pos ? dirlen : 1,
+ &ftpc->dirs[0], NULL,
+ FALSE);
+ if(result) {
freedirs(ftpc);
- return CURLE_OUT_OF_MEMORY;
+ return result;
}
ftpc->dirdepth = 1; /* we consider it to be a single dir */
filename = slash_pos ? slash_pos+1 : cur_pos; /* rest is file name */
@@ -4339,18 +4341,15 @@ CURLcode ftp_parse_url_path(struct connectdata *conn)
/* we skip empty path components, like "x//y" since the FTP command
CWD requires a parameter and a non-existent parameter a) doesn't
work on many servers and b) has no effect on the others. */
- int len = curlx_sztosi(slash_pos - cur_pos + absolute_dir);
- ftpc->dirs[ftpc->dirdepth] =
- curl_easy_unescape(conn->data, cur_pos - absolute_dir, len, NULL);
- if(!ftpc->dirs[ftpc->dirdepth]) { /* run out of memory ... */
- failf(data, "no memory");
- freedirs(ftpc);
- return CURLE_OUT_OF_MEMORY;
- }
- if(isBadFtpString(ftpc->dirs[ftpc->dirdepth])) {
+ size_t len = slash_pos - cur_pos + absolute_dir;
+ CURLcode result =
+ Curl_urldecode(conn->data, cur_pos - absolute_dir, len,
+ &ftpc->dirs[ftpc->dirdepth], NULL,
+ TRUE);
+ if(result) {
free(ftpc->dirs[ftpc->dirdepth]);
freedirs(ftpc);
- return CURLE_URL_MALFORMAT;
+ return result;
}
}
else {
@@ -4386,15 +4385,12 @@ CURLcode ftp_parse_url_path(struct connectdata *conn)
} /* switch */
if(filename && *filename) {
- ftpc->file = curl_easy_unescape(conn->data, filename, 0, NULL);
- if(NULL == ftpc->file) {
- freedirs(ftpc);
- failf(data, "no memory");
- return CURLE_OUT_OF_MEMORY;
- }
- if(isBadFtpString(ftpc->file)) {
+ CURLcode result =
+ Curl_urldecode(conn->data, filename, 0, &ftpc->file, NULL, TRUE);
+
+ if(result) {
freedirs(ftpc);
- return CURLE_URL_MALFORMAT;
+ return result;
}
}
else
@@ -4412,15 +4408,17 @@ CURLcode ftp_parse_url_path(struct connectdata *conn)
if(ftpc->prevpath) {
/* prevpath is "raw" so we convert the input path before we compare the
strings */
- int dlen;
- char *path = curl_easy_unescape(conn->data, data->state.path, 0, &dlen);
- if(!path) {
+ size_t dlen;
+ char *path;
+ CURLcode result =
+ Curl_urldecode(conn->data, data->state.path, 0, &path, &dlen, FALSE);
+ if(result) {
freedirs(ftpc);
- return CURLE_OUT_OF_MEMORY;
+ return result;
}
- dlen -= ftpc->file?curlx_uztosi(strlen(ftpc->file)):0;
- if((dlen == curlx_uztosi(strlen(ftpc->prevpath))) &&
+ dlen -= ftpc->file?strlen(ftpc->file):0;
+ if((dlen == strlen(ftpc->prevpath)) &&
strnequal(path, ftpc->prevpath, dlen)) {
infof(data, "Request has same path as previous transfer\n");
ftpc->cwddone = TRUE;
diff --git a/lib/gopher.c b/lib/gopher.c
index 19f2f5a..03c79f7 100644
--- a/lib/gopher.c
+++ b/lib/gopher.c
@@ -35,6 +35,7 @@
#include "rawstr.h"
#include "select.h"
#include "url.h"
+#include "escape.h"
#include "warnless.h"
#include "curl_memory.h"
/* The last #include file should be: */
@@ -83,7 +84,7 @@ static CURLcode gopher_do(struct connectdata *conn, bool *done)
char *sel;
char *sel_org = NULL;
ssize_t amount, k;
- int len;
+ size_t len;
*done = TRUE; /* unconditionally */
@@ -107,7 +108,7 @@ static CURLcode gopher_do(struct connectdata *conn, bool *done)
newp[i] = '\x09';
/* ... and finally unescape */
- sel = curl_easy_unescape(data, newp, 0, &len);
+ result = Curl_urldecode(data, newp, 0, &sel, &len, FALSE);
if(!sel)
return CURLE_OUT_OF_MEMORY;
sel_org = sel;
diff --git a/lib/ldap.c b/lib/ldap.c
index 4f4c707..57061bb 100644
--- a/lib/ldap.c
+++ b/lib/ldap.c
@@ -768,7 +768,7 @@ static bool split_str(char *str, char ***out, size_t *count)
*
* Defined in RFC4516 section 2.
*/
-static int _ldap_url_parse2 (const struct connectdata *conn, LDAPURLDesc *ludp)
+static int _ldap_url_parse2(const struct connectdata *conn, LDAPURLDesc *ludp)
{
int rc = LDAP_SUCCESS;
char *path;
@@ -799,12 +799,13 @@ static int _ldap_url_parse2 (const struct connectdata *conn, LDAPURLDesc *ludp)
if(*p) {
char *dn = p;
char *unescaped;
+ CURLcode result;
LDAP_TRACE (("DN '%s'\n", dn));
/* Unescape the DN */
- unescaped = curl_easy_unescape(conn->data, dn, 0, NULL);
- if(!unescaped) {
+ result = Curl_urldecode(conn->data, dn, 0, &unescaped, NULL, FALSE);
+ if(result) {
rc = LDAP_NO_MEMORY;
goto quit;
@@ -863,12 +864,14 @@ static int _ldap_url_parse2 (const struct connectdata *conn, LDAPURLDesc *ludp)
for(i = 0; i < count; i++) {
char *unescaped;
+ CURLcode result;
LDAP_TRACE (("attr[%d] '%s'\n", i, attributes[i]));
/* Unescape the attribute */
- unescaped = curl_easy_unescape(conn->data, attributes[i], 0, NULL);
- if(!unescaped) {
+ result = Curl_urldecode(conn->data, attributes[i], 0, &unescaped, NULL,
+ FALSE);
+ if(result) {
free(attributes);
rc = LDAP_NO_MEMORY;
@@ -931,12 +934,13 @@ static int _ldap_url_parse2 (const struct connectdata *conn, LDAPURLDesc *ludp)
if(*p) {
char *filter = p;
char *unescaped;
+ CURLcode result;
LDAP_TRACE (("filter '%s'\n", filter));
/* Unescape the filter */
- unescaped = curl_easy_unescape(conn->data, filter, 0, NULL);
- if(!unescaped) {
+ result = Curl_urldecode(conn->data, filter, 0, &unescaped, NULL, FALSE);
+ if(result) {
rc = LDAP_NO_MEMORY;
goto quit;
@@ -972,8 +976,8 @@ quit:
return rc;
}
-static int _ldap_url_parse (const struct connectdata *conn,
- LDAPURLDesc **ludpp)
+static int _ldap_url_parse(const struct connectdata *conn,
+ LDAPURLDesc **ludpp)
{
LDAPURLDesc *ludp = calloc(1, sizeof(*ludp));
int rc;
@@ -982,7 +986,7 @@ static int _ldap_url_parse (const struct connectdata *conn,
if(!ludp)
return LDAP_NO_MEMORY;
- rc = _ldap_url_parse2 (conn, ludp);
+ rc = _ldap_url_parse2(conn, ludp);
if(rc != LDAP_SUCCESS) {
_ldap_free_urldesc(ludp);
ludp = NULL;
@@ -991,7 +995,7 @@ static int _ldap_url_parse (const struct connectdata *conn,
return (rc);
}
-static void _ldap_free_urldesc (LDAPURLDesc *ludp)
+static void _ldap_free_urldesc(LDAPURLDesc *ludp)
{
size_t i;
diff --git a/lib/ssh.c b/lib/ssh.c
index 10bf546..2d9b6a2 100644
--- a/lib/ssh.c
+++ b/lib/ssh.c
@@ -407,12 +407,12 @@ static CURLcode ssh_getworkingpath(struct connectdata *conn,
struct SessionHandle *data = conn->data;
char *real_path = NULL;
char *working_path;
- int working_path_len;
-
- working_path = curl_easy_unescape(data, data->state.path, 0,
- &working_path_len);
- if(!working_path)
- return CURLE_OUT_OF_MEMORY;
+ size_t working_path_len;
+ CURLcode result =
+ Curl_urldecode(data, data->state.path, 0, &working_path,
+ &working_path_len, FALSE);
+ if(result)
+ return result;
/* Check for /~/ , indicating relative to the user's home directory */
if(conn->handler->protocol & CURLPROTO_SCP) {
diff --git a/lib/tftp.c b/lib/tftp.c
index 040d967..274a724 100644
--- a/lib/tftp.c
+++ b/lib/tftp.c
@@ -59,6 +59,7 @@
#include "speedcheck.h"
#include "curl_printf.h"
#include "select.h"
+#include "escape.h"
/* The last #include files should be: */
#include "curl_memory.h"
@@ -484,10 +485,10 @@ static CURLcode tftp_send_first(tftp_state_data_t *state, tftp_event_t event)
/* As RFC3617 describes the separator slash is not actually part of the
file name so we skip the always-present first letter of the path
string. */
- filename = curl_easy_unescape(data, &state->conn->data->state.path[1], 0,
- NULL);
- if(!filename)
- return CURLE_OUT_OF_MEMORY;
+ result = Curl_urldecode(data, &state->conn->data->state.path[1], 0,
+ &filename, NULL, FALSE);
+ if(result)
+ return result;
snprintf((char *)state->spacket.data+2,
state->blksize,
diff --git a/lib/url.c b/lib/url.c
index ff14dad..a88903c 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -4652,21 +4652,24 @@ static CURLcode parse_proxy(struct SessionHandle *data,
them. */
Curl_safefree(conn->proxyuser);
if(proxyuser && strlen(proxyuser) < MAX_CURL_USER_LENGTH)
- conn->proxyuser = curl_easy_unescape(data, proxyuser, 0, NULL);
- else
+ result = Curl_urldecode(data, proxyuser, 0, &conn->proxyuser, NULL,
+ FALSE);
+ else {
conn->proxyuser = strdup("");
+ if(!conn->proxyuser)
+ result = CURLE_OUT_OF_MEMORY;
+ }
- if(!conn->proxyuser)
- result = CURLE_OUT_OF_MEMORY;
- else {
+ if(!result) {
Curl_safefree(conn->proxypasswd);
if(proxypasswd && strlen(proxypasswd) < MAX_CURL_PASSWORD_LENGTH)
- conn->proxypasswd = curl_easy_unescape(data, proxypasswd, 0, NULL);
- else
+ result = Curl_urldecode(data, proxypasswd, 0,
+ &conn->proxypasswd, NULL, FALSE);
+ else {
conn->proxypasswd = strdup("");
-
- if(!conn->proxypasswd)
- result = CURLE_OUT_OF_MEMORY;
+ if(!conn->proxypasswd)
+ result = CURLE_OUT_OF_MEMORY;
+ }
}
if(!result) {
@@ -4773,6 +4776,7 @@ static CURLcode parse_proxy_auth(struct SessionHandle *data,
{
char proxyuser[MAX_CURL_USER_LENGTH]="";
char proxypasswd[MAX_CURL_PASSWORD_LENGTH]="";
+ CURLcode result;
if(data->set.str[STRING_PROXYUSERNAME] != NULL) {
strncpy(proxyuser, data->set.str[STRING_PROXYUSERNAME],
@@ -4785,15 +4789,11 @@ static CURLcode parse_proxy_auth(struct SessionHandle *data,
proxypasswd[MAX_CURL_PASSWORD_LENGTH-1] = '\0'; /*To be on safe side*/
}
- conn->proxyuser = curl_easy_unescape(data, proxyuser, 0, NULL);
- if(!conn->proxyuser)
- return CURLE_OUT_OF_MEMORY;
-
- conn->proxypasswd = curl_easy_unescape(data, proxypasswd, 0, NULL);
- if(!conn->proxypasswd)
- return CURLE_OUT_OF_MEMORY;
-
- return CURLE_OK;
+ result = Curl_urldecode(data, proxyuser, 0, &conn->proxyuser, NULL, FALSE);
+ if(!result)
+ result = Curl_urldecode(data, proxypasswd, 0, &conn->proxypasswd, NULL,
+ FALSE);
+ return result;
}
#endif /* CURL_DISABLE_PROXY */
@@ -4867,9 +4867,8 @@ static CURLcode parse_url_login(struct SessionHandle *data,
conn->bits.user_passwd = TRUE; /* enable user+password */
/* Decode the user */
- newname = curl_easy_unescape(data, userp, 0, NULL);
- if(!newname) {
- result = CURLE_OUT_OF_MEMORY;
+ result = Curl_urldecode(data, userp, 0, &newname, NULL, FALSE);
+ if(result) {
goto out;
}
@@ -4879,9 +4878,9 @@ static CURLcode parse_url_login(struct SessionHandle *data,
if(passwdp) {
/* We have a password in the URL so decode it */
- char *newpasswd = curl_easy_unescape(data, passwdp, 0, NULL);
- if(!newpasswd) {
- result = CURLE_OUT_OF_MEMORY;
+ char *newpasswd;
+ result = Curl_urldecode(data, passwdp, 0, &newpasswd, NULL, FALSE);
+ if(result) {
goto out;
}
@@ -4891,9 +4890,9 @@ static CURLcode parse_url_login(struct SessionHandle *data,
if(optionsp) {
/* We have an options list in the URL so decode it */
- char *newoptions = curl_easy_unescape(data, optionsp, 0, NULL);
- if(!newoptions) {
- result = CURLE_OUT_OF_MEMORY;
+ char *newoptions;
+ result = Curl_urldecode(data, optionsp, 0, &newoptions, NULL, FALSE);
+ if(result) {
goto out;
}
--
2.7.4