Brad Bishop | d7bf8c1 | 2018-02-25 22:55:05 -0500 | [diff] [blame] | 1 | From 1b2eba6f9745c064f7283e0ada8f46df9d9d6e42 Mon Sep 17 00:00:00 2001 |
| 2 | From: Li Zhou <li.zhou@windriver.com> |
| 3 | Date: Mon, 23 Oct 2017 00:26:50 -0700 |
| 4 | Subject: [PATCH] FTP: zero terminate the entry path even on bad input |
| 5 | |
| 6 | ... a single double quote could leave the entry path buffer without a zero |
| 7 | terminating byte. CVE-2017-1000254 |
| 8 | |
| 9 | Test 1152 added to verify. |
| 10 | |
| 11 | Reported-by: Max Dymond |
| 12 | Bug: https://curl.haxx.se/docs/adv_20171004.html |
| 13 | |
| 14 | Upstream-Status: Backport |
| 15 | CVE: CVE-2017-1000254 |
| 16 | Signed-off-by: Li Zhou <li.zhou@windriver.com> |
| 17 | --- |
| 18 | lib/ftp.c | 7 ++++-- |
| 19 | tests/data/Makefile.inc | 2 ++ |
| 20 | tests/data/test1152 | 61 +++++++++++++++++++++++++++++++++++++++++++++++++ |
| 21 | 3 files changed, 68 insertions(+), 2 deletions(-) |
| 22 | create mode 100644 tests/data/test1152 |
| 23 | |
| 24 | diff --git a/lib/ftp.c b/lib/ftp.c |
| 25 | index 5edec37..493dbf9 100644 |
| 26 | --- a/lib/ftp.c |
| 27 | +++ b/lib/ftp.c |
| 28 | @@ -2826,6 +2826,7 @@ static CURLcode ftp_statemach_act(struct connectdata *conn) |
| 29 | const size_t buf_size = data->set.buffer_size; |
| 30 | char *dir; |
| 31 | char *store; |
| 32 | + bool entry_extracted = FALSE; |
| 33 | |
| 34 | dir = malloc(nread + 1); |
| 35 | if(!dir) |
| 36 | @@ -2857,7 +2858,7 @@ static CURLcode ftp_statemach_act(struct connectdata *conn) |
| 37 | } |
| 38 | else { |
| 39 | /* end of path */ |
| 40 | - *store = '\0'; /* zero terminate */ |
| 41 | + entry_extracted = TRUE; |
| 42 | break; /* get out of this loop */ |
| 43 | } |
| 44 | } |
| 45 | @@ -2866,7 +2867,9 @@ static CURLcode ftp_statemach_act(struct connectdata *conn) |
| 46 | store++; |
| 47 | ptr++; |
| 48 | } |
| 49 | - |
| 50 | + *store = '\0'; /* zero terminate */ |
| 51 | + } |
| 52 | + if(entry_extracted) { |
| 53 | /* If the path name does not look like an absolute path (i.e.: it |
| 54 | does not start with a '/'), we probably need some server-dependent |
| 55 | adjustments. For example, this is the case when connecting to |
| 56 | diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc |
| 57 | index 7adbee6..5284654 100644 |
| 58 | --- a/tests/data/Makefile.inc |
| 59 | +++ b/tests/data/Makefile.inc |
| 60 | @@ -121,6 +121,8 @@ test1120 test1121 test1122 test1123 test1124 test1125 test1126 test1127 \ |
| 61 | test1128 test1129 test1130 test1131 test1132 test1133 test1134 test1135 \ |
| 62 | test1136 test1137 test1138 test1139 test1140 test1141 test1142 test1143 \ |
| 63 | test1144 test1145 test1146 \ |
| 64 | +test1152 \ |
| 65 | +\ |
| 66 | test1200 test1201 test1202 test1203 test1204 test1205 test1206 test1207 \ |
| 67 | test1208 test1209 test1210 test1211 test1212 test1213 test1214 test1215 \ |
| 68 | test1216 test1217 test1218 test1219 \ |
| 69 | diff --git a/tests/data/test1152 b/tests/data/test1152 |
| 70 | new file mode 100644 |
| 71 | index 0000000..aa8c0a7 |
| 72 | --- /dev/null |
| 73 | +++ b/tests/data/test1152 |
| 74 | @@ -0,0 +1,61 @@ |
| 75 | +<testcase> |
| 76 | +<info> |
| 77 | +<keywords> |
| 78 | +FTP |
| 79 | +PASV |
| 80 | +LIST |
| 81 | +</keywords> |
| 82 | +</info> |
| 83 | +# |
| 84 | +# Server-side |
| 85 | +<reply> |
| 86 | +<servercmd> |
| 87 | +REPLY PWD 257 "just one |
| 88 | +</servercmd> |
| 89 | + |
| 90 | +# When doing LIST, we get the default list output hard-coded in the test |
| 91 | +# FTP server |
| 92 | +<data mode="text"> |
| 93 | +total 20 |
| 94 | +drwxr-xr-x 8 98 98 512 Oct 22 13:06 . |
| 95 | +drwxr-xr-x 8 98 98 512 Oct 22 13:06 .. |
| 96 | +drwxr-xr-x 2 98 98 512 May 2 1996 curl-releases |
| 97 | +-r--r--r-- 1 0 1 35 Jul 16 1996 README |
| 98 | +lrwxrwxrwx 1 0 1 7 Dec 9 1999 bin -> usr/bin |
| 99 | +dr-xr-xr-x 2 0 1 512 Oct 1 1997 dev |
| 100 | +drwxrwxrwx 2 98 98 512 May 29 16:04 download.html |
| 101 | +dr-xr-xr-x 2 0 1 512 Nov 30 1995 etc |
| 102 | +drwxrwxrwx 2 98 1 512 Oct 30 14:33 pub |
| 103 | +dr-xr-xr-x 5 0 1 512 Oct 1 1997 usr |
| 104 | +</data> |
| 105 | +</reply> |
| 106 | + |
| 107 | +# |
| 108 | +# Client-side |
| 109 | +<client> |
| 110 | +<server> |
| 111 | +ftp |
| 112 | +</server> |
| 113 | + <name> |
| 114 | +FTP with uneven quote in PWD response |
| 115 | + </name> |
| 116 | + <command> |
| 117 | +ftp://%HOSTIP:%FTPPORT/test-1152/ |
| 118 | +</command> |
| 119 | +</client> |
| 120 | + |
| 121 | +# |
| 122 | +# Verify data after the test has been "shot" |
| 123 | +<verify> |
| 124 | +<protocol> |
| 125 | +USER anonymous |
| 126 | +PASS ftp@example.com |
| 127 | +PWD |
| 128 | +CWD test-1152 |
| 129 | +EPSV |
| 130 | +TYPE A |
| 131 | +LIST |
| 132 | +QUIT |
| 133 | +</protocol> |
| 134 | +</verify> |
| 135 | +</testcase> |
| 136 | -- |
| 137 | 2.11.0 |
| 138 | |