Welcome! Log In Create A New Profile

Advanced

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

Maxim Dounin
September 11, 2015 10:14AM
details: http://hg.nginx.org/nginx/rev/4821fc788c12
branches:
changeset: 6243:4821fc788c12
user: Maxim Dounin <mdounin@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;

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

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

Maxim Dounin 894 September 11, 2015 10:14AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 218
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready