- Add better protection against malicious gzip headers.
authorFabian Keil <fk@fabiankeil.de>
Wed, 21 Mar 2007 12:23:53 +0000 (12:23 +0000)
committerFabian Keil <fk@fabiankeil.de>
Wed, 21 Mar 2007 12:23:53 +0000 (12:23 +0000)
- Stop logging the first hundred bytes of decompressed content.
  It looks like it's working and there is always debug 16.
- Log the content size after decompression in decompress_iob()
  instead of pcrs_filter_response().

parsers.c

index 4aa786e..2d6087d 100644 (file)
--- a/parsers.c
+++ b/parsers.c
@@ -1,4 +1,4 @@
-const char parsers_rcs[] = "$Id: parsers.c,v 1.92 2007/03/05 13:25:32 fabiankeil Exp $";
+const char parsers_rcs[] = "$Id: parsers.c,v 1.93 2007/03/20 15:21:44 fabiankeil Exp $";
 /*********************************************************************
  *
  * File        :  $Source: /cvsroot/ijbswa/current/parsers.c,v $
@@ -44,6 +44,14 @@ const char parsers_rcs[] = "$Id: parsers.c,v 1.92 2007/03/05 13:25:32 fabiankeil
  *
  * Revisions   :
  *    $Log: parsers.c,v $
+ *    Revision 1.93  2007/03/20 15:21:44  fabiankeil
+ *    - Use dedicated header filter actions instead of abusing "filter".
+ *      Replace "filter-client-headers" and "filter-client-headers"
+ *      with "server-header-filter" and "client-header-filter".
+ *    - Remove filter_client_header() and filter_client_header(),
+ *      filter_header() now checks the shiny new
+ *      CSP_FLAG_CLIENT_HEADER_PARSING_DONE flag instead.
+ *
  *    Revision 1.92  2007/03/05 13:25:32  fabiankeil
  *    - Cosmetical changes for LOG_LEVEL_RE_FILTER messages.
  *    - Handle "Cookie:" and "Connection:" headers a bit smarter
@@ -881,6 +889,7 @@ jb_err decompress_iob(struct client_state *csp)
    char  *cur;       /* Current iob position (to keep the original 
                       * iob->cur unmodified if we return early) */
    size_t bufsize;   /* allocated size of the new buffer */
+   size_t old_size;  /* Content size before decompression */
    size_t skip_size; /* Number of bytes at the beginning of the iob
                         that we should NOT decompress. */
    int status;       /* return status of the inflate() call */
@@ -891,6 +900,7 @@ jb_err decompress_iob(struct client_state *csp)
 
    bufsize = csp->iob->size;
    skip_size = (size_t)(csp->iob->cur - csp->iob->buf);
+   old_size = (size_t)(csp->iob->eod - csp->iob->cur);
 
    cur = csp->iob->cur;
 
@@ -968,7 +978,7 @@ jb_err decompress_iob(struct client_state *csp)
             skip_bytes = *cur++;
             skip_bytes = *cur++ << 8;
 
-            assert(skip_bytes == *csp->iob->cur-2 + ((*csp->iob->cur-1) << 8));
+            assert(skip_bytes == *csp->iob->cur - 2 + ((*csp->iob->cur - 1) << 8));
 
             /*
              * The number of bytes to skip should be positive
@@ -990,14 +1000,16 @@ jb_err decompress_iob(struct client_state *csp)
          /* Skip the filename if necessary. */
          if (flags & 0x08)
          {
-            /* A null-terminated string follows. */
-            while (*cur++);
+            /* A null-terminated string is supposed to follow. */
+            while (*cur++ && (cur < csp->iob->eod));
+
          }
 
          /* Skip the comment if necessary. */
          if (flags & 0x10)
          {
-            while (*cur++);
+            /* A null-terminated string is supposed to follow. */
+            while (*cur++ && (cur < csp->iob->eod));
          }
 
          /* Skip the CRC if necessary. */
@@ -1005,6 +1017,18 @@ jb_err decompress_iob(struct client_state *csp)
          {
             cur += 2;
          }
+
+         if (cur >= csp->iob->eod)
+         {
+            /*
+             * If the current position pointer reached or passed
+             * the buffer end, we were obviously tricked to skip
+             * too much.
+             */
+            log_error (LOG_LEVEL_ERROR,
+               "Malformed gzip header detected. Aborting decompression.");
+            return JB_ERR_COMPRESS;
+         }
       }
    }
    else if (csp->content_type & CT_DEFLATE)
@@ -1186,15 +1210,18 @@ jb_err decompress_iob(struct client_state *csp)
     && (csp->iob->cur <= csp->iob->eod)
     && (csp->iob->eod <= csp->iob->buf + csp->iob->size))
    {
-      char t = csp->iob->cur[100];
-      csp->iob->cur[100] = '\0';
-      /*
-       * XXX: The debug level should be lowered
-       * before the next stable release.
-       */
-      log_error(LOG_LEVEL_INFO, "Sucessfully decompressed: %s", csp->iob->cur);
-      csp->iob->cur[100] = t;
-      return JB_ERR_OK;
+      const size_t new_size = (size_t)(csp->iob->eod - csp->iob->cur);
+      if (new_size > 0)
+      {
+         log_error(LOG_LEVEL_RE_FILTER,
+            "Decompression successful. Old size: %d, new size: %d.",
+            old_size, new_size);
+      }
+      else
+      {
+         /* zlib thinks this is OK, so lets do the same. */
+         log_error(LOG_LEVEL_INFO, "Decompression didn't result in any content.");
+      }
    }
    else
    {
@@ -1206,6 +1233,8 @@ jb_err decompress_iob(struct client_state *csp)
       return JB_ERR_COMPRESS;
    }
 
+   return JB_ERR_OK;
+
 }
 #endif /* defined(FEATURE_ZLIB) */