"HTTPoxy", use of HTTP_PROXY flag supplied by attacker in CGI scripts #71755

remram44 · 2016-07-18T22:30:14Z

BPO	27568
Nosy	@jcea, @orsenthil, @encukou, @vadmium, @sigmavirus24, @Lukasa, @remram44
Files	python-2.7-httpoxy.patch python-3.5-httpoxy.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/orsenthil'
closed_at = <Date 2016-07-31.06:57:15.341>
created_at = <Date 2016-07-18.22:30:13.775>
labels = ['type-security', 'library']
title = '"HTTPoxy", use of HTTP_PROXY flag supplied by attacker in CGI scripts'
updated_at = <Date 2016-08-11.03:10:11.407>
user = 'https://github.com/remram44'

bugs.python.org fields:

activity = <Date 2016-08-11.03:10:11.407>
actor = 'jcea'
assignee = 'orsenthil'
closed = True
closed_date = <Date 2016-07-31.06:57:15.341>
closer = 'orsenthil'
components = ['Library (Lib)']
creation = <Date 2016-07-18.22:30:13.775>
creator = 'remram'
dependencies = []
files = ['43943', '43944']
hgrepos = []
issue_num = 27568
keywords = ['patch']
message_count = 17.0
messages = ['270795', '270800', '270811', '270819', '270840', '270844', '270854', '270901', '271617', '271624', '271658', '271687', '271688', '271725', '271726', '271893', '272104']
nosy_count = 9.0
nosy_names = ['jcea', 'orsenthil', 'frispete', 'petr.viktorin', 'python-dev', 'martin.panter', 'icordasc', 'Lukasa', 'remram']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue27568'
versions = ['Python 2.7', 'Python 3.3', 'Python 3.4', 'Python 3.5', 'Python 3.6']

remram44 · 2016-07-18T22:30:14Z

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

vadmium · 2016-07-19T01:22:00Z

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.

remram44 · 2016-07-19T04:14:13Z

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.

Lukasa · 2016-07-19T09:45:43Z

I like this patch a great deal, I'll happily review it with docs and tests.

Lukasa · 2016-07-19T19:18:45Z

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!

orsenthil · 2016-07-19T21:17:15Z

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.

remram44 · 2016-07-20T04:07:43Z

Added CVE number
Link to full note on getproxies() doc
Improved comment on uppercase (lowercase will be preferred to mIxED_case too)

vadmium · 2016-07-21T01:54:18Z

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.)

encukou · 2016-07-29T10:07:39Z

The conversation seems to have stalled. Rémi, are you still working on the patch? Should someone take over?

remram44 · 2016-07-29T14:02:44Z

I was away for a bit, I will make the requested changes tonight.

remram44 · 2016-07-30T00:13:03Z

Here it goes

Clarified that _proxy suffix should be lowercase
Indented ..note: blocks under function/class

orsenthil · 2016-07-30T12:37:50Z

The patch looks good to me. I am checking this in.

orsenthil · 2016-07-30T13:14:09Z

For 3.3, 3.4 it seems reasonable to backport changes from bpo-26804 and then apply this patch. I will do this today.

python-dev · 2016-07-31T06:51:52Z

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

orsenthil · 2016-07-31T06:57:15Z

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 bpo-26804 which will allow lower cased proxies, but then, I decided against it as it will introduce unnecessary changes to this security fix releases).

frispete · 2016-08-03T11:15:23Z

(In msg271688, I pondered if I need to backport a behavior change from bpo-26804 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.

orsenthil · 2016-08-06T18:06:10Z

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 bpo-26804, 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.

remram44 mannequin added type-feature A feature request or enhancement stdlib Python modules in the Lib dir labels Jul 18, 2016

berkerpeksag added type-security A security issue and removed type-feature A feature request or enhancement labels Jul 18, 2016

orsenthil self-assigned this Jul 30, 2016

orsenthil closed this as completed Jul 31, 2016

ezio-melotti transferred this issue from another repository Apr 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

"HTTPoxy", use of HTTP_PROXY flag supplied by attacker in CGI scripts #71755

"HTTPoxy", use of HTTP_PROXY flag supplied by attacker in CGI scripts #71755

remram44 mannequin commented Jul 18, 2016

remram44 mannequin commented Jul 18, 2016

vadmium commented Jul 19, 2016

remram44 mannequin commented Jul 19, 2016

Lukasa mannequin commented Jul 19, 2016

Lukasa mannequin commented Jul 19, 2016

orsenthil commented Jul 19, 2016

remram44 mannequin commented Jul 20, 2016

vadmium commented Jul 21, 2016

encukou commented Jul 29, 2016

remram44 mannequin commented Jul 29, 2016

remram44 mannequin commented Jul 30, 2016

orsenthil commented Jul 30, 2016

orsenthil commented Jul 30, 2016

python-dev mannequin commented Jul 31, 2016

orsenthil commented Jul 31, 2016

frispete mannequin commented Aug 3, 2016

orsenthil commented Aug 6, 2016

"HTTPoxy", use of HTTP_PROXY flag supplied by attacker in CGI scripts #71755

"HTTPoxy", use of HTTP_PROXY flag supplied by attacker in CGI scripts #71755

Comments

remram44 mannequin commented Jul 18, 2016

remram44 mannequin commented Jul 18, 2016

vadmium commented Jul 19, 2016

remram44 mannequin commented Jul 19, 2016

Lukasa mannequin commented Jul 19, 2016

Lukasa mannequin commented Jul 19, 2016

orsenthil commented Jul 19, 2016

remram44 mannequin commented Jul 20, 2016

vadmium commented Jul 21, 2016

encukou commented Jul 29, 2016

remram44 mannequin commented Jul 29, 2016

remram44 mannequin commented Jul 30, 2016

orsenthil commented Jul 30, 2016

orsenthil commented Jul 30, 2016

python-dev mannequin commented Jul 31, 2016

orsenthil commented Jul 31, 2016

frispete mannequin commented Aug 3, 2016

orsenthil commented Aug 6, 2016