c260e0
From 45a125c1d6aaa2352c5ec04eecba322930e6d169 Mon Sep 17 00:00:00 2001
c260e0
From: Daniel Stenberg <daniel@haxx.se>
c260e0
Date: Fri, 17 Oct 2014 12:59:32 +0200
c260e0
Subject: [PATCH] curl_easy_duphandle: CURLOPT_COPYPOSTFIELDS read out of
c260e0
 bounds
c260e0
c260e0
When duplicating a handle, the data to post was duplicated using
c260e0
strdup() when it could be binary and contain zeroes and it was not even
c260e0
zero terminated! This caused read out of bounds crashes/segfaults.
c260e0
c260e0
Bug: http://curl.haxx.se/docs/adv_20141105.html
c260e0
CVE: CVE-2014-3707
c260e0
Reported-By: Symeon Paraschoudis
c260e0
Upstream-commit: b3875606925536f82fc61f3114ac42f29eaf6945
c260e0
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
c260e0
---
c260e0
 lib/formdata.c   | 50 ++++++++------------------------------------------
c260e0
 lib/strdup.c     | 29 ++++++++++++++++++++++++++---
c260e0
 lib/strdup.h     |  1 +
c260e0
 lib/url.c        | 19 +++++++++++++++----
c260e0
 lib/urldata.h    | 11 +++++++++--
c260e0
 src/Makefile.in  |  3 +--
c260e0
 src/Makefile.inc |  1 -
c260e0
 7 files changed, 60 insertions(+), 54 deletions(-)
c260e0
c260e0
diff --git a/lib/formdata.c b/lib/formdata.c
c260e0
index 3260928..050f538 100644
c260e0
--- a/lib/formdata.c
c260e0
+++ b/lib/formdata.c
c260e0
@@ -39,6 +39,7 @@
c260e0
 #include "strequal.h"
c260e0
 #include "curl_memory.h"
c260e0
 #include "sendf.h"
c260e0
+#include "strdup.h"
c260e0
 
c260e0
 #define _MPRINTF_REPLACE /* use our functions only */
c260e0
 #include <curl/mprintf.h>
c260e0
@@ -216,46 +217,6 @@ static const char * ContentTypeForFilename (const char *filename,
c260e0
 
c260e0
 /***************************************************************************
c260e0
  *
c260e0
- * memdup()
c260e0
- *
c260e0
- * Copies the 'source' data to a newly allocated buffer buffer (that is
c260e0
- * returned). Uses buffer_length if not null, else uses strlen to determine
c260e0
- * the length of the buffer to be copied
c260e0
- *
c260e0
- * Returns the new pointer or NULL on failure.
c260e0
- *
c260e0
- ***************************************************************************/
c260e0
-static char *memdup(const char *src, size_t buffer_length)
c260e0
-{
c260e0
-  size_t length;
c260e0
-  bool add = FALSE;
c260e0
-  char *buffer;
c260e0
-
c260e0
-  if(buffer_length)
c260e0
-    length = buffer_length;
c260e0
-  else if(src) {
c260e0
-    length = strlen(src);
c260e0
-    add = TRUE;
c260e0
-  }
c260e0
-  else
c260e0
-    /* no length and a NULL src pointer! */
c260e0
-    return strdup("");
c260e0
-
c260e0
-  buffer = malloc(length+add);
c260e0
-  if(!buffer)
c260e0
-    return NULL; /* fail */
c260e0
-
c260e0
-  memcpy(buffer, src, length);
c260e0
-
c260e0
-  /* if length unknown do null termination */
c260e0
-  if(add)
c260e0
-    buffer[length] = '\0';
c260e0
-
c260e0
-  return buffer;
c260e0
-}
c260e0
-
c260e0
-/***************************************************************************
c260e0
- *
c260e0
  * FormAdd()
c260e0
  *
c260e0
  * Stores a formpost parameter and builds the appropriate linked list.
c260e0
@@ -684,7 +645,10 @@ CURLFORMcode FormAdd(struct curl_httppost **httppost,
c260e0
              app passed in a bad combo, so we better check for that first. */
c260e0
           if(form->name)
c260e0
             /* copy name (without strdup; possibly contains null characters) */
c260e0
-            form->name = memdup(form->name, form->namelength);
c260e0
+            form->name = Curl_memdup(form->name, form->namelength?
c260e0
+                                     form->namelength:
c260e0
+                                     strlen(form->name)+1);
c260e0
+
c260e0
           if(!form->name) {
c260e0
             return_value = CURL_FORMADD_MEMORY;
c260e0
             break;
c260e0
@@ -695,7 +659,9 @@ CURLFORMcode FormAdd(struct curl_httppost **httppost,
c260e0
                             HTTPPOST_PTRCONTENTS | HTTPPOST_PTRBUFFER |
c260e0
                             HTTPPOST_CALLBACK)) ) {
c260e0
           /* copy value (without strdup; possibly contains null characters) */
c260e0
-          form->value = memdup(form->value, form->contentslength);
c260e0
+          form->value = Curl_memdup(form->value, form->contentslength?
c260e0
+                                    form->contentslength:
c260e0
+                                    strlen(form->value)+1);
c260e0
           if(!form->value) {
c260e0
             return_value = CURL_FORMADD_MEMORY;
c260e0
             break;
c260e0
diff --git a/lib/strdup.c b/lib/strdup.c
c260e0
index 3b776b1..14f370f 100644
c260e0
--- a/lib/strdup.c
c260e0
+++ b/lib/strdup.c
c260e0
@@ -19,12 +19,13 @@
c260e0
  * KIND, either express or implied.
c260e0
  *
c260e0
  ***************************************************************************/
c260e0
-/*
c260e0
- * This file is 'mem-include-scan' clean. See test 1132.
c260e0
- */
c260e0
 #include "curl_setup.h"
c260e0
 
c260e0
 #include "strdup.h"
c260e0
+#include "curl_memory.h"
c260e0
+
c260e0
+/* The last #include file should be: */
c260e0
+#include "memdebug.h"
c260e0
 
c260e0
 #ifndef HAVE_STRDUP
c260e0
 char *curlx_strdup(const char *str)
c260e0
@@ -50,3 +51,25 @@ char *curlx_strdup(const char *str)
c260e0
 
c260e0
 }
c260e0
 #endif
c260e0
+
c260e0
+/***************************************************************************
c260e0
+ *
c260e0
+ * Curl_memdup(source, length)
c260e0
+ *
c260e0
+ * Copies the 'source' data to a newly allocated buffer (that is
c260e0
+ * returned). Copies 'length' bytes.
c260e0
+ *
c260e0
+ * Returns the new pointer or NULL on failure.
c260e0
+ *
c260e0
+ ***************************************************************************/
c260e0
+char *Curl_memdup(const char *src, size_t length)
c260e0
+{
c260e0
+  char *buffer = malloc(length);
c260e0
+  if(!buffer)
c260e0
+    return NULL; /* fail */
c260e0
+
c260e0
+  memcpy(buffer, src, length);
c260e0
+
c260e0
+  /* if length unknown do null termination */
c260e0
+  return buffer;
c260e0
+}
c260e0
diff --git a/lib/strdup.h b/lib/strdup.h
c260e0
index 49af911..36cc430 100644
c260e0
--- a/lib/strdup.h
c260e0
+++ b/lib/strdup.h
c260e0
@@ -26,5 +26,6 @@
c260e0
 #ifndef HAVE_STRDUP
c260e0
 extern char *curlx_strdup(const char *str);
c260e0
 #endif
c260e0
+char *Curl_memdup(const char *src, size_t buffer_length);
c260e0
 
c260e0
 #endif /* HEADER_CURL_STRDUP_H */
c260e0
diff --git a/lib/url.c b/lib/url.c
c260e0
index b0aade1..0aa5a33 100644
c260e0
--- a/lib/url.c
c260e0
+++ b/lib/url.c
c260e0
@@ -123,6 +123,7 @@ int curl_win32_idn_to_ascii(const char *in, char **out);
c260e0
 #include "bundles.h"
c260e0
 #include "conncache.h"
c260e0
 #include "multihandle.h"
c260e0
+#include "strdup.h"
c260e0
 
c260e0
 #define _MPRINTF_REPLACE /* use our functions only */
c260e0
 #include <curl/mprintf.h>
c260e0
@@ -344,14 +345,24 @@ CURLcode Curl_dupset(struct SessionHandle * dst, struct SessionHandle * src)
c260e0
   memset(dst->set.str, 0, STRING_LAST * sizeof(char *));
c260e0
 
c260e0
   /* duplicate all strings */
c260e0
-  for(i=(enum dupstring)0; i< STRING_LAST; i++) {
c260e0
+  for(i=(enum dupstring)0; i< STRING_LASTZEROTERMINATED; i++) {
c260e0
     r = setstropt(&dst->set.str[i], src->set.str[i]);
c260e0
     if(r != CURLE_OK)
c260e0
-      break;
c260e0
+      return r;
c260e0
   }
c260e0
 
c260e0
-  /* If a failure occurred, freeing has to be performed externally. */
c260e0
-  return r;
c260e0
+  /* duplicate memory areas pointed to */
c260e0
+  i = STRING_COPYPOSTFIELDS;
c260e0
+  if(src->set.postfieldsize && src->set.str[i]) {
c260e0
+    /* postfieldsize is curl_off_t, Curl_memdup() takes a size_t ... */
c260e0
+    dst->set.str[i] = Curl_memdup(src->set.str[i], src->set.postfieldsize);
c260e0
+    if(!dst->set.str[i])
c260e0
+      return CURLE_OUT_OF_MEMORY;
c260e0
+    /* point to the new copy */
c260e0
+    dst->set.postfields = dst->set.str[i];
c260e0
+  }
c260e0
+
c260e0
+  return CURLE_OK;
c260e0
 }
c260e0
 
c260e0
 /*
c260e0
diff --git a/lib/urldata.h b/lib/urldata.h
c260e0
index 640cbb1..d03440b 100644
c260e0
--- a/lib/urldata.h
c260e0
+++ b/lib/urldata.h
c260e0
@@ -1337,7 +1337,6 @@ enum dupstring {
c260e0
   STRING_KRB_LEVEL,       /* krb security level */
c260e0
   STRING_NETRC_FILE,      /* if not NULL, use this instead of trying to find
c260e0
                              $HOME/.netrc */
c260e0
-  STRING_COPYPOSTFIELDS,  /* if POST, set the fields' values here */
c260e0
   STRING_PROXY,           /* proxy to use */
c260e0
   STRING_SET_RANGE,       /* range, if used */
c260e0
   STRING_SET_REFERER,     /* custom string for the HTTP referer field */
c260e0
@@ -1376,7 +1375,15 @@ enum dupstring {
c260e0
   STRING_TLSAUTH_PASSWORD,     /* TLS auth <password> */
c260e0
 #endif
c260e0
 
c260e0
-  /* -- end of strings -- */
c260e0
+  /* -- end of zero-terminated strings -- */
c260e0
+
c260e0
+  STRING_LASTZEROTERMINATED,
c260e0
+
c260e0
+  /* -- below this are pointers to binary data that cannot be strdup'ed.
c260e0
+     Each such pointer must be added manually to Curl_dupset() --- */
c260e0
+
c260e0
+  STRING_COPYPOSTFIELDS,  /* if POST, set the fields' values here */
c260e0
+
c260e0
   STRING_LAST /* not used, just an end-of-list marker */
c260e0
 };
c260e0
 
c260e0
diff --git a/src/Makefile.in b/src/Makefile.in
c260e0
index 86e3d3a..74e8b9e 100644
c260e0
--- a/src/Makefile.in
c260e0
+++ b/src/Makefile.in
c260e0
@@ -111,7 +111,7 @@ am__objects_1 = curl-tool_binmode.$(OBJEXT) curl-tool_bname.$(OBJEXT) \
c260e0
 	curl-tool_urlglob.$(OBJEXT) curl-tool_util.$(OBJEXT) \
c260e0
 	curl-tool_vms.$(OBJEXT) curl-tool_writeenv.$(OBJEXT) \
c260e0
 	curl-tool_writeout.$(OBJEXT) curl-tool_xattr.$(OBJEXT)
c260e0
-am__objects_2 = curl-strtoofft.$(OBJEXT) curl-strdup.$(OBJEXT) \
c260e0
+am__objects_2 = curl-strtoofft.$(OBJEXT) \
c260e0
 	curl-rawstr.$(OBJEXT) curl-nonblock.$(OBJEXT)
c260e0
 am__objects_3 =
c260e0
 am_curl_OBJECTS = $(am__objects_1) $(am__objects_2) $(am__objects_3)
c260e0
@@ -376,7 +376,6 @@ AM_CPPFLAGS = -I$(top_builddir)/include/curl -I$(top_builddir)/include \
c260e0
 # the official API, but we re-use the code here to avoid duplication.
c260e0
 CURLX_ONES = \
c260e0
 	../lib/strtoofft.c \
c260e0
-	../lib/strdup.c \
c260e0
 	../lib/rawstr.c \
c260e0
 	../lib/nonblock.c
c260e0
 
c260e0
diff --git a/src/Makefile.inc b/src/Makefile.inc
c260e0
index 3f9044d..ea81000 100644
c260e0
--- a/src/Makefile.inc
c260e0
+++ b/src/Makefile.inc
c260e0
@@ -11,7 +11,6 @@
c260e0
 # the official API, but we re-use the code here to avoid duplication.
c260e0
 CURLX_ONES = \
c260e0
 	../lib/strtoofft.c \
c260e0
-	../lib/strdup.c \
c260e0
 	../lib/rawstr.c \
c260e0
 	../lib/nonblock.c
c260e0
 
c260e0
-- 
c260e0
2.1.0
c260e0