Hello!
On Thu, Apr 27, 2023 at 09:53:30PM +0400, Sergey Kandaurov wrote:
> # HG changeset patch
> # User Sergey Kandaurov <pluk...@nginx.com>
> # Date 1682617947 -14400
> # Thu Apr 27 21:52:27 2023 +0400
> # Node ID c1ec385d885fba38f15d54263944eed2c74b5733
> # Parent 77d5c662f3d9d9b90425128109d3369c30ef5f07
> Variables: avoid possible buffer overrun with some "$sent_http_*".
>
> The existing logic to evaluate multi header "$sent_http_*" variables,
> such as $sent_http_cache_control, as previously introduced in 1.23.0,
> doesn't take into account that one or more elements can be cleared,
> yet still present in a linked list, pointed to by the next field.
> Such elements don't contribute to the resulting variable length, an
> attempt to append a separator for them ends up in out of bounds write.
>
> This is not possible with standard modules, though at least one third
> party module is known to override multi header values this way, so it
> makes sense to harden the logic.
>
> The fix restores a generic boundary check.
>
> diff --git a/src/http/ngx_http_variables.c b/src/http/ngx_http_variables.c
> --- a/src/http/ngx_http_variables.c
> +++ b/src/http/ngx_http_variables.c
> @@ -828,7 +828,7 @@ ngx_http_variable_headers_internal(ngx_h
> ngx_http_variable_value_t *v, uintptr_t data, u_char sep)
> {
> size_t len;
> - u_char *p;
> + u_char *p, *end;
> ngx_table_elt_t *h, *th;
>
> h = *(ngx_table_elt_t **) ((char *) r + data);
> @@ -870,6 +870,8 @@ ngx_http_variable_headers_internal(ngx_h
> v->len = len;
> v->data = p;
>
> + end = p + len;
> +
> for (th = h; th; th = th->next) {
>
> if (th->hash == 0) {
> @@ -878,7 +880,7 @@ ngx_http_variable_headers_internal(ngx_h
>
> p = ngx_copy(p, th->value.data, th->value.len);
>
> - if (th->next == NULL) {
> + if (p == end) {
> break;
> }
>
Looks good.
--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
