diff --git a/lib/Plack/Handler/Gazelle.pm b/lib/Plack/Handler/Gazelle.pm index 98345c0..db36e31 100644 --- a/lib/Plack/Handler/Gazelle.pm +++ b/lib/Plack/Handler/Gazelle.pm @@ -161,24 +161,11 @@ sub run { my $res = $bad_response; my $chunked = do { no warnings; lc delete $env->{HTTP_TRANSFER_ENCODING} eq 'chunked' }; - if (my $cl = $env->{CONTENT_LENGTH}) { - my $buffer = Stream::Buffered->new($cl); - while ($cl > 0) { - my $chunk = ""; - if (length $buf) { - $chunk = $buf; - $buf = ''; - } else { - read_timeout( - $conn, \$chunk, $cl, 0, $self->{timeout}) - or next PROC_LOOP; - } - $buffer->print($chunk); - $cl -= length $chunk; - } - $env->{'psgi.input'} = $buffer->rewind; - } elsif ( $chunked ) { - my $buffer = Stream::Buffered->new($cl); + # RFC 7230 §3.3.3: Transfer-Encoding takes precedence over + # Content-Length. Requests carrying both are rejected upstream + # in the XS layer; this ordering is defence-in-depth. + if ( $chunked ) { + my $buffer = Stream::Buffered->new; my $chunk_buffer = ''; my $length; DECHUNK: while(1) { @@ -210,6 +197,22 @@ sub run { } $env->{CONTENT_LENGTH} = $length; $env->{'psgi.input'} = $buffer->rewind; + } elsif (my $cl = $env->{CONTENT_LENGTH}) { + my $buffer = Stream::Buffered->new($cl); + while ($cl > 0) { + my $chunk = ""; + if (length $buf) { + $chunk = $buf; + $buf = ''; + } else { + read_timeout( + $conn, \$chunk, $cl, 0, $self->{timeout}) + or next PROC_LOOP; + } + $buffer->print($chunk); + $cl -= length $chunk; + } + $env->{'psgi.input'} = $buffer->rewind; } else { $env->{'psgi.input'} = $null_io; } diff --git a/lib/Plack/Handler/Gazelle.xs b/lib/Plack/Handler/Gazelle.xs index be92a40..d5594f4 100644 --- a/lib/Plack/Handler/Gazelle.xs +++ b/lib/Plack/Handler/Gazelle.xs @@ -271,6 +271,8 @@ _parse_http_request(pTHX_ char *buf, ssize_t buf_len, HV *env) { (void)hv_stores(env, "QUERY_STRING", newSVpvn(path + question_at, path_len - question_at)); last_value = NULL; + int seen_content_length = 0; + int seen_transfer_encoding = 0; for (i = 0; i < num_headers; ++i) { if (headers[i].name != NULL) { const char* name; @@ -280,8 +282,28 @@ _parse_http_request(pTHX_ char *buf, ssize_t buf_len, HV *env) { name = "CONTENT_TYPE"; name_len = sizeof("CONTENT_TYPE") - 1; } else if (header_is(headers + i, "CONTENT-LENGTH", sizeof("CONTENT-LENGTH") - 1)) { + seen_content_length = 1; + if (seen_transfer_encoding) { + /* RFC 7230 §3.3.3: Transfer-Encoding overrides Content-Length; + * receiving both is a potential request-smuggling attack. Reject. + * env will be freed via sv_2mortal in the badexit_clear path. */ + ret = -1; + goto done; + } name = "CONTENT_LENGTH"; name_len = sizeof("CONTENT_LENGTH") - 1; + } else if (header_is(headers + i, "TRANSFER-ENCODING", sizeof("TRANSFER-ENCODING") - 1)) { + seen_transfer_encoding = 1; + if (seen_content_length) { + /* RFC 7230 §3.3.3: Transfer-Encoding overrides Content-Length; + * receiving both is a potential request-smuggling attack. Reject. + * env will be freed via sv_2mortal in the badexit_clear path. */ + ret = -1; + goto done; + } + name = tmp; + strcpy(tmp, "HTTP_TRANSFER_ENCODING"); + name_len = sizeof("HTTP_TRANSFER_ENCODING") - 1; } else { const char* s; char* d;