]> granicus.if.org Git - curl/commitdiff
formpost: better random boundaries
authorDaniel Stenberg <daniel@haxx.se>
Mon, 24 Jun 2013 20:24:35 +0000 (22:24 +0200)
committerDaniel Stenberg <daniel@haxx.se>
Tue, 25 Jun 2013 07:55:49 +0000 (09:55 +0200)
When doing multi-part formposts, libcurl used a pseudo-random value that
was seeded with time(). This turns out to be bad for users who formpost
data that is provided with users who then can guess how the boundary
string will look like and then they can forge a different formpost part
and trick the receiver.

My advice to such implementors is (still even after this change) to not
rely on the boundary strings being cryptographically strong. Fix your
code and logic to not depend on them that much!

I moved the Curl_rand() function into the sslgen.c source file now to be
able to take advantage of the SSL library's random function if it
provides one. If not, try to use the RANDOM_FILE for seeding and as a
last resort keep the old logic, just modified to also add microseconds
which makes it harder to properly guess the exact seed.

The formboundary() function in formdata.c is now using 64 bit entropy
for the boundary and therefore the string of dashes was reduced by 4
letters and there are 16 hex digits following it. The total length is
thus still the same.

Bug: http://curl.haxx.se/bug/view.cgi?id=1251
Reported-by: "Floris"
16 files changed:
lib/Makefile.inc
lib/curl_darwinssl.h
lib/curl_rand.c [deleted file]
lib/curl_rand.h [deleted file]
lib/curl_sasl.c
lib/easy.c
lib/formdata.c
lib/gtls.h
lib/nssg.h
lib/sslgen.c
lib/sslgen.h
lib/ssluse.h
tests/data/test158
tests/data/test277
tests/data/test554
tests/data/test587

index 4228bf6b842e1e28c4ab9dd5306e07af05d55bdf..f3845a0fbabdb0523594d775a64e200012024bc3 100644 (file)
@@ -13,7 +13,7 @@ CSOURCES = file.c timeval.c base64.c hostip.c progress.c formdata.c   \
   netrc.c getinfo.c transfer.c strequal.c easy.c security.c krb4.c     \
   curl_fnmatch.c fileinfo.c ftplistparser.c wildcard.c krb5.c          \
   memdebug.c http_chunks.c strtok.c connect.c llist.c hash.c multi.c   \
-  content_encoding.c share.c http_digest.c md4.c md5.c curl_rand.c     \
+  content_encoding.c share.c http_digest.c md4.c md5.c \
   http_negotiate.c inet_pton.c strtoofft.c strerror.c amigaos.c                \
   hostasyn.c hostip4.c hostip6.c hostsyn.c inet_ntop.c parsedate.c     \
   select.c gtls.c sslgen.c tftp.c splay.c strdup.c socks.c ssh.c nss.c \
@@ -30,7 +30,7 @@ CSOURCES = file.c timeval.c base64.c hostip.c progress.c formdata.c   \
 HHEADERS = arpa_telnet.h netrc.h file.h timeval.h qssl.h hostip.h      \
   progress.h formdata.h cookie.h http.h sendf.h ftp.h url.h dict.h     \
   if2ip.h speedcheck.h urldata.h curl_ldap.h ssluse.h escape.h telnet.h        \
-  getinfo.h strequal.h krb4.h memdebug.h http_chunks.h curl_rand.h     \
+  getinfo.h strequal.h krb4.h memdebug.h http_chunks.h \
   curl_fnmatch.h wildcard.h fileinfo.h ftplistparser.h strtok.h                \
   connect.h llist.h hash.h content_encoding.h share.h curl_md4.h       \
   curl_md5.h http_digest.h http_negotiate.h inet_pton.h amigaos.h      \
index 4c0e53ff10de585306258fb5732f88928081cc4d..432d3d7ce404c0a30b6fccb656683437b5a2ad41 100644 (file)
@@ -7,7 +7,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 2012, Nick Zitzmann, <nickzman@gmail.com>.
+ * Copyright (C) 2012 - 2013, Nick Zitzmann, <nickzman@gmail.com>.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -52,6 +52,10 @@ void Curl_darwinssl_md5sum(unsigned char *tmp, /* input */
                            unsigned char *md5sum, /* output */
                            size_t md5len);
 
+/* this backend provides these functions: */
+#define have_curlssl_random 1
+#define have_curlssl_md5sum 1
+
 /* API setup for SecureTransport */
 #define curlssl_init() (1)
 #define curlssl_cleanup() Curl_nop_stmt
diff --git a/lib/curl_rand.c b/lib/curl_rand.c
deleted file mode 100644 (file)
index dc49289..0000000
+++ /dev/null
@@ -1,61 +0,0 @@
-/***************************************************************************
- *                                  _   _ ____  _
- *  Project                     ___| | | |  _ \| |
- *                             / __| | | | |_) | |
- *                            | (__| |_| |  _ <| |___
- *                             \___|\___/|_| \_\_____|
- *
- * Copyright (C) 1998 - 2009, 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.
- *
- * You may opt to use, copy, modify, merge, publish, distribute and/or sell
- * copies of the Software, and permit persons to whom the Software is
- * furnished to do so, under the terms of the COPYING file.
- *
- * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
- * KIND, either express or implied.
- *
- ***************************************************************************/
-
-#include "curl_setup.h"
-
-#include <curl/curl.h>
-
-#include "curl_rand.h"
-
-#define _MPRINTF_REPLACE /* use our functions only */
-#include <curl/mprintf.h>
-
-#include "curl_memory.h"
-/* The last #include file should be: */
-#include "memdebug.h"
-
-/* Private pseudo-random number seed. Unsigned integer >= 32bit. Threads
-   mutual exclusion is not implemented to acess it since we do not require
-   high quality random numbers (only used in form boudary generation). */
-
-static unsigned int randseed;
-
-/* Pseudo-random number support. */
-
-unsigned int Curl_rand(void)
-{
-  unsigned int r;
-  /* Return an unsigned 32-bit pseudo-random number. */
-  r = randseed = randseed * 1103515245 + 12345;
-  return (r << 16) | ((r >> 16) & 0xFFFF);
-}
-
-void Curl_srand(void)
-{
-  /* Randomize pseudo-random number sequence. */
-
-  randseed = (unsigned int) time(NULL);
-  Curl_rand();
-  Curl_rand();
-  Curl_rand();
-}
-
diff --git a/lib/curl_rand.h b/lib/curl_rand.h
deleted file mode 100644 (file)
index 26cfb7f..0000000
+++ /dev/null
@@ -1,29 +0,0 @@
-#ifndef HEADER_CURL_RAND_H
-#define HEADER_CURL_RAND_H
-/***************************************************************************
- *                                  _   _ ____  _
- *  Project                     ___| | | |  _ \| |
- *                             / __| | | | |_) | |
- *                            | (__| |_| |  _ <| |___
- *                             \___|\___/|_| \_\_____|
- *
- * Copyright (C) 1998 - 2009, 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.
- *
- * You may opt to use, copy, modify, merge, publish, distribute and/or sell
- * copies of the Software, and permit persons to whom the Software is
- * furnished to do so, under the terms of the COPYING file.
- *
- * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
- * KIND, either express or implied.
- *
- ***************************************************************************/
-
-void Curl_srand(void);
-
-unsigned int Curl_rand(void);
-
-#endif /* HEADER_CURL_RAND_H */
index 39d451a3701b47dd7612bcf4c4d5c40fa9c2b7ca..fcb001948a34e1cce322b7ce05dd06b9796c25e5 100644 (file)
@@ -32,7 +32,7 @@
 
 #include "curl_base64.h"
 #include "curl_md5.h"
-#include "curl_rand.h"
+#include "sslgen.h"
 #include "curl_hmac.h"
 #include "curl_ntlm_msgs.h"
 #include "curl_sasl.h"
@@ -314,7 +314,7 @@ CURLcode Curl_sasl_create_digest_md5_message(struct SessionHandle *data,
 
   /* Generate 64 bits of random data */
   for(i = 0; i < 8; i++)
-    cnonce[i] = table16[Curl_rand()%16];
+    cnonce[i] = table16[Curl_rand(data)%16];
 
   /* So far so good, now calculate A1 and H(A1) according to RFC 2831 */
   ctxt = Curl_MD5_init(Curl_DIGEST_MD5);
index 51da1c254f655c611e0bdd37cc2769001bcb93c1..9d4e01176553057969ff2ca1f2f1ac0b097861b1 100644 (file)
@@ -74,7 +74,6 @@
 #include "connect.h" /* for Curl_getconnectinfo */
 #include "slist.h"
 #include "amigaos.h"
-#include "curl_rand.h"
 #include "non-ascii.h"
 #include "warnless.h"
 #include "conncache.h"
@@ -330,10 +329,6 @@ CURLcode curl_global_init(long flags)
 
   init_flags  = flags;
 
-  /* Preset pseudo-random number sequence. */
-
-  Curl_srand();
-
   return CURLE_OK;
 }
 
index 49c5d2943b8ce887ce75876c3041939eec1439d1..decb84d9f453807e6f02dcf01a3ca5d6d869b929 100644 (file)
@@ -24,9 +24,6 @@
 
 #include <curl/curl.h>
 
-/* Length of the random boundary string. */
-#define BOUNDARY_LENGTH 40
-
 #if !defined(CURL_DISABLE_HTTP) || defined(USE_SSLEAY)
 
 #if defined(HAVE_LIBGEN_H) && defined(HAVE_BASENAME)
@@ -35,7 +32,7 @@
 
 #include "urldata.h" /* for struct SessionHandle */
 #include "formdata.h"
-#include "curl_rand.h"
+#include "sslgen.h"
 #include "strequal.h"
 #include "curl_memory.h"
 #include "sendf.h"
@@ -56,7 +53,7 @@ static char *Curl_basename(char *path);
 #endif
 
 static size_t readfromfile(struct Form *form, char *buffer, size_t size);
-static char *formboundary(void);
+static char *formboundary(struct SessionHandle *data);
 
 /* What kind of Content-Type to use on un-specified files with unrecognized
    extensions. */
@@ -1101,7 +1098,7 @@ CURLcode Curl_getformdata(struct SessionHandle *data,
   if(!post)
     return result; /* no input => no output! */
 
-  boundary = formboundary();
+  boundary = formboundary(data);
   if(!boundary)
     return CURLE_OUT_OF_MEMORY;
 
@@ -1157,7 +1154,7 @@ CURLcode Curl_getformdata(struct SessionHandle *data,
          the magic to include several files with the same field name */
 
       Curl_safefree(fileboundary);
-      fileboundary = formboundary();
+      fileboundary = formboundary(data);
       if(!fileboundary) {
         result = CURLE_OUT_OF_MEMORY;
         break;
@@ -1464,28 +1461,12 @@ char *Curl_formpostheader(void *formp, size_t *len)
  * formboundary() creates a suitable boundary string and returns an allocated
  * one.
  */
-static char *formboundary(void)
+static char *formboundary(struct SessionHandle *data)
 {
-  char *retstring;
-  size_t i;
-
-  static const char table16[]="0123456789abcdef";
-
-  retstring = malloc(BOUNDARY_LENGTH+1);
-
-  if(!retstring)
-    return NULL; /* failed */
-
-  strcpy(retstring, "----------------------------");
-
-  for(i=strlen(retstring); i<BOUNDARY_LENGTH; i++)
-    retstring[i] = table16[Curl_rand()%16];
-
-  /* 28 dashes and 12 hexadecimal digits makes 12^16 (184884258895036416)
+  /* 24 dashes and 16 hexadecimal digits makes 64 bit (18446744073709551615)
      combinations */
-  retstring[BOUNDARY_LENGTH]=0; /* zero terminate */
-
-  return retstring;
+  return aprintf("------------------------%08x%08x",
+                 Curl_rand(data), Curl_rand(data));
 }
 
 #else  /* CURL_DISABLE_HTTP */
index 84e1396b56ed7065442755f2e6729e3875702a93..453542e1ec99b35d5e642e53c0284f12c57b54bf 100644 (file)
@@ -7,7 +7,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2012, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2013, 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
@@ -55,6 +55,10 @@ void Curl_gtls_md5sum(unsigned char *tmp, /* input */
                       unsigned char *md5sum, /* output */
                       size_t md5len);
 
+/* this backend provides these functions: */
+#define have_curlssl_random 1
+#define have_curlssl_md5sum 1
+
 /* API setup for GnuTLS */
 #define curlssl_init Curl_gtls_init
 #define curlssl_cleanup Curl_gtls_cleanup
index a881a9ad2f2426781add4a52f05635b6ca901712..cd32706a717c1c65b6c13089fe5d2e8569977e09 100644 (file)
@@ -7,7 +7,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2012, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2013, 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
@@ -60,6 +60,10 @@ void Curl_nss_md5sum(unsigned char *tmp, /* input */
                      unsigned char *md5sum, /* output */
                      size_t md5len);
 
+/* this backend provides these functions: */
+#define have_curlssl_random 1
+#define have_curlssl_md5sum 1
+
 /* API setup for NSS */
 #define curlssl_init Curl_nss_init
 #define curlssl_cleanup Curl_nss_cleanup
index 48758742a2072c8a5a50e23fbb273a325655cbfe..ba995cb5deb103a6c64633f4ea6f5a9a78b290ec 100644 (file)
@@ -5,7 +5,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2012, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2013, 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
 
 #include "curl_setup.h"
 
+#ifdef HAVE_SYS_TYPES_H
+#include <sys/types.h>
+#endif
+#ifdef HAVE_SYS_STAT_H
+#include <sys/stat.h>
+#endif
+#ifdef HAVE_FCNTL_H
+#include <fcntl.h>
+#endif
+
 #include "urldata.h"
 #define SSLGEN_C
 #include "sslgen.h" /* generic SSL protos etc */
@@ -63,6 +73,7 @@
 #include "curl_memory.h"
 #include "progress.h"
 #include "share.h"
+#include "timeval.h"
 /* The last #include file should be: */
 #include "memdebug.h"
 
@@ -159,6 +170,63 @@ void Curl_free_ssl_config(struct ssl_config_data* sslc)
   Curl_safefree(sslc->random_file);
 }
 
+
+/*
+ * Curl_rand() returns a random unsigned integer, 32bit.
+ *
+ * This non-SSL function is put here only because this file is the only one
+ * with knowledge of what the underlying SSL libraries provide in terms of
+ * randomizers.
+ *
+ * NOTE: 'data' may be passed in as NULL when coming from external API without
+ * easy handle!
+ *
+ */
+
+unsigned int Curl_rand(struct SessionHandle *data)
+{
+  unsigned int r;
+  static unsigned int randseed;
+  static bool seeded;
+
+#ifdef have_curlssl_random
+  if(!data) {
+#endif
+
+    if(!seeded) {
+
+#ifdef RANDOM_FILE
+      /* if there's a random file to read a seed from, use it */
+      int fd = open(RANDOM_FILE, O_RDONLY);
+      seeded = TRUE;
+      if(fd > -1) {
+        /* read random data into the randseed variable */
+        read(fd, &randseed, sizeof(randseed));
+        close(fd);
+      }
+      else
+#endif /* RANDOM_FILE */
+      {
+        struct timeval now = curlx_tvnow();
+        randseed += (unsigned int) now.tv_usec + (unsigned int)now.tv_sec;
+        Curl_rand(data);
+        Curl_rand(data);
+        Curl_rand(data);
+      }
+    }
+    /* Return an unsigned 32-bit pseudo-random number. */
+    r = randseed = randseed * 1103515245 + 12345;
+    return (r << 16) | ((r >> 16) & 0xFFFF);
+
+#ifdef have_curlssl_random
+  }
+  else {
+    Curl_ssl_random(data, (unsigned char *)&r, sizeof(r));
+    return r;
+  }
+#endif
+}
+
 #ifdef USE_SSL
 
 /* "global" init done? */
@@ -518,17 +586,18 @@ void Curl_ssl_free_certinfo(struct SessionHandle *data)
   }
 }
 
-#if defined(USE_SSLEAY) || defined(USE_GNUTLS) || defined(USE_NSS) || \
-    defined(USE_DARWINSSL)
-/* these functions are only used by some SSL backends */
+/* these functions are only provided by some SSL backends */
 
+#ifdef have_curlssl_random
 void Curl_ssl_random(struct SessionHandle *data,
                      unsigned char *entropy,
                      size_t length)
 {
   curlssl_random(data, entropy, length);
 }
+#endif
 
+#ifdef have_curlssl_md5sum
 void Curl_ssl_md5sum(unsigned char *tmp, /* input */
                      size_t tmplen,
                      unsigned char *md5sum, /* output */
@@ -536,6 +605,6 @@ void Curl_ssl_md5sum(unsigned char *tmp, /* input */
 {
   curlssl_md5sum(tmp, tmplen, md5sum, md5len);
 }
-#endif /* USE_SSLEAY || USE_GNUTLS || USE_NSS || USE_DARWINSSL */
+#endif
 
 #endif /* USE_SSL */
index 17ad8e343d8283c4df7497f475210a2f513f4c1f..182dd8f2444cb5c09f2775e79f9f3225315ed283 100644 (file)
@@ -33,6 +33,8 @@ bool Curl_clone_ssl_config(struct ssl_config_data* source,
                            struct ssl_config_data* dest);
 void Curl_free_ssl_config(struct ssl_config_data* sslc);
 
+unsigned int Curl_rand(struct SessionHandle *);
+
 #ifdef USE_SSL
 int Curl_ssl_init(void);
 void Curl_ssl_cleanup(void);
@@ -83,6 +85,13 @@ void Curl_ssl_md5sum(unsigned char *tmp, /* input */
 
 #define SSL_SHUTDOWN_TIMEOUT 10000 /* ms */
 
+#ifdef have_curlssl_random
+#define HAVE_CURL_SSL_RANDOM
+#endif
+#ifdef have_curlssl_md5sum
+#define HAVE_CURL_SSL_MD5SUM
+#endif
+
 #else
 /* When SSL support is not present, just define away these function calls */
 #define Curl_ssl_init() 1
index bdec2b513eb123cd0964c9b087087824b2b3f98f..d6efcb2717cc027535db922c66ffaae5c4e7ee08 100644 (file)
@@ -7,7 +7,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2012, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2013, 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
@@ -73,6 +73,10 @@ void Curl_ossl_md5sum(unsigned char *tmp, /* input */
                       unsigned char *md5sum /* output */,
                       size_t unused);
 
+/* this backend provides these functions: */
+#define have_curlssl_random 1
+#define have_curlssl_md5sum 1
+
 /* API setup for OpenSSL */
 #define curlssl_init Curl_ossl_init
 #define curlssl_cleanup Curl_ossl_cleanup
index 5cbc97ab65c57a8b959e968898636275275d5b9b..9c4b22f70751645f070a888173663266ab8d61e0 100644 (file)
@@ -33,7 +33,7 @@ http://%HOSTIP:%HTTPPORT/158 -F name=daniel
 <strip>
 ^User-Agent:.*
 ^Content-Type: multipart/form-data.*
-^---------------------------.*
+^-----------------------.*
 </strip>
 <protocol>
 POST /158 HTTP/1.1\r
index 18e419850c5f48756aa7009dcebd1498c016a764..a509b40ec38e57d0ba8503266b9b32a89019fe32 100644 (file)
@@ -37,8 +37,8 @@ http://%HOSTIP:%HTTPPORT/want/277 -F name=daniel -H "Content-Type: text/info"
 ^User-Agent:.*
 </strip>
 <strippart>
-s/^------------------------------[a-z0-9]*/------------------------------/
-s/boundary=----------------------------[a-z0-9]*/boundary=----------------------------/
+s/^--------------------------[a-z0-9]*/--------------------------/
+s/boundary=------------------------[a-z0-9]*/boundary=------------------------/
 </strippart>
 <protocol>
 POST /want/277 HTTP/1.1\r
@@ -47,13 +47,13 @@ Host: %HOSTIP:%HTTPPORT
 Accept: */*\r
 Content-Length: 145\r
 Expect: 100-continue\r
-Content-Type: text/info; boundary=----------------------------\r
+Content-Type: text/info; boundary=------------------------\r
 \r
-------------------------------\r
+--------------------------\r
 Content-Disposition: form-data; name="name"\r
 \r
 daniel\r
---------------------------------\r
+----------------------------\r
 </protocol>
 </verify>
 </testcase>
index 9d9bbcca74b211ddc93a82bf86f1bc6dad2e94ba..8c6b762ef72a7bd1a5d4beb92d1dce4910b1bdd0 100644 (file)
@@ -35,8 +35,8 @@ http://%HOSTIP:%HTTPPORT/554
 # Verify data after the test has been "shot"
 <verify>
 <strippart>
-s/^------------------------------[a-z0-9]*/------------------------------/
-s/boundary=----------------------------[a-z0-9]*/boundary=----------------------------/
+s/^--------------------------[a-z0-9]*/------------------------------/
+s/boundary=------------------------[a-z0-9]*/boundary=----------------------------/
 </strippart>
 # Note that the stripping above removes 12 bytes from every occurance of the
 # boundary string and since 5 of them are in the body contents, we see
index 6e1239a6ab5c5dc91612d697be2948a6662ea3c6..d936372c5ea56729db09d0fc99c8379fdb139bd3 100644 (file)
@@ -28,8 +28,8 @@ http://%HOSTIP:%HTTPPORT/587
 # Verify data after the test has been "shot"
 <verify>
 <strippart>
-s/^------------------------------[a-z0-9]*/------------------------------/
-s/boundary=----------------------------[a-z0-9]*/boundary=----------------------------/
+s/^--------------------------[a-z0-9]*/------------------------------/
+s/boundary=------------------------[a-z0-9]*/boundary=----------------------------/
 </strippart>
 <protocol>
 POST /587 HTTP/1.1\r