Disabled bare LF in chunked transfer encoding.

Chunked transfer encoding, since originally introduced in HTTP/1.1
in RFC 2068, is specified to use CRLF as the only line terminator.

Although tolerant applications may recognize a single LF, formally
this covers the start line and fields, and doesn't apply to chunks.
Strict chunked parsing is reaffirmed as intentional in RFC errata
ID 7633, notably "because it does not have to retain backwards
compatibility with 1.0 parsers".

A general RFC 2616 recommendation to tolerate deviations whenever
interpreted unambiguously doesn't apply here, because chunked body
is used to determine HTTP message framing; a relaxed parsing may
cause various security problems due to a broken delimitation.
For instance, this is possible when receiving chunked body from
intermediates that blindly parse chunk-ext or a trailer section
until CRLF, and pass it further without re-coding.

diff --git a/src/http/modules/ngx_http_proxy_module.c b/src/http/modules/ngx_http_proxy_module.c
index d3836602e14e04f05a8c98fd5b6f5ed635dcc478..5e6b0c434c08099ac504f48311da8f5cc6bee8e5 100644 (file)
--- a/src/http/modules/ngx_http_proxy_module.c
+++ b/src/http/modules/ngx_http_proxy_module.c
@@ -2119,7 +2119,7 @@ ngx_http_proxy_input_filter_init(void *data)
         /* chunked */
 
         u->pipe->input_filter = ngx_http_proxy_chunked_filter;
-        u->pipe->length = 3; /* "0" LF LF */
+        u->pipe->length = 5; /* "0" CRLF CRLF */
 
         u->input_filter = ngx_http_proxy_non_buffered_chunked_filter;
         u->length = 1;

diff --git a/src/http/ngx_http_parse.c b/src/http/ngx_http_parse.c
index 059edae44a8ea9a348c6d2d5431eed693b3b1c38..e60dc425e0918c50effb3e301d29072ab1a5779d 100644 (file)
--- a/src/http/ngx_http_parse.c
+++ b/src/http/ngx_http_parse.c
@@ -2254,12 +2254,6 @@ ngx_http_parse_chunked(ngx_http_request_t *r, ngx_buf_t *b,
                 case CR:
                     state = sw_last_chunk_extension_almost_done;
                     break;
-                case LF:
-                    if (keep_trailers) {
-                        goto done;
-                    }
-                    state = sw_trailer;
-                    break;
                 case ';':
                 case ' ':
                 case '\t':
@@ -2276,9 +2270,6 @@ ngx_http_parse_chunked(ngx_http_request_t *r, ngx_buf_t *b,
             case CR:
                 state = sw_chunk_extension_almost_done;
                 break;
-            case LF:
-                state = sw_chunk_data;
-                break;
             case ';':
             case ' ':
             case '\t':
@@ -2296,7 +2287,7 @@ ngx_http_parse_chunked(ngx_http_request_t *r, ngx_buf_t *b,
                 state = sw_chunk_extension_almost_done;
                 break;
             case LF:
-                state = sw_chunk_data;
+                goto invalid;
             }
             break;
 
@@ -2316,9 +2307,6 @@ ngx_http_parse_chunked(ngx_http_request_t *r, ngx_buf_t *b,
             case CR:
                 state = sw_after_data_almost_done;
                 break;
-            case LF:
-                state = sw_chunk_start;
-                break;
             default:
                 goto invalid;
             }
@@ -2337,10 +2325,7 @@ ngx_http_parse_chunked(ngx_http_request_t *r, ngx_buf_t *b,
                 state = sw_last_chunk_extension_almost_done;
                 break;
             case LF:
-                if (keep_trailers) {
-                    goto done;
-                }
-                state = sw_trailer;
+                goto invalid;
             }
             break;
 
@@ -2360,7 +2345,7 @@ ngx_http_parse_chunked(ngx_http_request_t *r, ngx_buf_t *b,
                 state = sw_trailer_almost_done;
                 break;
             case LF:
-                goto done;
+                goto invalid;
             default:
                 state = sw_trailer_header;
             }
@@ -2378,7 +2363,7 @@ ngx_http_parse_chunked(ngx_http_request_t *r, ngx_buf_t *b,
                 state = sw_trailer_header_almost_done;
                 break;
             case LF:
-                state = sw_trailer;
+                goto invalid;
             }
             break;
 
@@ -2404,35 +2389,45 @@ data:
     switch (state) {
 
     case sw_chunk_start:
-        ctx->length = 3 /* "0" LF LF */;
+        ctx->length = 5 /* "0" CRLF CRLF */;
         break;
     case sw_chunk_size:
-        ctx->length = 1 /* LF */
-                      + (ctx->size ? ctx->size + 4 /* LF "0" LF LF */
-                                   : 1 /* LF */);
+        ctx->length = 2 /* CRLF */
+                      + (ctx->size ? ctx->size + 7 /* CRLF "0" CRLF CRLF */
+                                   : 2 /* CRLF */);
         break;
     case sw_chunk_extension:
+        ctx->length = 2 /* CRLF */ + ctx->size + 7 /* CRLF "0" CRLF CRLF */;
+        break;
     case sw_chunk_extension_almost_done:
-        ctx->length = 1 /* LF */ + ctx->size + 4 /* LF "0" LF LF */;
+        ctx->length = 1 /* LF */ + ctx->size + 7 /* CRLF "0" CRLF CRLF */;
         break;
     case sw_chunk_data:
-        ctx->length = ctx->size + 4 /* LF "0" LF LF */;
+        ctx->length = ctx->size + 7 /* CRLF "0" CRLF CRLF */;
         break;
     case sw_after_data:
+        ctx->length = 7 /* CRLF "0" CRLF CRLF */;
+        break;
     case sw_after_data_almost_done:
-        ctx->length = 4 /* LF "0" LF LF */;
+        ctx->length = 6 /* LF "0" CRLF CRLF */;
         break;
     case sw_last_chunk_extension:
+        ctx->length = 4 /* CRLF CRLF */;
+        break;
     case sw_last_chunk_extension_almost_done:
-        ctx->length = 2 /* LF LF */;
+        ctx->length = 3 /* LF CRLF */;
         break;
     case sw_trailer:
+        ctx->length = 2 /* CRLF */;
+        break;
     case sw_trailer_almost_done:
         ctx->length = 1 /* LF */;
         break;
     case sw_trailer_header:
+        ctx->length = 4 /* CRLF CRLF */;
+        break;
     case sw_trailer_header_almost_done:
-        ctx->length = 2 /* LF LF */;
+        ctx->length = 3 /* LF CRLF */;
         break;
 
     }