Commit b3875606 authored by Daniel Stenberg's avatar Daniel Stenberg
Browse files

curl_easy_duphandle: CURLOPT_COPYPOSTFIELDS read out of bounds 

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.

Since the lib/strdup.c file no longer is easily shared with the curl
tool with this change, it now uses its own version instead.

Bug: http://curl.haxx.se/docs/adv_20141105.html
CVE: CVE-2014-3707
Reported-By: Symeon Paraschoudis
parent d997c8b2

lib/formdata.c

+9 −43
Original line number Diff line number Diff line
@@ -36,6 +36,7 @@ 
#include "strequal.h"

#include "curl_memory.h"

#include "sendf.h"

#include "strdup.h"



#define _MPRINTF_REPLACE /* use our functions only */

#include <curl/mprintf.h>
@@ -208,46 +209,6 @@ 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()
@@ -678,9 +639,12 @@ CURLFORMcode FormAdd(struct curl_httppost **httppost, 
           (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;
@@ -691,7 +655,9 @@ CURLFORMcode FormAdd(struct curl_httppost **httppost, 
                            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;

lib/strdup.c

+27 −5
Original line number Diff line number Diff line
@@ -5,7 +5,7 @@ 
 *                            | (__| |_| |  _ <| |___

 *                             \___|\___/|_| \_\_____|

 *

 * 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
@@ -19,12 +19,12 @@ 
 * 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)
@@ -50,3 +50,25 @@ char *curlx_strdup(const char *str) 


}

#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;

}

lib/strdup.h

+2 −1
Original line number Diff line number Diff line
@@ -7,7 +7,7 @@ 
 *                            | (__| |_| |  _ <| |___

 *                             \___|\___/|_| \_\_____|

 *

 * 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
@@ -26,5 +26,6 @@ 
#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 */

lib/url.c

+17 −5
Original line number Diff line number Diff line
@@ -125,6 +125,7 @@ int curl_win32_idn_to_ascii(const char *in, char **out); 
#include "multihandle.h"

#include "pipeline.h"

#include "dotdot.h"

#include "strdup.h"



#define _MPRINTF_REPLACE /* use our functions only */

#include <curl/mprintf.h>
@@ -270,8 +271,9 @@ 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);
@@ -356,14 +358,24 @@ CURLcode Curl_dupset(struct SessionHandle *dst, struct SessionHandle *src) 
  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++) {

    result = setstropt(&dst->set.str[i], src->set.str[i]);

    if(result)

      break;

      return result;

  }



  /* If a failure occurred, freeing has to be performed externally. */

  return result;

  /* 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];

  }



  return CURLE_OK;

}



/*

lib/urldata.h

+9 −2
Original line number Diff line number Diff line
@@ -1372,7 +1372,6 @@ enum dupstring { 
  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 */
@@ -1415,7 +1414,15 @@ enum dupstring { 


  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 */

};