Attachment 948796 Details for Bug 1154941 – preliminary patch from upstream

[patch] preliminary patch from upstream

libcurl-copypostfields-preliminary.patch (text/plain), 10.18 KB, created by Murray McAllister on 2014-10-21 05:14:16 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Murray McAllister

Created: 2014-10-21 05:14:16 UTC

Size: 10.18 KB

patch

obsolete

>From 2697a93c0772c38f32b447d731ffa84285def446 Mon Sep 17 00:00:00 2001
>From: Daniel Stenberg <daniel@haxx.se>
>Date: Fri, 17 Oct 2014 12:59:32 +0200
>Subject: [PATCH] curl_easy_duphandle: fix CURLOPT_COPYPOSTFIELDS duplication
>
>When duplicating a handle, the data to post was duplicated using
>strdup() when it could be binary and contain zeroes and it was not even
>zero terminated! This caused read out of bounds crashes/segfaults.
>
>Reported-By: Symeon Paraschoudis
>---
> lib/formdata.c | 52 +++++++++-------------------------------------------
> lib/strdup.c   | 32 +++++++++++++++++++++++++++-----
> lib/strdup.h   |  3 ++-
> lib/url.c      | 22 +++++++++++++++++-----
> lib/urldata.h  | 11 +++++++++--
> 5 files changed, 64 insertions(+), 56 deletions(-)
>
>diff --git a/lib/formdata.c b/lib/formdata.c
>index a5ee546..2f0201a 100644
>--- a/lib/formdata.c
>+++ b/lib/formdata.c
>@@ -34,10 +34,11 @@
> #include "formdata.h"
> #include "vtls/vtls.h"
> #include "strequal.h"
> #include "curl_memory.h"
> #include "sendf.h"
>+#include "strdup.h"
> 
> #define _MPRINTF_REPLACE /* use our functions only */
> #include <curl/mprintf.h>
> 
> /* The last #include file should be: */
>@@ -208,50 +209,10 @@ static const char *ContentTypeForFilename(const char *filename,
>   return contenttype;
> }
> 
> /***************************************************************************
>  *
>- * memdup()
>- *
>- * Copies the 'source' data to a newly allocated buffer buffer (that is
>- * returned). Uses buffer_length if not null, else uses strlen to determine
>- * the length of the buffer to be copied
>- *
>- * Returns the new pointer or NULL on failure.
>- *
>- ***************************************************************************/
>-static char *memdup(const char *src, size_t buffer_length)
>-{
>-  size_t length;
>-  bool add = FALSE;
>-  char *buffer;
>-
>-  if(buffer_length)
>-    length = buffer_length;
>-  else if(src) {
>-    length = strlen(src);
>-    add = TRUE;
>-  }
>-  else
>-    /* no length and a NULL src pointer! */
>-    return strdup("");
>-
>-  buffer = malloc(length+add);
>-  if(!buffer)
>-    return NULL; /* fail */
>-
>-  memcpy(buffer, src, length);
>-
>-  /* if length unknown do null termination */
>-  if(add)
>-    buffer[length] = '\0';
>-
>-  return buffer;
>-}
>-
>-/***************************************************************************
>- *
>  * FormAdd()
>  *
>  * Stores a formpost parameter and builds the appropriate linked list.
>  *
>  * Has two principal functionalities: using files and byte arrays as
>@@ -676,24 +637,29 @@ CURLFORMcode FormAdd(struct curl_httppost **httppost,
>         }
>         if(!(form->flags & HTTPPOST_PTRNAME) &&
>            (form == first_form) ) {
>           /* Note that there's small risk that form->name is NULL here if the
>              app passed in a bad combo, so we better check for that first. */
>-          if(form->name)
>+          if(form->name) {
>             /* copy name (without strdup; possibly contains null characters) */
>-            form->name = memdup(form->name, form->namelength);
>+            form->name = Curl_memdup(form->name, form->namelength?
>+                                     form->namelength:
>+                                     strlen(form->name)+1);
>+          }
>           if(!form->name) {
>             return_value = CURL_FORMADD_MEMORY;
>             break;
>           }
>           form->name_alloc = TRUE;
>         }
>         if(!(form->flags & (HTTPPOST_FILENAME | HTTPPOST_READFILE |
>                             HTTPPOST_PTRCONTENTS | HTTPPOST_PTRBUFFER |
>                             HTTPPOST_CALLBACK)) && form->value) {
>           /* copy value (without strdup; possibly contains null characters) */
>-          form->value = memdup(form->value, form->contentslength);
>+          form->value = Curl_memdup(form->value, form->contentslength?
>+                                    form->contentslength:
>+                                    strlen(form->value)+1);
>           if(!form->value) {
>             return_value = CURL_FORMADD_MEMORY;
>             break;
>           }
>           form->value_alloc = TRUE;
>diff --git a/lib/strdup.c b/lib/strdup.c
>index 3b776b1..4b5bd40 100644
>--- a/lib/strdup.c
>+++ b/lib/strdup.c
>@@ -3,11 +3,11 @@
>  *  Project                     ___| | | |  _ \| |
>  *                             / __| | | | |_) | |
>  *                            | (__| |_| |  _ <| |___
>  *                             \___|\___/|_| \_\_____|
>  *
>- * Copyright (C) 1998 - 2012, Daniel Stenberg, <daniel@haxx.se>, et al.
>+ * Copyright (C) 1998 - 2014, Daniel Stenberg, <daniel@haxx.se>, et al.
>  *
>  * This software is licensed as described in the file COPYING, which
>  * you should have received as part of this distribution. The terms
>  * are also available at http://curl.haxx.se/docs/copyright.html.
>  *
>@@ -17,16 +17,16 @@
>  *
>  * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
>  * KIND, either express or implied.
>  *
>  ***************************************************************************/
>-/*
>- * This file is 'mem-include-scan' clean. See test 1132.
>- */
> #include "curl_setup.h"
>-
> #include "strdup.h"
>+#include "curl_memory.h"
>+
>+/* The last #include file should be: */
>+#include "memdebug.h"
> 
> #ifndef HAVE_STRDUP
> char *curlx_strdup(const char *str)
> {
>   size_t len;
>@@ -48,5 +48,27 @@ char *curlx_strdup(const char *str)
> 
>   return newstr;
> 
> }
> #endif
>+
>+/***************************************************************************
>+ *
>+ * Curl_memdup(source, length)
>+ *
>+ * Copies the 'source' data to a newly allocated buffer (that is
>+ * returned). Copies 'length' bytes.
>+ *
>+ * Returns the new pointer or NULL on failure.
>+ *
>+ ***************************************************************************/
>+char *Curl_memdup(const char *src, size_t length)
>+{
>+  char *buffer = malloc(length);
>+  if(!buffer)
>+    return NULL; /* fail */
>+
>+  memcpy(buffer, src, length);
>+
>+  /* if length unknown do null termination */
>+  return buffer;
>+}
>diff --git a/lib/strdup.h b/lib/strdup.h
>index 49af911..23a71f8 100644
>--- a/lib/strdup.h
>+++ b/lib/strdup.h
>@@ -5,11 +5,11 @@
>  *  Project                     ___| | | |  _ \| |
>  *                             / __| | | | |_) | |
>  *                            | (__| |_| |  _ <| |___
>  *                             \___|\___/|_| \_\_____|
>  *
>- * Copyright (C) 1998 - 2010, Daniel Stenberg, <daniel@haxx.se>, et al.
>+ * Copyright (C) 1998 - 2014, Daniel Stenberg, <daniel@haxx.se>, et al.
>  *
>  * This software is licensed as described in the file COPYING, which
>  * you should have received as part of this distribution. The terms
>  * are also available at http://curl.haxx.se/docs/copyright.html.
>  *
>@@ -24,7 +24,8 @@
> #include "curl_setup.h"
> 
> #ifndef HAVE_STRDUP
> extern char *curlx_strdup(const char *str);
> #endif
>+char *Curl_memdup(const char *src, size_t buffer_length);
> 
> #endif /* HEADER_CURL_STRDUP_H */
>diff --git a/lib/url.c b/lib/url.c
>index 6db79de..a06f433 100644
>--- a/lib/url.c
>+++ b/lib/url.c
>@@ -123,10 +123,11 @@ int curl_win32_idn_to_ascii(const char *in, char **out);
> #include "bundles.h"
> #include "conncache.h"
> #include "multihandle.h"
> #include "pipeline.h"
> #include "dotdot.h"
>+#include "strdup.h"
> 
> #define _MPRINTF_REPLACE /* use our functions only */
> #include <curl/mprintf.h>
> 
> #include "curl_memory.h"
>@@ -268,12 +269,13 @@ static const struct Curl_handler Curl_handler_dummy = {
> 
> void Curl_freeset(struct SessionHandle *data)
> {
>   /* Free all dynamic strings stored in the data->set substructure. */
>   enum dupstring i;
>-  for(i=(enum dupstring)0; i < STRING_LAST; i++)
>+  for(i=(enum dupstring)0; i < STRING_LAST; i++) {
>     Curl_safefree(data->set.str[i]);
>+  }
> 
>   if(data->change.referer_alloc) {
>     Curl_safefree(data->change.referer);
>     data->change.referer_alloc = FALSE;
>   }
>@@ -354,18 +356,28 @@ CURLcode Curl_dupset(struct SessionHandle *dst, struct SessionHandle *src)
> 
>   /* clear all string pointers first */
>   memset(dst->set.str, 0, STRING_LAST * sizeof(char *));
> 
>   /* duplicate all strings */
>-  for(i=(enum dupstring)0; i< STRING_LAST; i++) {
>+  for(i=(enum dupstring)0; i< STRING_LASTZEROTERMINATED; i++) {
>     r = setstropt(&dst->set.str[i], src->set.str[i]);
>     if(r != CURLE_OK)
>-      break;
>+      return r;
>+  }
>+
>+  /* duplicate memory areas pointed to */
>+  i = STRING_COPYPOSTFIELDS;
>+  if(src->set.postfieldsize && src->set.str[i]) {
>+    /* postfieldsize is curl_off_t, Curl_memdup() takes a size_t ... */
>+    dst->set.str[i] = Curl_memdup(src->set.str[i], src->set.postfieldsize);
>+    if(!dst->set.str[i])
>+      return CURLE_OUT_OF_MEMORY;
>+    /* point to the new copy */
>+    dst->set.postfields = dst->set.str[i];
>   }
> 
>-  /* If a failure occurred, freeing has to be performed externally. */
>-  return r;
>+  return CURLE_OK;
> }
> 
> /*
>  * This is the internal function curl_easy_cleanup() calls. This should
>  * cleanup and free all resources associated with this sessionhandle.
>diff --git a/lib/urldata.h b/lib/urldata.h
>index 900b8c8..1068f14 100644
>--- a/lib/urldata.h
>+++ b/lib/urldata.h
>@@ -1369,11 +1369,10 @@ enum dupstring {
>   STRING_KEY_PASSWD,      /* plain text private key password */
>   STRING_KEY_TYPE,        /* format for private key (default: PEM) */
>   STRING_KRB_LEVEL,       /* krb security level */
>   STRING_NETRC_FILE,      /* if not NULL, use this instead of trying to find
>                              $HOME/.netrc */
>-  STRING_COPYPOSTFIELDS,  /* if POST, set the fields' values here */
>   STRING_PROXY,           /* proxy to use */
>   STRING_SET_RANGE,       /* range, if used */
>   STRING_SET_REFERER,     /* custom string for the HTTP referer field */
>   STRING_SET_URL,         /* what original URL to work on */
>   STRING_SSL_CAPATH,      /* CA directory name (doesn't work on windows) */
>@@ -1412,11 +1411,19 @@ enum dupstring {
>   STRING_TLSAUTH_PASSWORD,     /* TLS auth <password> */
> #endif
> 
>   STRING_BEARER,          /* <bearer>, if used */
> 
>-  /* -- end of strings -- */
>+  /* -- end of zero-terminated strings -- */
>+
>+  STRING_LASTZEROTERMINATED,
>+
>+  /* -- below this are pointers to binary data that cannot be strdup'ed.
>+     Each such pointer must be added manually to Curl_dupset() --- */
>+
>+  STRING_COPYPOSTFIELDS,  /* if POST, set the fields' values here */
>+
>   STRING_LAST /* not used, just an end-of-list marker */
> };
> 
> struct UserDefined {
>   FILE *err;         /* the stderr user data goes here */
>-- 
>2.1.1
>

Actions: View | Diff

Attachments on bug 1154941: 948796