django.contrib.auth.views.login refuses to redirect to urls with spaces

[Jeremy Lainé]

While logged out, I am trying to access a page which is protected by the "login_required" decorator at:

​http://example.com/foo%20bar/

I get redirected to:

​http://example.com/accounts/login?next=/foo%20bar/

Once I enter my credentials, instead of getting redirected to the expected page, I get sent to the default URL as defined by settings.LOGIN_REDIRECT_URL.

This bug is due to the code at line 24 of django/contrib/auth/views.py:

# Light security check -- make sure redirect_to isn't garbage.
if not redirect_to or '' in redirect_to or ' ' in redirect_to:

redirect_to = settings.LOGIN_REDIRECT_URL

Could someone please explain how checking for spaces or double slashes is a "security check"? From my point of view, it's a bug, django refuses to redirect me to an URL which is perfectly valid!

Many thanks in advance!

[Luke Plant]

I'm guessing the double slash check is to stop redirecting to an external site, which would be a phishing vulnerability. I don't know about the space thing, it does seem like it should be allowed to redirect to a URL with a space in it, as you are allowed to have spaces in the path element of the URL.

If you allow a space, it seems to work fine (tested in Firefox and Opera). Some browsers, such as Firefox, actually display a space in the address bar, rather than %20.

The only concern I have is with the redirect. Django sets "Location" to "/foo bar/" if you allow the space. With the development server, this somehow gets translated to "Location: /foo%20bar/" in the header that is sent to the browser, I haven't tested with modpython.

[Russell Keith-Magee]

Duplicate of #11457.

[Aymeric Augustin]

#11457 handles URLs containing , but it does not address the main issue of this ticket, which is URLs containing spaces.

The comment explaining the restriction on spaces is unclear:

# Light security check -- make sure redirect_to isn't garbage.

and no better reason was found, see:
​http://code.djangoproject.com/ticket/11457#comment:3
​http://code.djangoproject.com/ticket/11457#comment:4

Could the restriction be either explained or removed?

To remove it, line 34 of django/contrib/auth/views.py must be changed from:

    if not redirect_to or ' ' in redirect_to:

to

    if not redirect_to:

[Russell Keith-Magee]

Ok - #11457 *was* a duplicate; but it got committed without addressing this issue.

[anonymous]

The url below (where the redirect_to field has a query string embedded) has a space in one of the query string parameters. This breaks the login view since it rejects the redirect_to.

​http://127.0.0.1:8000/login/?next=/%3Fanswer%3Dtest%20answer%26puzzle_id%3D461

[John Anderson]

I looked in the history and it has been that way as long as Django has been public, I don't think there is any true security reason for having that check.

[Jannis Vajen]

Removes check for space in redirect_to

[Aymeric Augustin]

RFC 2616 (HTTP) says:

For definitive information on URL syntax and semantics,
see "Uniform Resource Identifiers (URI): Generic Syntax and Semantics," RFC 2396

RFC 2396 allows using the SPACE character in URLs, encoded as %20 (it is already decoded when the view is called). It even uses it as an example in §2.4.1 :

For example, "%20" is the escaped encoding for the US-ASCII space character."

So there is no reason to forbid the SPACE character.

Regarding lukeplant's comment above, HttpResponseRedirect and friends re-encode the URL in the Location header like this:

self['Location'] = iri_to_uri(redirect_to)

So we are safe.

The patch applies cleanly (with -p1 - it's a git patch) and does not break any tests. Looks good.

[Aymeric Augustin]

Added test.

[Jannis Leidel]

In [15702]:

Fixed #12534 -- Loosened the the security check for "next" redirects after logins slightly to allow paths that contain spaces. Thanks for the patch, jnns and aaugustin.

[Jacob]

Milestone 1.3 deleted