[issue11662] Redirect vulnerability in urllib/urllib2

[Guido van Rossum]

Guido van Rossum <guido at python.org> added the comment:

> Which patch should be reviewed? They seem to be different.

Both. Mine's for the Python 2 line while Senthil seems to deal with
Python 3. (However the presence of Senthil's patch somehow overrode my
patch in Rietveld. It looks like Martin didn't think of this use
case.) I'd like to have agreement over the Python 2 patch first, then
we can think about forward porting.

> Senthil's patch allows a redirect to ftp while Guido's doesn't.

That is a good question. Should we? It doesn't look like ftp:
participates in the vulnerability, but I'm not sure how useful it is
either.

> Senthil's patch doesn't seem to fix urllib-inherited code, only urllib2- (see FancyURLopener.redirect_internal()).

Right, that's for Python 3.

> Guido's patch doesn't close the file (fp.close()) when the redirect is denied.

But the calling code does. Note that when there is no URI or Location
header, redirect_internal() also returns without closing the file; if
the error handler returns no result, http_error() will call
http_error_default() which closes the file.

> Both patches apparently return silently (?), while it might be better to raise an exception.

This follows the tradition of returning silently when no URI or
Location header is found. The 302 error will be treated the same as
any other error.

> Both would deserve a test :)

If someone would contribute one I'd appreciate it. Otherwise I will
get on it myself.

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue11662>
_______________________________________