renable deflate frame buffer expansion fixing DoS 86/2886/1
authorAndy Green <andy.green@linaro.org>
Sat, 12 Jan 2013 15:09:36 +0000 (23:09 +0800)
committerKevron Rees <kevron_m_rees@linux.intel.com>
Thu, 7 Mar 2013 21:01:24 +0000 (13:01 -0800)
This reverts the removal of the deflate_frame code that was crashing after porting
David Galeano's code: he pointed out there's a typo in the merged version causing
the crash which is fixed here.

However the fixed code has a problem, there's no limit (other than int size) to the
amount of memory it will try to malloc, which can allow a DoS of the server by the
client sending malicious compression states that inflate to a large amount.  I have
added checking for OOM already that will avert the segfault that would otherwise follow
but the server will be unusuable if malicious connections were made repeatedly each
forcing it to allocate large buffers and cause small allocations on other connections
to fail.

The patch changes the code to use realloc(), and introduces a configurable limit
on the amount of memory one connection may need for zlib before the server hangs
up the connection.  It defaults to 64KBytes but can be set from ./configure as
described now in the README.

Signed-off-by: Andy Green <andy.green@linaro.org>
README-test-server
lib/extension-deflate-frame.c

index a2ce210..f580672 100644 (file)
@@ -70,6 +70,21 @@ There are a couple of other possible configure options
                        one is used.
 
 
+Externally configurable important constants
+-------------------------------------------
+
+You can control these from configure by just setting them as commandline
+args throgh CFLAGS, eg
+
+./configure CFLAGS="-DLWS_MAX_ZLIB_CONN_BUFFER=8192"
+
+They all have defaults so you only need to take care about them if you want
+to tune them to the amount of memory available.
+
+ - LWS_MAX_ZLIB_CONN_BUFFER  maximum size a compression buffer is allowed to
+grow to before closing the connection.  Default is 64KBytes.
+
+
 Testing server with a browser
 -----------------------------
 
index c8ba423..a96d996 100644 (file)
@@ -9,6 +9,9 @@
 
 #define MIN_SIZE_TO_DEFLATE 4
 
+#ifndef LWS_MAX_ZLIB_CONN_BUFFER
+#define LWS_MAX_ZLIB_CONN_BUFFER (64 * 1024)
+#endif
 
 int lws_extension_callback_deflate_frame(
                struct libwebsocket_context *context,
@@ -22,6 +25,7 @@ int lws_extension_callback_deflate_frame(
        struct lws_tokens *eff_buf = (struct lws_tokens *)in;
        size_t current_payload, remaining_payload, total_payload;
        int n;
+       size_t len_so_far;
 
        switch (reason) {
 
@@ -149,23 +153,54 @@ bail:
 
                conn->zs_in.avail_in = total_payload + 4;
 
-               conn->zs_in.next_out = conn->buf_in + LWS_SEND_BUFFER_PRE_PADDING;
-               conn->zs_in.avail_out = conn->buf_in_length;
-
-               n = inflate(&conn->zs_in, Z_SYNC_FLUSH);
-               switch (n) {
-               case Z_NEED_DICT:
-               case Z_STREAM_ERROR:
-               case Z_DATA_ERROR:
-               case Z_MEM_ERROR:
-                       /*
-                        * screwed.. close the connection... we will get a
-                        * destroy callback to take care of closing nicely
-                        */
-                       lwsl_ext("zlib error inflate %d: %s",
-                                                  n, conn->zs_in.msg);
-                       return -1;
-               }
+               conn->zs_in.next_out = conn->buf_in + LWS_SEND_BUFFER_PRE_PADDING;
+               conn->zs_in.avail_out = conn->buf_in_length;
+
+               while (1) {
+                       n = inflate(&conn->zs_in, Z_SYNC_FLUSH);
+                       switch (n) {
+                       case Z_NEED_DICT:
+                       case Z_STREAM_ERROR:
+                       case Z_DATA_ERROR:
+                       case Z_MEM_ERROR:
+                               /*
+                                * screwed.. close the connection... we will get a
+                                * destroy callback to take care of closing nicely
+                                */
+                               fprintf(stderr, "zlib error inflate %d: %s",
+                                                          n, conn->zs_in.msg);
+                               return -1;
+                       }
+
+                       if (conn->zs_in.avail_out)
+                               break;
+
+                       len_so_far = conn->zs_in.next_out -
+                               (conn->buf_in + LWS_SEND_BUFFER_PRE_PADDING);
+
+                       conn->buf_in_length *= 2;
+                       if (conn->buf_in_length > LWS_MAX_ZLIB_CONN_BUFFER) {
+                               lwsl_ext("zlib in buffer tried to exceed "
+                                       "max allowed of %u\n",
+                                               LWS_MAX_ZLIB_CONN_BUFFER);
+                               return -1;
+                       }
+                       conn->buf_in = (unsigned char *)realloc(conn->buf_in,
+                                       LWS_SEND_BUFFER_PRE_PADDING +
+                                               conn->buf_in_length +
+                                                LWS_SEND_BUFFER_POST_PADDING);
+                       if (!conn->buf_in) {
+                               lwsl_err("Out of memory\n");
+                               return -1;
+                       }
+                       lwsl_debug(
+                               "deflate-frame ext RX did realloc to %ld\n",
+                                       conn->buf_in_length);
+                       conn->zs_in.next_out = conn->buf_in +
+                                    LWS_SEND_BUFFER_PRE_PADDING + len_so_far;
+                       conn->zs_in.avail_out =
+                                             conn->buf_in_length - len_so_far;
+               }
 
                /* rewrite the buffer pointers and length */
                eff_buf->token = (char *)(conn->buf_in + LWS_SEND_BUFFER_PRE_PADDING);
@@ -201,35 +236,36 @@ bail:
                                return -1;
                        }
 
-                       /*
-                        * AG: uncertain about this log buffer expansion approach...
-                        * same approach in Rx led to memory runaway OOM
-                        */
-                       if (!conn->zs_out.avail_out) {
-                               size_t len_so_far = (conn->zs_out.next_out -
+                       if (conn->zs_out.avail_out)
+                               break;
+
+                       len_so_far = (conn->zs_out.next_out -
                                        (conn->buf_out +
                                                 LWS_SEND_BUFFER_PRE_PADDING));
-                               unsigned char *new_buf;
-                               conn->buf_out_length *= 2;
-                               new_buf = (unsigned char *)
-                                       malloc(LWS_SEND_BUFFER_PRE_PADDING +
+                       conn->buf_out_length *= 2;
+                       if (conn->buf_out_length > LWS_MAX_ZLIB_CONN_BUFFER) {
+                               lwsl_ext("zlib out buffer tried to exceed "
+                                 "max allowed of %u\n",
+                                       LWS_MAX_ZLIB_CONN_BUFFER);
+                               return -1;
+                       }
+                       conn->buf_out = (unsigned char *)realloc(
+                                       conn->buf_out,
+                                       LWS_SEND_BUFFER_PRE_PADDING +
                                                  conn->buf_out_length +
                                                  LWS_SEND_BUFFER_POST_PADDING);
-                               if (!new_buf) {
-                                       lwsl_err("Out of memory\n");
-                                       return -1;
-                               }
-                               memcpy(new_buf + LWS_SEND_BUFFER_PRE_PADDING,
-                                       conn->buf_out + LWS_SEND_BUFFER_PRE_PADDING,
-                                       len_so_far);
-                               free(conn->buf_out);
-                               conn->buf_out = new_buf;
-                               conn->zs_out.next_out = (new_buf +
+                       if (!conn->buf_out) {
+                               lwsl_err("Out of memory\n");
+                               return -1;
+                       }
+                       lwsl_debug(
+                               "deflate-frame ext TX did realloc to %ld\n",
+                                       conn->buf_in_length);
+
+                       conn->zs_out.next_out = (conn->buf_out +
                                     LWS_SEND_BUFFER_PRE_PADDING + len_so_far);
-                               conn->zs_out.avail_out =
+                       conn->zs_out.avail_out =
                                           (conn->buf_out_length - len_so_far);
-                       } else
-                               break;
                }
 
                conn->compressed_out = 1;