phusion / passenger

A fast and robust web server and application server for Ruby, Python and Node.js
https://www.phusionpassenger.com/
MIT License
5.01k stars 548 forks source link

Connection is not closed in response to unrecognized `Transfer-Encoding` values when `Host` header is missing #2553

Open kenballus opened 4 months ago

kenballus commented 4 months ago

The expected behavior

When an unrecognized Transfer-Encoding value is received, it is likely because of an attempted request smuggling attack. Thus, the safest action to take whenever such a header is received is to close the connection (and probably also respond with an error status). This is what most HTTP implementations do, including AIOHTTP, Apache httpd, Cheroot, Deno, FastHTTP, Go net/http, Gunicorn, H2O, HAProxy, Hyper, Hypercorn, Jetty, Libsoup, Lighttpd, Mongoose, Nginx, Node.js, Puma, Apache Tomcat, Twisted, Uvicorn, Waitress, WEBrick, and OpenBSD httpd.

The actual behavior

Typically, when Passenger receives a request with an unrecognized Transfer-Encoding header value, it responds 400 and closes the connection, as expected.

However, if that request also has no Host header, then Passenger still sends a 400 response, but doesn't close the connection, and continues parsing requests from after the end of the invalid request's headers.

To reproduce

  1. Install Passenger and Node.
  2. Copy the following script into your filesystem as app.js:
    require('http').createServer((request, response) => {
    let body = [];
    request.on('data', (chunk) => {
        body.push(chunk);
    }).on('end', () => {
        response.end(JSON.stringify({
            "headers": Object.keys(request.headers).map((key) => [btoa(key), btoa(request.headers[key])]),
            "body": Buffer.concat(body).toString('base64'),
            "uri": btoa(request.url),
            "method": btoa(request.method),
            "version": btoa(request.httpVersion),
        }));
    });
    }).listen(80);
  3. Start Passenger:
    passenger start --app-type node --startup-file app.js --engine builtin --port 8000
  4. Send a request with an invalid Transfer-Encoding value and a valid Host header, followed by a valid pipelined request:
    printf 'POST / HTTP/1.1\r\nHost: a\r\nTransfer-Encoding: a\r\n\r\nGET / HTTP/1.1\r\nHost: a\r\n\r\n' \
    | nc localhost 8000
  5. Observe that Passenger responds with a 400, then closes the connection, as expected:
    
    HTTP/1.1 400 Bad Request
    Status: 400 Bad Request
    Date: Sun, 07 Jul 2024 20:59:14 GMT
    Connection: close
    X-Powered-By: Phusion Passenger(R) 6.0.23
5. Send a request with an invalid `Transfer-Encoding` value and **no** `Host` header, followed by a valid pipelined request:
```sh
printf 'POST / HTTP/1.1\r\nTransfer-Encoding: a\r\n\r\nGET / HTTP/1.1\r\nHost: a\r\n\r\n' \
    | nc localhost 8000
  1. Observe that Passenger responds with a 400, doesn't close the connection, and then responds 200:
    
    HTTP/1.1 400 Bad Request
    Status: 400 Bad Request
    Date: Sun, 07 Jul 2024 21:06:07 GMT
    Transfer-Encoding: chunked
    X-Powered-By: Phusion Passenger(R) 6.0.23

0

HTTP/1.1 200 OK Status: 200 OK Date: Sun, 07 Jul 2024 21:06:07 GMT Content-Length: 121 X-Powered-By: Phusion Passenger(R) 6.0.23

{"headers":[["Y29ubmVjdGlvbg==","Y2xvc2U="],["aG9zdA==","YQ=="]],"body":"","uri":"Lw==","method":"R0VU","version":"MS4x"}



# Engine
This has been tested only on the builtin engine, and very likely does not work when using the Nginx engine.

# Versions
- Passenger version: `Phusion Passenger(R) 6.0.23`
  - Built from source from the `stable-6.0` branch, commit 1ba2f1be504425d3470835e52167ee3180c270fd, which is latest at time of writing.

- OS version: Linux 6.9.7, Debian 13.

# Installation method

I have verified that this behavior exists both when using the Debian 12 passenger package, and when building Passenger from source.

# Programming language
- Node.js v20.14.0

# Containerization
- Docker version 27.0.3, build 7d4bcd863a
- [Dockerfile](https://raw.githubusercontent.com/narfindustries/http-garden/main/images/passenger/Dockerfile), [base image Dockerfile](https://raw.githubusercontent.com/narfindustries/http-garden/main/images/http-garden-soil/Dockerfile).