[PATCH] HTTP/3: fixed handling of malformed request body length

[Roman Arutyunyan]

Hi,

On Tue, Apr 23, 2024 at 05:14:54PM +0400, Sergey Kandaurov wrote:
> 
> > On 22 Apr 2024, at 19:00, Sergey Kandaurov <pluknet at nginx.com> wrote:
> > 
> > # HG changeset patch
> > # User Sergey Kandaurov <pluknet at nginx.com>
> > # Date 1713798017 -14400
> > #      Mon Apr 22 19:00:17 2024 +0400
> > # Node ID 754e32eaca24ad751e2a94790e1afd55202c1aba
> > # Parent  9f84f2e49c624e82e054a2dcd48723119c44029c
> > HTTP/3: fixed handling of malformed request body length.
> > 
> > Previously, a request body larger than declared in Content-Length resulted in
> > a 413 status code, because Content-Length was mistakenly used as the maximum
> > allowed request body, similar to client_max_body_size.  Following the HTTP/3
> > specification, such requests are now rejected with the 400 error as malformed.
> > 
> > diff --git a/src/http/v3/ngx_http_v3_request.c b/src/http/v3/ngx_http_v3_request.c
> > --- a/src/http/v3/ngx_http_v3_request.c
> > +++ b/src/http/v3/ngx_http_v3_request.c
> > @@ -1575,12 +1575,21 @@ ngx_http_v3_request_body_filter(ngx_http
> >                 /* rc == NGX_OK */
> > 
> >                 if (max != -1 && (uint64_t) (max - rb->received) < st->length) {
> > -                    ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
> > -                                  "client intended to send too large "
> > -                                  "body: %O+%ui bytes",
> > -                                  rb->received, st->length);
> > +
> > +                    if (max == r->headers_in.content_length_n) {
> > +                        ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
> > +                                      "client intended to send body data "
> > +                                      "larger than declared");
> > +
> > +                        return NGX_HTTP_BAD_REQUEST;
> > 
> > -                    return NGX_HTTP_REQUEST_ENTITY_TOO_LARGE;
> > +                    } else {
> > +                        ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
> > +                                      "client intended to send too large body: "
> > +                                      "%O+%ui bytes", rb->received, st->length);
> > +
> > +                        return NGX_HTTP_REQUEST_ENTITY_TOO_LARGE;
> > +                    }
> >                 }
> > 
> >                 continue;
> 
> After more thoughts I replaced "max == r->headers_in.content_length_n"
> with a more clear check "r->headers_in.content_length_n != -1"
> (also to make it more similar to the HTTP/2 request body filter).
> The resulting diff is minimized since both conditions result in return.
> 
> # HG changeset patch
> # User Sergey Kandaurov <pluknet at nginx.com>
> # Date 1713876685 -14400
> #      Tue Apr 23 16:51:25 2024 +0400
> # Node ID c935bfd88ebf05aed181fc03231df8bd95475334
> # Parent  9f84f2e49c624e82e054a2dcd48723119c44029c
> HTTP/3: fixed handling of malformed request body length.
> 
> Previously, a request body larger than declared in Content-Length resulted in
> a 413 status code, because Content-Length was mistakenly used as the maximum
> allowed request body, similar to client_max_body_size.  Following the HTTP/3
> specification, such requests are now rejected with the 400 error as malformed.
> 
> diff --git a/src/http/v3/ngx_http_v3_request.c b/src/http/v3/ngx_http_v3_request.c
> --- a/src/http/v3/ngx_http_v3_request.c
> +++ b/src/http/v3/ngx_http_v3_request.c
> @@ -1575,6 +1575,15 @@ ngx_http_v3_request_body_filter(ngx_http
>                  /* rc == NGX_OK */
>                    if (max != -1 && (uint64_t) (max - rb->received) < st->length) {
> +
> +                    if (r->headers_in.content_length_n != -1) {
> +                        ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
> +                                      "client intended to send body data "
> +                                      "larger than declared");
> +
> +                        return NGX_HTTP_BAD_REQUEST;
> +                    }
> +
>                      ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
>                                    "client intended to send too large "
>                                    "body: %O+%ui bytes",
> 
> 
> -- 
> Sergey Kandaurov
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel

Looks ok