File curl-7.37.0-CVE-2018-1000120.patch of Package curl.34221
From 8d68181ecaf1623f0ae80b1daa7f2d4fe25113d8 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.
---
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
Index: curl-7.37.0/lib/ftp.c
===================================================================
--- curl-7.37.0.orig/lib/ftp.c
+++ curl-7.37.0/lib/ftp.c
@@ -1488,25 +1488,22 @@ static CURLcode ftp_state_list(struct co
then just do LIST (in that case: nothing to do here)
*/
char *cmd,*lstArg,*slashPos;
+ const char *inpath = data->state.path;
lstArg = NULL;
if((data->set.ftp_filemethod == FTPFILE_NOCWD) &&
- data->state.path &&
- data->state.path[0] &&
- strchr(data->state.path,'/')) {
-
- lstArg = strdup(data->state.path);
- if(!lstArg)
- return CURLE_OUT_OF_MEMORY;
+ inpath && inpath[0] && strchr(inpath, '/')) {
+ size_t n = strlen(inpath);
/* Check if path does not end with /, as then we cut off the file part */
- if(lstArg[strlen(lstArg) - 1] != '/') {
-
+ if(inpath[n - 1] != '/') {
/* chop off the file part if format is dir/dir/file */
- slashPos = strrchr(lstArg,'/');
- if(slashPos)
- *(slashPos+1) = '\0';
+ slashPos = strrchr(inpath, '/');
+ n = slashPos - inpath;
}
+ result = Curl_urldecode(data, inpath, n, &lstArg, NULL, TRUE);
+ if(result)
+ return result;
}
cmd = aprintf( "%s%s%s",
@@ -1517,19 +1514,16 @@ static CURLcode ftp_state_list(struct co
lstArg? lstArg: "" );
if(!cmd) {
- if(lstArg)
- free(lstArg);
+ free(lstArg);
return CURLE_OUT_OF_MEMORY;
}
result = Curl_pp_sendf(&conn->proto.ftpc.pp, "%s", cmd);
- if(lstArg)
- free(lstArg);
-
+ free(lstArg);
free(cmd);
- if(result != CURLE_OK)
+ if(result)
return result;
state(conn, FTP_LIST);
@@ -3211,8 +3205,7 @@ static CURLcode ftp_done(struct connectd
ssize_t nread;
int ftpcode;
CURLcode result = CURLE_OK;
- bool was_ctl_valid = ftpc->ctl_valid;
- char *path;
+ char *path = NULL;
const char *path_to_use = data->state.path;
if(!ftp)
@@ -3240,10 +3233,9 @@ static CURLcode ftp_done(struct connectd
/* the connection stays alive fine even though this happened */
/* fall-through */
case CURLE_OK: /* doesn't affect the control connection's status */
- if(!premature) {
- ftpc->ctl_valid = was_ctl_valid;
+ if(!premature)
break;
- }
+
/* until we cope better with prematurely ended requests, let them
* fallback as if in complete failure */
default: /* by default, an error means the control connection is
@@ -3257,23 +3249,20 @@ static CURLcode ftp_done(struct connectd
}
/* now store a copy of the directory we are in */
- if(ftpc->prevpath)
- free(ftpc->prevpath);
+ free(ftpc->prevpath);
if(data->set.wildcardmatch) {
if(data->set.chunk_end && ftpc->file) {
data->set.chunk_end(data->wildcard.customptr);
}
ftpc->known_filesize = -1;
}
/* get the "raw" path */
- path = curl_easy_unescape(data, path_to_use, 0, NULL);
- if(!path) {
- /* 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)
- result = CURLE_OUT_OF_MEMORY;
+ 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) */
ftpc->ctl_valid = FALSE; /* mark control connection as bad */
conn->bits.close = TRUE; /* mark for connection closure */
ftpc->prevpath = NULL; /* no path remembering */
@@ -4238,15 +4226,16 @@ CURLcode ftp_parse_url_path(struct conne
the first condition in the if() right here, is there just in case
someone decides to set path to NULL one day
*/
- if(data->state.path &&
- data->state.path[0] &&
- (data->state.path[strlen(data->state.path) - 1] != '/') )
- filename = data->state.path; /* this is a full file path */
+ if(path_to_use[0] &&
+ (path_to_use[strlen(path_to_use) - 1] != '/') )
+ filename = path_to_use; /* this is a full file path */
/*
+ else {
ftpc->file is not used anywhere other than for operations on a file.
In other words, never for directory operations.
So we can safely leave filename as NULL here and use it as a
argument in dir/file decisions.
+ }
*/
break;
@@ -4260,6 +4249,7 @@ CURLcode ftp_parse_url_path(struct conne
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)
@@ -4268,12 +4258,13 @@ CURLcode ftp_parse_url_path(struct conne
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,
+ TRUE);
+ 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 */
@@ -4291,7 +4282,7 @@ CURLcode ftp_parse_url_path(struct conne
return CURLE_OUT_OF_MEMORY;
/* we have a special case for listing the root dir only */
- if(strequal(path_to_use, "/")) {
+ if(!strcmp(path_to_use, "/")) {
cur_pos++; /* make it point to the zero byte */
ftpc->dirs[0] = strdup("/");
ftpc->dirdepth++;
@@ -4308,18 +4299,14 @@ CURLcode ftp_parse_url_path(struct conne
/* 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])) {
- free(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) {
freedirs(ftpc);
- return CURLE_URL_MALFORMAT;
+ return result;
}
}
else {
@@ -4355,15 +4342,12 @@ CURLcode ftp_parse_url_path(struct conne
} /* 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
@@ -4381,11 +4365,13 @@ CURLcode ftp_parse_url_path(struct conne
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, TRUE);
+ if(result) {
freedirs(ftpc);
- return CURLE_OUT_OF_MEMORY;
+ return result;
}
dlen -= ftpc->file?curlx_uztosi(strlen(ftpc->file)):0;
Index: curl-7.37.0/tests/data/test340
===================================================================
--- /dev/null
+++ curl-7.37.0/tests/data/test340
@@ -0,0 +1,40 @@
+<testcase>
+<info>
+<keywords>
+FTP
+PASV
+CWD
+--ftp-method
+singlecwd
+</keywords>
+</info>
+#
+# Server-side
+<reply>
+</reply>
+
+# Client-side
+<client>
+<server>
+ftp
+</server>
+ <name>
+FTP using %00 in path with singlecwd
+ </name>
+ <command>
+--ftp-method singlecwd ftp://%HOSTIP:%FTPPORT/%00first/second/third/340
+</command>
+</client>
+
+# Verify data after the test has been "shot"
+<verify>
+<protocol>
+USER anonymous
+PASS ftp@example.com
+PWD
+</protocol>
+<errorcode>
+3
+</errorcode>
+</verify>
+</testcase>
Index: curl-7.37.0/tests/data/Makefile.am
===================================================================
--- curl-7.37.0.orig/tests/data/Makefile.am
+++ curl-7.37.0/tests/data/Makefile.am
@@ -39,6 +39,9 @@ test298 test299 test300 test301 test302
test307 test308 test309 test310 test311 test312 test313 \
test317 test318 test320 test321 test322 test323 test324 \
test325 \
+\
+test340 \
+\
test350 test351 test352 test353 test354 \
\
test400 test401 test402 test403 test404 test405 test406 test407 test408 \
Index: curl-7.37.0/tests/data/Makefile.in
===================================================================
--- curl-7.37.0.orig/tests/data/Makefile.in
+++ curl-7.37.0/tests/data/Makefile.in
@@ -347,6 +347,9 @@ test298 test299 test300 test301 test302
test307 test308 test309 test310 test311 test312 test313 \
test317 test318 test320 test321 test322 test323 test324 \
test325 \
+\
+test340 \
+\
test350 test351 test352 test353 test354 \
\
test400 test401 test402 test403 test404 test405 test406 test407 test408 \
