Title: [security] http.client: HTTP Header Injection in the HTTP method
Type: security Stage: patch review
Components: Library (Lib) Versions: Python 3.9, Python 3.8, Python 3.7, Python 3.6, Python 3.5, Python 2.7
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Amir, kmaork, maxpl0it, orsenthil, vstinner, xtreak
Priority: normal Keywords: patch

Created on 2020-02-10 19:29 by maxpl0it, last changed 2020-02-18 23:17 by kmaork.

Pull Requests
URL Status Linked Edit
PR 18480 closed Amir, 2020-02-12 11:05
PR 18485 open Amir, 2020-02-12 13:54
Messages (7)
msg361710 - (view) Author: Max (maxpl0it) Date: 2020-02-10 19:29
I recently came across a bug during a pentest that's allowed me to perform some really interesting attacks on a target. While originally discovered in requests, I had been forwarded to one of the urllib3 developers after agreeing that fixing it at it's lowest level would be preferable. I was informed that the vulnerability is also present in http.client and that I should report it here as well.

The 'method' parameter is not filtered to prevent the injection from altering the entire request.

For example:
>>> conn = http.client.HTTPConnection("localhost", 80)
>>> conn.request(method="GET / HTTP/1.1\r\nHost: abc\r\nRemainder:", url="/index.html")

This will result in the following request being generated:
GET / HTTP/1.1
Host: abc
Remainder: /index.html HTTP/1.1
Host: localhost
Accept-Encoding: identity

This was originally found in an HTTP proxy that was utilising Requests. It allowed me to manipulate the original path to access different files from an internal server since the developers had assumed that the method would filter out non-standard HTTP methods.

The recommended solution is to only allow the standard HTTP methods of GET, HEAD, POST, PUT, DELETE, CONNECT, OPTIONS, TRACE, and PATCH.

An alternate solution that would allow programmers to use non-standard methods would be to only support characters [a-z] and stop reading at any special characters (especially newlines and spaces).
msg361808 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-02-11 12:29
> The recommended solution is to only allow the standard HTTP methods of GET, HEAD, POST, PUT, DELETE, CONNECT, OPTIONS, TRACE, and PATCH.

I don't think that we have to be so strict. We can maybe restrict the HTTP method to ASCII letters, or just reject control characters (U+0000-U+001f).

Similar issues (fixed):

msg361818 - (view) Author: Max (maxpl0it) Date: 2020-02-11 14:13
I agree that the solution is quite restrictive.
Restricting to ASCII characters alone would certainly work.
msg361828 - (view) Author: Amir Mohamadi (Amir) * Date: 2020-02-11 18:34
can I work on it?!
msg361865 - (view) Author: Amir Mohamadi (Amir) * Date: 2020-02-12 07:09
@vstinner sorry to bother you, I have a quick question.

the request(...) method is like this:

def request(self, method, url, body=None, headers={}, *,          
    """Send a complete request to the server."""                  
    self._send_request(method, url, body, headers, encode_chunked)

'request' calls '_send_request' method and '_send_request' calls 'putrequest' inside itself.

So is it good if I encode 'method' parameter to ASCII inside 'putrequest'??!
msg361896 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2020-02-12 14:35
Welcome to work on the patch, Amir.

* We shouldn't be encoding anything.
* Create reject for Unicode control characters and reject the request if the request contains any control character. Write tests for this.

It will similar to one of the examples Victor has shared.
msg362239 - (view) Author: Maor Kleinberger (kmaork) * Date: 2020-02-18 23:17
Hey, it's been a week since the last activity here...
Amir, if you are not working on it I'd be glad to work on it as well :)
Date User Action Args
2020-02-18 23:17:06kmaorksetnosy: + kmaork
messages: + msg362239
2020-02-12 14:35:43orsenthilsetmessages: + msg361896
2020-02-12 13:54:56Amirsetpull_requests: + pull_request17858
2020-02-12 11:05:08Amirsetkeywords: + patch
stage: patch review
pull_requests: + pull_request17850
2020-02-12 07:09:21Amirsetmessages: + msg361865
2020-02-11 18:34:30Amirsetnosy: + Amir
messages: + msg361828
2020-02-11 14:13:39maxpl0itsetmessages: + msg361818
2020-02-11 12:54:59xtreaksetnosy: + xtreak
2020-02-11 12:29:20vstinnersetnosy: + vstinner, orsenthil
messages: + msg361808
2020-02-11 12:27:34vstinnersettitle: Injection in http.client -> [security] http.client: HTTP Header Injection in the HTTP method
2020-02-10 19:29:35maxpl0itcreate