classification
Title: "HTTPoxy", use of HTTP_PROXY flag supplied by attacker in CGI scripts
Type: security Stage: resolved
Components: Library (Lib) Versions: Python 3.6, Python 3.5, Python 3.3, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: orsenthil Nosy List: Lukasa, encukou, frispete, icordasc, jcea, martin.panter, orsenthil, python-dev, remram
Priority: normal Keywords: patch

Created on 2016-07-18 22:30 by remram, last changed 2016-08-11 03:10 by jcea. This issue is now closed.

Files
File name Uploaded Description Edit
python-2.7-httpoxy.patch remram, 2016-07-30 00:12
python-3.5-httpoxy.patch remram, 2016-07-30 00:13
Messages (17)
msg270795 - (view) Author: Rémi Rampin (remram) * Date: 2016-07-18 22:30
https://httpoxy.org/

It is possible to set the HTTP_PROXY in CGI scripts by passing the Proxy header. If the script is a Python script and downloads files, urllib will happily use the attacker-supplied proxy to make requests.

This should be mitigated like it is in Perl (since 2001), Ruby, and libraries like curl.

See also: bug against python-requests https://github.com/kennethreitz/requests/issues/3422
msg270800 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-07-19 01:21
I suspect this won’t help on OSes like Windows where environment variable names are case-insensitive (correct me if I am wrong).

Regardless, it may be worth making the change. It would be nice to also add test case(s). And I wonder if it would be appropriate to add a notice to the documentation saying that uppercase HTTP_PROXY is ignored if REQUEST_METHOD exists.
msg270811 - (view) Author: Rémi Rampin (remram) * Date: 2016-07-19 04:14
I am willing to work on documentation and tests if there is an interest in the patch.

On Windows, if REQUEST_METHOD is set, it is probably safe to assume that HTTP_* variables come from the web server: setting this variable is not the way we set a proxy there, so ignoring this dubious variable is probably fine.
msg270819 - (view) Author: Cory Benfield (Lukasa) * Date: 2016-07-19 09:45
I like this patch a great deal, I'll happily review it with docs and tests.
msg270840 - (view) Author: Cory Benfield (Lukasa) * Date: 2016-07-19 19:18
Ok, so I've taken a preliminary look at this patch. It looks good to me! I have one question: right now the patch as written will blow away not just HTTP_PROXY, but also any other mixed-case spelling of that name (e.g. HtTp_PrOxY) in a CGI environment.

That's *probably* not a concern: I think in practice such a spelling is almost never used. However, I wanted to draw it out explicitly: we should probably include a code comment that indicates that we know that it's a side effect of the code, and that we don't care.

For what it's worth, we should also consider commenting with a note regarding the CVE number assigned to Python. We may want to consider getting a CVE number for this specific fix as well, though I'd need to chat to someone in the PSRT at this point to get an idea of what they think.

Good work!
msg270844 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2016-07-19 21:17
Thanks for this patch. The CVE number assigned to python -  CVE-2016-1000110.

There is redundancy in /Doc/library/urllib.request.rst change where the same paragraph is repeated twice. See if you can have it at a single location as a `Note` and reference it.
msg270854 - (view) Author: Rémi Rampin (remram) * Date: 2016-07-20 04:07
- Added CVE number
- Link to full note on getproxies() doc
- Improved comment on uppercase (lowercase will be preferred to mIxED_case too)
msg270901 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-07-21 01:54
I think I misunderstood the Windows situation. Now I understand Windows has no lower-case variable names, so this patch would stop accepting any HTTP_PROXY variable there (in CGI mode). But that is okay by me.

I agree the mixed-case scenario is not worth worrying too much about. The normal scenario is all lowercase (http_proxy), and I think all-uppercase (HTTP_PROXY) is only supported for compatibility with some older browsers or OSes (can’t remember the details). However, since we already document “a case-insensitive approach”, perhaps it needs tweaking somehow. Perhaps it would be more correct to say, in CGI mode:

* Only lowercase _proxy suffix is accepted (stricter than just ignoring uppercase)
* No variable is accepted where names must be uppercase, i.e. Windows. As I understand it, you cannot have a lowercase http_proxy variable there.

Also, I think the “note” additions should be indented under the getproxies() etc headings. (Or drop the markup and make it an ordinary sentence or paragraph. “Note that” is also redundant IMO.)
msg271617 - (view) Author: Petr Viktorin (encukou) * Date: 2016-07-29 10:07
The conversation seems to have stalled. Rémi, are you still working on the patch? Should someone take over?
msg271624 - (view) Author: Rémi Rampin (remram) * Date: 2016-07-29 14:02
I was away for a bit, I will make the requested changes tonight.
msg271658 - (view) Author: Rémi Rampin (remram) * Date: 2016-07-30 00:13
Here it goes
- Clarified that _proxy suffix should be lowercase
- Indented ..note: blocks under function/class
msg271687 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2016-07-30 12:37
The patch looks good to me. I am checking this in.
msg271688 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2016-07-30 13:14
For 3.3, 3.4 it seems reasonable to backport changes from issue26804 and then apply this patch. I will do this today.
msg271725 - (view) Author: Roundup Robot (python-dev) Date: 2016-07-31 06:51
New changeset 95b09ccc8a3e by Senthil Kumaran in branch '3.3':
Prevent HTTPoxy attack (CVE-2016-1000110)
https://hg.python.org/cpython/rev/95b09ccc8a3e

New changeset 3c19023c9fec by Senthil Kumaran in branch '3.4':
[merge from 3.3] Prevent HTTPoxy attack (CVE-2016-1000110)
https://hg.python.org/cpython/rev/3c19023c9fec

New changeset a0ac52ed8f79 by Senthil Kumaran in branch '3.5':
[merge from 3.4] - Prevent HTTPoxy attack (CVE-2016-1000110)
https://hg.python.org/cpython/rev/a0ac52ed8f79

New changeset 6c2e2de5ab8e by Senthil Kumaran in branch 'default':
[merge from 3.5] - Prevent HTTPoxy attack (CVE-2016-1000110)
https://hg.python.org/cpython/rev/6c2e2de5ab8e
msg271726 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2016-07-31 06:57
This is also committed in 2.7 branch in ba915d561667.

This is committed in all active versions(2.7, 3.5, 3.6) and also versions which receive security updates (3.3 and 3.4).

This issue is resolved. Thank you for the patch, Rémi.


(In msg271688, I pondered if I need to backport a behavior change from issue26804 which will allow lower cased proxies, but then, I decided against it as it will introduce unnecessary changes to this security fix releases).
msg271893 - (view) Author: Hans-Peter Jansen (frispete) * Date: 2016-08-03 11:15
> (In msg271688, I pondered if I need to backport a behavior change from issue26804 which will allow lower cased proxies, but then, I decided against it as it will introduce unnecessary changes to this security fix releases).

Hmm, Senthil, while I understand, that you want to avoid unnecessary changes, doesn't this result in non deterministic behaviour of proxy handling without my patch? 

+       header. If you need to use an HTTP proxy in a CGI environment, either use
+       ``ProxyHandler`` explicitly, or make sure the variable name is in
+       lowercase (or at least the ``_proxy`` suffix).

Without 26804, this fix works by chance only for 3.3 and 3.4, since it depends on os.environ dictionary order, which is non deterministic by definition. 26804 resolves this by making sure, a lower case _proxy var has a higher priority over the other variants.
msg272104 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2016-08-06 18:06
Hi Hans-Peter,

In 3.3 (95b09ccc8a3e) and 3.4 (3c19023c9fec) the change completely removes any variant of http_proxy if REQUEST_METHOD is set.  The only way to have http based proxy in cgi environment by using ProxyHandler method. This is solution introduced for the security fix.

If I backport your patch from issue26804, I imagined that we will be introducing a new feature for other environment variables like NO_PROXY, which folks might be prepared for in the security fix release. That was my concern in not making the other change.  Hope this reasoning helps.
History
Date User Action Args
2016-08-11 03:10:11jceasetnosy: + jcea
2016-08-06 18:06:11orsenthilsetmessages: + msg272104
2016-08-03 11:15:23frispetesetnosy: + frispete
messages: + msg271893
2016-07-31 07:50:45berker.peksagsetstage: patch review -> resolved
2016-07-31 06:57:15orsenthilsetstatus: open -> closed
resolution: fixed
messages: + msg271726
2016-07-31 06:51:52python-devsetnosy: + python-dev
messages: + msg271725
2016-07-30 13:14:09orsenthilsetassignee: orsenthil
messages: + msg271688
2016-07-30 12:37:50orsenthilsetmessages: + msg271687
2016-07-30 00:13:03remramsetfiles: + python-3.5-httpoxy.patch

messages: + msg271658
2016-07-30 00:12:14remramsetfiles: + python-2.7-httpoxy.patch
2016-07-30 00:11:49remramsetfiles: - python-3.5-httpoxy.patch
2016-07-30 00:11:48remramsetfiles: - python-2.7-httpoxy.patch
2016-07-29 14:02:44remramsetmessages: + msg271624
2016-07-29 10:43:44icordascsetnosy: + icordasc
2016-07-29 10:07:39encukousetnosy: + encukou
messages: + msg271617
2016-07-21 01:54:18martin.pantersetmessages: + msg270901
2016-07-20 04:08:14remramsetfiles: - python-3.5-httpoxy.patch
2016-07-20 04:08:13remramsetfiles: - python-2.7-httpoxy.patch
2016-07-20 04:07:56remramsetfiles: + python-3.5-httpoxy.patch
2016-07-20 04:07:46remramsetfiles: + python-2.7-httpoxy.patch

messages: + msg270854
2016-07-19 21:17:15orsenthilsetmessages: + msg270844
2016-07-19 19:18:44Lukasasetmessages: + msg270840
2016-07-19 18:44:08remramsetfiles: + python-3.5-httpoxy.patch
2016-07-19 18:43:55remramsetfiles: + python-2.7-httpoxy.patch
2016-07-19 18:41:46remramsetfiles: - python-3.5-httpoxy-mitigation.patch
2016-07-19 18:41:39remramsetfiles: - python-2.7-httpoxy-mitigation.patch
2016-07-19 09:45:43Lukasasetnosy: + Lukasa
messages: + msg270819
2016-07-19 04:14:13remramsetmessages: + msg270811
2016-07-19 01:22:00martin.pantersetnosy: + martin.panter
messages: + msg270800
2016-07-18 22:47:04berker.peksagsetnosy: + orsenthil
stage: patch review
type: enhancement -> security

versions: + Python 2.7, Python 3.3, Python 3.4, Python 3.5, Python 3.6
2016-07-18 22:31:20remramsetfiles: + python-3.5-httpoxy-mitigation.patch
2016-07-18 22:31:03remramsetfiles: + python-2.7-httpoxy-mitigation.patch
keywords: + patch
2016-07-18 22:30:13remramcreate