[patch] reject http header without colon (:) in the header name

Maksim Yevmenkin maksim.yevmenkin at gmail.com
Wed May 8 17:59:30 UTC 2024


> > it appears that nginx would happily accept http header without colon
> > (:) in the header name. the patch below tries to address this.
>
> Yes.  I believe it was allowed as a part of the following change
> in nginx 0.1.29 (509:9b8c906f6e63,
> https://freenginx.org/hg/nginx/rev/9b8c906f6e63#l85.68):
>
>     *) Change: nginx now passes the invalid lines in a client request
>        headers or a backend response header.
>
> With introduction of "ignore_invalid_headers" in nginx 0.1.30
> (511:c12967aadd87) this wasn't changed though, which is probably
> an oversight.
>
> Such headers are essentially recognized as headers with an
> empty value, and I don't think that this can be an issue.  Still,
> I don't object hardening the parser and rejecting such headers.
>
> I don't think this change will affect any real clients, though
> potentially might affect some misbehaving backend code, such as
> loosely written fastcgi scripts.  If its the case, such code needs
> to be fixed.
>
> Alternatively, we can consider marking such headers as invalid, so
> they will be ignored from clients (unless "ignore_invalid_headers off;"
> is specified) and passed from backend servers to clients.  Not
> sure it worth the effort though.

[...]

> Removing relevant if blocks should be enough, these will be
> handled by the following "if (ch <= 0x20...)" block.  Please take
> a look at the following patch:
>
> # HG changeset patch
> # User Maxim Dounin <mdounin at mdounin.ru>
> # Date 1715122480 -10800
> #      Wed May 08 01:54:40 2024 +0300
> # Node ID e9aa91a1861dcabfd1e9ebb7d5d96cdb42c8dcd4
> # Parent  93bbb9fbf30dd82709551610f05e22eac17717d4
> Disabled handling of headers without a colon.
>
> Starting with nginx 0.1.29 (509:9b8c906f6e63), header names not followed
> by a colon and a value were allowed.  Such headers were interpreted as
> headers with an empty value.  With this change, such headers are
> unconditionally rejected.
>
> Requested by Maksim Yevmenkin.
>
> diff --git a/src/http/ngx_http_parse.c b/src/http/ngx_http_parse.c
> --- a/src/http/ngx_http_parse.c
> +++ b/src/http/ngx_http_parse.c
> @@ -961,21 +961,6 @@ ngx_http_parse_header_line(ngx_http_requ
>                  break;
>              }
>
> -            if (ch == CR) {
> -                r->header_name_end = p;
> -                r->header_start = p;
> -                r->header_end = p;
> -                state = sw_almost_done;
> -                break;
> -            }
> -
> -            if (ch == LF) {
> -                r->header_name_end = p;
> -                r->header_start = p;
> -                r->header_end = p;
> -                goto done;
> -            }
> -
>              /* IIS may send the duplicate "HTTP/1.1 ..." lines */
>              if (ch == '/'
>                  && r->upstream

this patch works.

!thanks
max



More information about the nginx-devel mailing list