[nginx] Cache: check the whole cache key in addition to hashes.

[Maxim Dounin]

details:   http://hg.nginx.org/nginx/rev/4821fc788c12
branches:  
changeset: 6243:4821fc788c12
user:      Maxim Dounin <mdounin at mdounin.ru>
date:      Fri Sep 11 17:03:56 2015 +0300
description:
Cache: check the whole cache key in addition to hashes.

This prevents a potential attack that discloses cached data if an attacker
will be able to craft a hash collision between some cache key the attacker
is allowed to access and another cache key with protected data.

See http://mailman.nginx.org/pipermail/nginx-devel/2015-September/007288.html.

Thanks to Gena Makhomed and Sergey Brester.

diffstat:

 src/http/ngx_http_file_cache.c |  21 +++++++++++++++++++--
 1 files changed, 19 insertions(+), 2 deletions(-)

diffs (53 lines):

diff --git a/src/http/ngx_http_file_cache.c b/src/http/ngx_http_file_cache.c
--- a/src/http/ngx_http_file_cache.c
+++ b/src/http/ngx_http_file_cache.c
@@ -521,9 +521,12 @@ wakeup:
 static ngx_int_t
 ngx_http_file_cache_read(ngx_http_request_t *r, ngx_http_cache_t *c)
 {
+    u_char                        *p;
     time_t                         now;
     ssize_t                        n;
+    ngx_str_t                     *key;
     ngx_int_t                      rc;
+    ngx_uint_t                     i;
     ngx_http_file_cache_t         *cache;
     ngx_http_file_cache_header_t  *h;
 
@@ -547,12 +550,27 @@ ngx_http_file_cache_read(ngx_http_reques
         return NGX_DECLINED;
     }
 
-    if (h->crc32 != c->crc32) {
+    if (h->crc32 != c->crc32 || h->header_start != c->header_start) {
         ngx_log_error(NGX_LOG_CRIT, r->connection->log, 0,
                       "cache file \"%s\" has md5 collision", c->file.name.data);
         return NGX_DECLINED;
     }
 
+    p = c->buf->pos + sizeof(ngx_http_file_cache_header_t)
+        + sizeof(ngx_http_file_cache_key);
+
+    key = c->keys.elts;
+    for (i = 0; i < c->keys.nelts; i++) {
+        if (ngx_memcmp(p, key[i].data, key[i].len) != 0) {
+            ngx_log_error(NGX_LOG_CRIT, r->connection->log, 0,
+                          "cache file \"%s\" has md5 collision",
+                          c->file.name.data);
+            return NGX_DECLINED;
+        }
+
+        p += key[i].len;
+    }
+
     if ((size_t) h->body_start > c->body_start) {
         ngx_log_error(NGX_LOG_CRIT, r->connection->log, 0,
                       "cache file \"%s\" has too long header",
@@ -583,7 +601,6 @@ ngx_http_file_cache_read(ngx_http_reques
     c->last_modified = h->last_modified;
     c->date = h->date;
     c->valid_msec = h->valid_msec;
-    c->header_start = h->header_start;
     c->body_start = h->body_start;
     c->etag.len = h->etag_len;
     c->etag.data = h->etag;