[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