Redirect security check in login code is incomplete

[Sander Dijkhuis <sander.dijkhuis@…]

The security check for the value of redirect_to in django.contrib.auth.views.login is incomplete. It's meant to block incorrect URLs and external locations, but it will still redirect to external sites if the URL doesn't include the protocol name. This is because '' isn't blocked. So currently, /accounts/login/?next=example.com/ will redirect the user to ​http://example.com/ after a successful authentication. This can be considered a small security problem.

It can be fixed by modifying ​line 20:

            if not redirect_to or '://' in redirect_to or ' ' in redirect_to:

should be:

            if not redirect_to or '//' in redirect_to or ' ' in redirect_to:

[Adrian Holovaty]

(In [6004]) Fixed #5227 -- Made the redirect security check in django.contrib.auth.views.login() tighter. Thanks, Sander Dijkhuis

[dnagoda@…]

I believe that the implemented fix is overly broad. My concrete example is the OAuth token authorization cycle. In order for a user to authorize the token a GET request is made to the OAuth provider with an oauth_callback parameter containing the callback URL. Properly escaped this parameter will look like this:

oauth_callback=http%3D//example.com/

Given that the check for '' in the redirect checks the entire string, a query parameter as above will cause the security check to be triggered and the user will be redirected incorrectly.

If the concern is that a redirect request that starts with '' could (that's a big "could" because I believe that it's browser dependent what the behavior would be) then I think the appropriate fix is to change the check to this:

if not redirect_to redirect_to.startswith('//') or '://' in redirect_to or ' ' in redirect_to:

[Alex Gaynor]

This bug was fixed 2 years ago, if you think there is a new bug please file a new ticket.