File curl-7.37.0-CVE-2018-1000120.patch of Package curl

Overview Repositories Revisions Requests Users Attributes Meta

File curl-7.37.0-CVE-2018-1000120.patch of Package curl (Revision 87f7ac353a4559ad477ad2fa84c171a9)

Currently displaying revision 87f7ac353a4559ad477ad2fa84c171a9 , Show latest

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 \

Places

File curl-7.37.0-CVE-2018-1000120.patch of Package curl (Revision 87f7ac353a4559ad477ad2fa84c171a9)

Places