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