aports/main/nginx/nginx_cookie_flag_module~fix-mem-allocations.patch
2021-01-07 17:12:44 +01:00

182 lines
7.7 KiB
Diff

From fc68e3670624660b7fdf89bc1b7316f5a267d7e0 Mon Sep 17 00:00:00 2001
From: "Oleg A. Mamontov" <oleg@mamontov.net>
Date: Wed, 8 Jul 2020 19:44:02 +0300
Subject: [PATCH] ngx_http_cookie_flag_filter_handler() allocated not enough
memory for "cookie_name". The strcat() call would write '\0' outside the
allocated buffer. The current code also incorrectly matches any cookie whose
name ends in "foo" if "set_cookie_flag foo ..." is specified. Both bugs
fixed by rewriting the code that matches cookies by name.
ngx_http_cookie_flag_filter_append() allocated not enough memory
when editing cookie values. Generally, strings in nginx are not
NUL-terminated, but there are some exceptions, including the
values of request/response headers. While that assumption allows
searching for substrings with ngx_strcasestrn(), the edited values
were not NUL-terminated. This is fixed by allocating enough memory
to have NUL-terminated strings.
---
ngx_http_cookie_flag_filter_module.c | 117 ++++++++++++++++-----------
1 file changed, 71 insertions(+), 46 deletions(-)
Patch-Source: https://github.com/AirisX/nginx_cookie_flag_module/pull/17
diff --git a/ngx_http_cookie_flag_filter_module.c b/ngx_http_cookie_flag_filter_module.c
index b0316aa..63b8269 100644
--- a/ngx_http_cookie_flag_filter_module.c
+++ b/ngx_http_cookie_flag_filter_module.c
@@ -235,58 +235,89 @@ ngx_http_cookie_flag_filter_init(ngx_conf_t *cf)
static ngx_int_t
ngx_http_cookie_flag_filter_append(ngx_http_request_t *r, ngx_http_cookie_t *cookie, ngx_table_elt_t *header)
{
- ngx_str_t tmp;
+ u_char *data, *p;
+ size_t len;
+ ngx_http_cookie_t c;
- if (cookie->httponly == 1 && ngx_strcasestrn(header->value.data, "; HttpOnly", 10 - 1) == NULL) {
- tmp.data = ngx_pnalloc(r->pool, header->value.len + sizeof("; HttpOnly") - 1);
- if (tmp.data == NULL) {
- return NGX_ERROR;
+ c = *cookie;
+ len = 0;
+
+ if (c.httponly) {
+ if (ngx_strcasestrn(header->value.data, "; HttpOnly", 10 - 1) == NULL) {
+ len += sizeof("; HttpOnly") - 1;
+ } else {
+ c.httponly = 0;
}
- tmp.len = ngx_sprintf(tmp.data, "%V; HttpOnly", &header->value) - tmp.data;
- header->value.data = tmp.data;
- header->value.len = tmp.len;
}
- if (cookie->secure == 1 && ngx_strcasestrn(header->value.data, "; secure", 8 - 1) == NULL) {
- tmp.data = ngx_pnalloc(r->pool, header->value.len + sizeof("; secure") - 1);
- if (tmp.data == NULL) {
- return NGX_ERROR;
+ if (c.secure) {
+ if (ngx_strcasestrn(header->value.data, "; secure", 8 - 1) == NULL) {
+ len += sizeof("; secure") - 1;
+ } else {
+ c.secure = 0;
}
- tmp.len = ngx_sprintf(tmp.data, "%V; secure", &header->value) - tmp.data;
- header->value.data = tmp.data;
- header->value.len = tmp.len;
}
- if (cookie->samesite == 1 && ngx_strcasestrn(header->value.data, "; SameSite", 10 - 1) == NULL) {
- tmp.data = ngx_pnalloc(r->pool, header->value.len + sizeof("; SameSite") - 1);
- if (tmp.data == NULL) {
- return NGX_ERROR;
+ if (c.samesite) {
+ if (ngx_strcasestrn(header->value.data, "; SameSite", 10 - 1) == NULL) {
+ len += sizeof("; SameSite") - 1;
+ } else {
+ c.samesite = 0;
}
- tmp.len = ngx_sprintf(tmp.data, "%V; SameSite", &header->value) - tmp.data;
- header->value.data = tmp.data;
- header->value.len = tmp.len;
}
- if (cookie->samesite_lax == 1 && ngx_strcasestrn(header->value.data, "; SameSite=Lax", 14 - 1) == NULL) {
- tmp.data = ngx_pnalloc(r->pool, header->value.len + sizeof("; SameSite=Lax") - 1);
- if (tmp.data == NULL) {
- return NGX_ERROR;
+ if (c.samesite_lax) {
+ if (ngx_strcasestrn(header->value.data, "; SameSite=Lax", 14 - 1) == NULL) {
+ len += sizeof("; SameSite=Lax") - 1;
+ } else {
+ c.samesite_lax = 0;
}
- tmp.len = ngx_sprintf(tmp.data, "%V; SameSite=Lax", &header->value) - tmp.data;
- header->value.data = tmp.data;
- header->value.len = tmp.len;
}
- if (cookie->samesite_strict == 1 && ngx_strcasestrn(header->value.data, "; SameSite=Strict", 17 - 1) == NULL) {
- tmp.data = ngx_pnalloc(r->pool, header->value.len + sizeof("; SameSite=Strict") - 1);
- if (tmp.data == NULL) {
- return NGX_ERROR;
+ if (c.samesite_strict) {
+ if (ngx_strcasestrn(header->value.data, "; SameSite=Strict", 17 - 1) == NULL) {
+ len += sizeof("; SameSite=Strict") - 1;
+ } else {
+ c.samesite_strict = 0;
}
- tmp.len = ngx_sprintf(tmp.data, "%V; SameSite=Strict", &header->value) - tmp.data;
- header->value.data = tmp.data;
- header->value.len = tmp.len;
}
+ if (len == 0) {
+ return NGX_OK;
+ }
+
+ data = ngx_pnalloc(r->pool, header->value.len + len + 1);
+ if (data == NULL) {
+ return NGX_ERROR;
+ }
+
+ p = ngx_sprintf(data, "%V", &header->value);
+
+ if (c.httponly) {
+ p = ngx_sprintf(p, "; HttpOnly");
+ }
+
+ if (c.secure) {
+ p = ngx_sprintf(p, "; secure");
+ }
+
+ if (c.samesite) {
+ p = ngx_sprintf(p, "; SameSite");
+ }
+
+ if (c.samesite_lax) {
+ p = ngx_sprintf(p, "; SameSite=Lax");
+ }
+
+ if (c.samesite_strict) {
+ p = ngx_sprintf(p, "; SameSite=Strict");
+ }
+
+ *p = '\0';
+
+ header->value.data = data;
+ header->value.len = p - data;
+
return NGX_OK;
}
@@ -332,17 +363,11 @@ ngx_http_cookie_flag_filter_handler(ngx_http_request_t *r)
// for each security cookie we check whether preset it within Set-Cookie value. If not then we append.
for (j = 0; j < flcf->cookies->nelts; j++) {
- if (ngx_strncasecmp(cookie[j].cookie_name.data, (u_char *) "*", 1) != 0) {
- // append "=" to the security cookie name. The result will be something like "cookie_name="
- char *cookie_name = ngx_pnalloc(r->pool, sizeof("=") - 1 + cookie[j].cookie_name.len);
- if (cookie_name == NULL) {
- return NGX_ERROR;
- }
- strcpy(cookie_name, (char *) cookie[j].cookie_name.data);
- strcat(cookie_name, "=");
+ if (cookie[j].cookie_name.len != 1 || cookie[j].cookie_name.data[0] != '*') {
+ ngx_str_t *cookie_name = &cookie[j].cookie_name;
// if Set-Cookie contains a cookie from settings
- if (ngx_strcasestrn(header[i].value.data, cookie_name, strlen(cookie_name) - 1) != NULL) {
+ if (header[i].value.len > cookie_name->len && header[i].value.data[cookie_name->len] == '=' && ngx_strncasecmp(header[i].value.data, cookie_name->data, cookie_name->len) == 0) {
ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "filter http_cookie_flag - add flags for cookie \"%V\"", &cookie[j].cookie_name);
ngx_int_t res = ngx_http_cookie_flag_filter_append(r, &cookie[j], &header[i]);
if (res != NGX_OK) {
@@ -350,7 +375,7 @@ ngx_http_cookie_flag_filter_handler(ngx_http_request_t *r)
}
break; // otherwise default value will be added
}
- } else if (ngx_strncasecmp(cookie[j].cookie_name.data, (u_char *) "*", 1) == 0) {
+ } else {
ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "filter http_cookie_flag - add default cookie flags");
ngx_int_t res = ngx_http_cookie_flag_filter_append(r, &cookie[j], &header[i]);
if (res != NGX_OK) {