You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
446 lines
15 KiB
446 lines
15 KiB
From 5452fdc5ae93f3571074c591fdf28cdf630796a0 Mon Sep 17 00:00:00 2001 |
|
From: Daniel Stenberg <daniel@haxx.se> |
|
Date: Tue, 12 Sep 2017 09:29:01 +0200 |
|
Subject: [PATCH 1/3] FTP: URL decode path for dir listing in nocwd mode |
|
|
|
Reported-by: Zenju on github |
|
|
|
Test 244 added to verify |
|
Fixes #1974 |
|
Closes #1976 |
|
|
|
Upstream-commit: ecf21c551fa3426579463abe34b623111b8d487c |
|
Signed-off-by: Kamil Dudka <kdudka@redhat.com> |
|
--- |
|
lib/ftp.c | 93 +++++++++++++++++++++++--------------------------- |
|
tests/data/Makefile.am | 3 +- |
|
tests/data/test244 | 54 +++++++++++++++++++++++++++++ |
|
3 files changed, 99 insertions(+), 51 deletions(-) |
|
create mode 100644 tests/data/test244 |
|
|
|
diff --git a/lib/ftp.c b/lib/ftp.c |
|
index bcba6bb..fb3a716 100644 |
|
--- a/lib/ftp.c |
|
+++ b/lib/ftp.c |
|
@@ -1003,7 +1003,7 @@ static CURLcode ftp_state_use_port(struct connectdata *conn, |
|
char *port_start = NULL; |
|
char *port_sep = NULL; |
|
|
|
- addr = calloc(addrlen+1, 1); |
|
+ addr = calloc(addrlen + 1, 1); |
|
if(!addr) |
|
return CURLE_OUT_OF_MEMORY; |
|
|
|
@@ -1041,7 +1041,7 @@ static CURLcode ftp_state_use_port(struct connectdata *conn, |
|
/* parse the port */ |
|
if(ip_end != NULL) { |
|
if((port_start = strchr(ip_end, ':')) != NULL) { |
|
- port_min = curlx_ultous(strtoul(port_start+1, NULL, 10)); |
|
+ port_min = curlx_ultous(strtoul(port_start + 1, NULL, 10)); |
|
if((port_sep = strchr(port_start, '-')) != NULL) { |
|
port_max = curlx_ultous(strtoul(port_sep + 1, NULL, 10)); |
|
} |
|
@@ -1469,25 +1469,22 @@ static CURLcode ftp_state_post_listtype(struct connectdata *conn) |
|
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, FALSE); |
|
+ if(result) |
|
+ return result; |
|
} |
|
|
|
cmd = aprintf( "%s%s%s", |
|
@@ -3327,12 +3324,10 @@ 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) |
|
- result = CURLE_OUT_OF_MEMORY; |
|
ftpc->ctl_valid = FALSE; /* mark control connection as bad */ |
|
conn->bits.close = TRUE; /* mark for connection closure */ |
|
ftpc->prevpath = NULL; /* no path remembering */ |
|
@@ -3643,7 +3638,7 @@ static CURLcode ftp_range(struct connectdata *conn) |
|
} |
|
else { |
|
/* X-Y */ |
|
- data->req.maxdownload = (to-from)+1; /* include last byte */ |
|
+ data->req.maxdownload = (to - from) + 1; /* include last byte */ |
|
data->state.resume_from = from; |
|
DEBUGF(infof(conn->data, "FTP RANGE from %" FORMAT_OFF_T |
|
" getting %" FORMAT_OFF_T " bytes\n", |
|
@@ -4332,20 +4327,22 @@ CURLcode ftp_parse_url_path(struct connectdata *conn) |
|
} |
|
slash_pos=strrchr(cur_pos, '/'); |
|
if(slash_pos || !*cur_pos) { |
|
+ CURLcode result; |
|
ftpc->dirs = calloc(1, sizeof(ftpc->dirs[0])); |
|
if(!ftpc->dirs) |
|
return CURLE_OUT_OF_MEMORY; |
|
|
|
- ftpc->dirs[0] = curl_easy_unescape(conn->data, slash_pos ? cur_pos : "/", |
|
- slash_pos ? |
|
- curlx_sztosi(slash_pos-cur_pos) : 1, |
|
- NULL); |
|
- if(!ftpc->dirs[0]) { |
|
+ result = Curl_urldecode(conn->data, slash_pos ? cur_pos : "/", |
|
+ slash_pos ? |
|
+ curlx_sztosi(slash_pos-cur_pos) : 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 */ |
|
+ filename = slash_pos ? slash_pos + 1 : cur_pos; /* rest is file name */ |
|
} |
|
else |
|
filename = cur_pos; /* this is a file name only */ |
|
@@ -4377,18 +4374,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 { |
|
@@ -4415,15 +4409,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 |
|
@@ -4441,15 +4432,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/tests/data/Makefile.am b/tests/data/Makefile.am |
|
index 56cb286..e7955ee 100644 |
|
--- a/tests/data/Makefile.am |
|
+++ b/tests/data/Makefile.am |
|
@@ -28,7 +28,8 @@ test200 test201 test202 test203 test204 test205 test206 test207 test208 \ |
|
test209 test210 test211 test212 test213 test214 test215 test216 test217 \ |
|
test218 test220 test221 test222 test223 test224 test225 test226 test227 \ |
|
test228 test229 test231 test233 test234 test235 test236 test237 test238 \ |
|
-test239 test240 test241 test242 test243 test245 test246 test247 test248 \ |
|
+test239 test240 test241 test242 test243 \ |
|
+test244 test245 test246 test247 test248 \ |
|
test249 test250 test251 test252 test253 test254 test255 test256 test257 \ |
|
test258 test259 test260 test261 test262 test263 test264 test265 test266 \ |
|
test267 test268 test269 test270 test271 test272 test273 test274 test275 \ |
|
diff --git a/tests/data/test244 b/tests/data/test244 |
|
new file mode 100644 |
|
index 0000000..8ce4b63 |
|
--- /dev/null |
|
+++ b/tests/data/test244 |
|
@@ -0,0 +1,54 @@ |
|
+<testcase> |
|
+<info> |
|
+<keywords> |
|
+FTP |
|
+PASV |
|
+CWD |
|
+--ftp-method |
|
+nocwd |
|
+</keywords> |
|
+</info> |
|
+# |
|
+# Server-side |
|
+<reply> |
|
+<data mode="text"> |
|
+total 20 |
|
+drwxr-xr-x 8 98 98 512 Oct 22 13:06 . |
|
+drwxr-xr-x 8 98 98 512 Oct 22 13:06 .. |
|
+drwxr-xr-x 2 98 98 512 May 2 1996 .NeXT |
|
+-r--r--r-- 1 0 1 35 Jul 16 1996 README |
|
+lrwxrwxrwx 1 0 1 7 Dec 9 1999 bin -> usr/bin |
|
+dr-xr-xr-x 2 0 1 512 Oct 1 1997 dev |
|
+drwxrwxrwx 2 98 98 512 May 29 16:04 download.html |
|
+dr-xr-xr-x 2 0 1 512 Nov 30 1995 etc |
|
+drwxrwxrwx 2 98 1 512 Oct 30 14:33 pub |
|
+dr-xr-xr-x 5 0 1 512 Oct 1 1997 usr |
|
+</data> |
|
+</reply> |
|
+ |
|
+# Client-side |
|
+<client> |
|
+<server> |
|
+ftp |
|
+</server> |
|
+ <name> |
|
+FTP dir listing with nocwd and URL encoded path |
|
+ </name> |
|
+ <command> |
|
+--ftp-method nocwd ftp://%HOSTIP:%FTPPORT/fir%23t/th%69rd/244/ |
|
+</command> |
|
+</client> |
|
+ |
|
+# Verify data after the test has been "shot" |
|
+<verify> |
|
+<protocol> |
|
+USER anonymous |
|
+PASS ftp@example.com |
|
+PWD |
|
+EPSV |
|
+TYPE A |
|
+LIST fir#t/third/244/ |
|
+QUIT |
|
+</protocol> |
|
+</verify> |
|
+</testcase> |
|
-- |
|
2.14.3 |
|
|
|
|
|
From 295fc8b0dc5c94a1cbf6688bfba768128b13cde6 Mon Sep 17 00:00:00 2001 |
|
From: Daniel Stenberg <daniel@haxx.se> |
|
Date: Wed, 2 Nov 2016 07:22:27 +0100 |
|
Subject: [PATCH 2/3] ftp_done: don't clobber the passed in error code |
|
|
|
Coverity CID 1374359 pointed out the unused result value. |
|
|
|
Upstream-commit: f81a8364618caf99b4691ffd494a9b2d4c9fb1f6 |
|
Signed-off-by: Kamil Dudka <kdudka@redhat.com> |
|
--- |
|
lib/ftp.c | 9 +++++---- |
|
1 file changed, 5 insertions(+), 4 deletions(-) |
|
|
|
diff --git a/lib/ftp.c b/lib/ftp.c |
|
index 9da5a24..0259a14 100644 |
|
--- a/lib/ftp.c |
|
+++ b/lib/ftp.c |
|
@@ -3323,11 +3323,12 @@ static CURLcode ftp_done(struct connectdata *conn, CURLcode status, |
|
ftpc->known_filesize = -1; |
|
} |
|
|
|
- /* get the "raw" path */ |
|
- result = Curl_urldecode(data, path_to_use, 0, &path, NULL, FALSE); |
|
+ if(!result) |
|
+ /* get the "raw" 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) */ |
|
+ /* 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 */ |
|
-- |
|
2.14.4 |
|
|
|
|
|
From 9534442aae1da4e6cf2ce815e47dbcd82695c3d4 Mon Sep 17 00:00:00 2001 |
|
From: Daniel Stenberg <daniel@haxx.se> |
|
Date: Wed, 31 Jan 2018 08:40:11 +0100 |
|
Subject: [PATCH 3/3] 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 |
|
|
|
Upstream-commit: 535432c0adb62fe167ec09621500470b6fa4eb0f |
|
Signed-off-by: Kamil Dudka <kdudka@redhat.com> |
|
--- |
|
lib/ftp.c | 8 ++++---- |
|
tests/data/Makefile.am | 1 + |
|
tests/data/test340 | 40 ++++++++++++++++++++++++++++++++++++++++ |
|
3 files changed, 45 insertions(+), 4 deletions(-) |
|
create mode 100644 tests/data/test340 |
|
|
|
diff --git a/lib/ftp.c b/lib/ftp.c |
|
index fb3a716..268efdd 100644 |
|
--- a/lib/ftp.c |
|
+++ b/lib/ftp.c |
|
@@ -1482,7 +1482,7 @@ static CURLcode ftp_state_post_listtype(struct connectdata *conn) |
|
slashPos = strrchr(inpath, '/'); |
|
n = slashPos - inpath; |
|
} |
|
- result = Curl_urldecode(data, inpath, n, &lstArg, NULL, FALSE); |
|
+ result = Curl_urldecode(data, inpath, n, &lstArg, NULL, TRUE); |
|
if(result) |
|
return result; |
|
} |
|
@@ -3325,7 +3325,7 @@ static CURLcode ftp_done(struct connectdata *conn, CURLcode status, |
|
|
|
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) */ |
|
@@ -4337,7 +4337,7 @@ CURLcode ftp_parse_url_path(struct connectdata *conn) |
|
slash_pos ? |
|
curlx_sztosi(slash_pos-cur_pos) : 1, |
|
&ftpc->dirs[0], NULL, |
|
- FALSE); |
|
+ TRUE); |
|
if(result) { |
|
freedirs(ftpc); |
|
return result; |
|
@@ -4436,7 +4436,7 @@ CURLcode ftp_parse_url_path(struct connectdata *conn) |
|
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; |
|
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am |
|
index e7955ee..910db5b 100644 |
|
--- a/tests/data/Makefile.am |
|
+++ b/tests/data/Makefile.am |
|
@@ -39,6 +39,7 @@ test294 test295 test296 test297 test298 test299 test300 test301 test302 \ |
|
test303 test304 test305 test306 test307 test308 test309 test310 test311 \ |
|
test312 test313 test317 test318 \ |
|
test320 test321 test322 test323 test324 test350 test351 \ |
|
+test340 \ |
|
test352 test353 test354 test400 test401 test402 test403 test404 test405 \ |
|
test406 test407 test408 test409 test500 test501 test502 test503 test504 \ |
|
test505 test506 test507 test508 test510 test511 test512 test513 test514 \ |
|
diff --git a/tests/data/test340 b/tests/data/test340 |
|
new file mode 100644 |
|
index 0000000..d834d76 |
|
--- /dev/null |
|
+++ b/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> |
|
-- |
|
2.14.3 |
|
|
|
|