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.

2 files changed, 24 insertions, 29 deletions

diff --git a/src/http/modules/ngx_http_proxy_module.c b/src/http/modules/ngx_http_proxy_module.c
index d3836602e..5e6b0c434 100644
--- 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 059edae44..e60dc425e 100644
--- 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;
 
     }