[Bug-wget] [PATCH] Fix stack overflow with way too many cookies.

[Tobias Stoeckmann]

If wget supports cookies, which is the default, it will eventually sort
them based on their domain, path, and name attributes. In order to
perform this sorting, quick sort is used. And for this, an array
containing pointers to all relevant cookies is constructed on the stack
with alloca().

If wget has to handle an insanely large amount of cookies (~700,000 on
32 bit systems or ~530,000 on 64 bit systems), the stack is not large
enough to hold these pointers, leading to undefined behaviour according
to POSIX; expect a segmentation fault in real life. ;)

This is a very unlikely thing to happen. Either you have to create a
cookie file that contains so many cookies, in which case it's your fault!
Or you are running "wget -m http://some_host";. In that case, an evil
server could exploit wget's default infinite recursion, adding cookies
in every single HTTP response to wget. This way, the cookie storage will
slowly fill up and wget could crash.

I write "could crash" because the runtime performance of cookie handling
is clearly not designed for such a large amount of cookies. It's like
approaching the event horizon of a black hole due to O(n^2). ;)

This command would create a crashing cookie-file and calls wget with it:

$ for i in $(seq 1 700000)
do
  echo "localhost:8080  FALSE   /       FALSE   0       $i      1"
done > cookies
$ wget --load-cookies cookies http://localhost:8080

If you want to see that something happens, I recommend to disable the
qsort() calls in src/cookies.c as well as the find_matching_cookie call.
This cookie file guarantees that the cookies are unique.

My patch moves the memory handling to the heap. But I think it would
also be totally sufficient to add a constant like COOKIE_MAX to avoid
overflows on the stack.

Signed-off-by: Tobias Stoeckmann <address@hidden>
---
 src/cookies.c | 10 ++++++++--
 src/http.c    |  7 +++++--
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/cookies.c b/src/cookies.c
index 81ecfa5..624c9ff 100644
--- a/src/cookies.c
+++ b/src/cookies.c
@@ -45,6 +45,7 @@ as that of the covered work.  */
 
 #include "wget.h"
 
+#include <limits.h>
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
@@ -1018,7 +1019,7 @@ cookie_header (struct cookie_jar *jar, const char *host,
 
   struct cookie *cookie;
   struct weighed_cookie *outgoing;
-  int count, i, ocnt;
+  size_t count, i, ocnt;
   char *result;
   int result_size, pos;
   PREPEND_SLASH (path);         /* see cookie_handle_set_cookie */
@@ -1051,7 +1052,11 @@ cookie_header (struct cookie_jar *jar, const char *host,
     return NULL;                /* no cookies matched */
 
   /* Allocate the array. */
-  outgoing = alloca_array (struct weighed_cookie, count);
+  if (count > SIZE_MAX / sizeof (struct weighed_cookie))
+    return NULL;                /* unable to process so many cookies */
+  outgoing = malloc(count * sizeof (struct weighed_cookie));
+  if (outgoing == NULL)
+    return NULL;               /* out of memory */
 
   /* Fill the array with all the matching cookies from the chains that
      match HOST. */
@@ -1111,6 +1116,7 @@ cookie_header (struct cookie_jar *jar, const char *host,
         }
     }
   result[pos++] = '\0';
+  free(outgoing);
   assert (pos == result_size);
   return result;
 }
diff --git a/src/http.c b/src/http.c
index 1091121..cf6d7a9 100644
--- a/src/http.c
+++ b/src/http.c
@@ -344,7 +344,9 @@ request_send (const struct request *req, int fd, FILE 
*warc_tmp)
   /* "\r\n\0" */
   size += 3;
 
-  p = request_string = alloca_array (char, size);
+  p = request_string = malloc (size);
+  if (request_string == NULL)
+    return -1;
 
   /* Generate the request. */
 
@@ -379,8 +381,9 @@ request_send (const struct request *req, int fd, FILE 
*warc_tmp)
       /* Write a copy of the data to the WARC record. */
       int warc_tmp_written = fwrite (request_string, 1, size - 1, warc_tmp);
       if (warc_tmp_written != size - 1)
-        return -2;
+        write_error = -2;
     }
+  free(request_string);
   return write_error;
 }
 
-- 
2.9.2