Hello! (It looks like you aren't subscribed to the mailing list, hence Cc.)
On Tue, May 07, 2024 at 02:33:21PM -0700, Maksim Yevmenkin wrote: > hello, > > 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. > --- a/ports/netflix/nginx/files/nginx/src/http/ngx_http_parse.c > +++ b/ports/netflix/nginx/files/nginx/src/http/ngx_http_parse.c > @@ -941,14 +941,14 @@ ngx_http_parse_header_line(ngx_http_request_t > *r, ngx_buf_t *b, > r->header_start = p; > r->header_end = p; > state = sw_almost_done; > - break; > + return NGX_HTTP_PARSE_INVALID_HEADER; > } > > if (ch == LF) { > r->header_name_end = p; > r->header_start = p; > r->header_end = p; > - goto done; > + return NGX_HTTP_PARSE_INVALID_HEADER; > } > > /* IIS may send the duplicate "HTTP/1.1 ..." lines */ 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 <mdou...@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 -- Maxim Dounin http://mdounin.ru/ -- nginx-devel mailing list nginx-devel@freenginx.org https://freenginx.org/mailman/listinfo/nginx-devel
