audit site for open redirects

[chadwhitacre]

Reticketed from #30.

[seanlinsley]

Searching for redirect in the code, I found two files of interest: sign-out.html.spt and associate.spt.

The sign out page isn't vulnerable since it requires a CSRF token to POST, so only you can exit your current session (unlike @homakov's Facebook example). Though in my opinion it seems a bit over-engineered. I'd simplify it a bit:

-    if 'back_to' in request.body:
-        back_to = request.body['back_to']
-    else:
-        back_to = request.headers.get('referer', '/')
-
-    request.redirect(back_to)
+    request.redirect(request.headers.get('referer', '/'))

 [-----------------------------------------------------------------------------]
 {% extends "templates/base.html" %}
@@ -25,7 +20,6 @@ if POST:
     <div class="as-content">
         <h1>Are you sure you wish to sign out?</h1>
         <form id="sign-out" method="POST">
-            <input name="back_to" type="hidden" value="/" />
             <input name="csrf_token" type="hidden" value="{{ csrf_token }}" />
             <button type="submit">Yes!</button>

The associate page redirects based on a string provided by the request itself. I was able to get part way through the process with this hand-crafted request

curl "localhost:8537/on/twitter/associate.html?oauth_token=foo" \
  -b twitter_foo=`echo '["a","b","c","d"]' | base64`

This doesn't seem to be exploitable because the OAuth library we use consistently raises exceptions since it's expecting valid data back from the POST it sends to the given OAuth provider. Still, it might make sense to add an assertation in the code to be safe.

assert url_on_gittip_domain(then) # however we'd like to determine that
request.redirect(then)

Alternatively, this appears to work well:

request.redirect(canonical_scheme+'://'+canonical_host+then)

On this subject @whit537, it'd be nice if there was an option you could pass to redirect to make it only redirect to URLs on the current domain.

[chadwhitacre]

!m @seanlinsley

it'd be nice if there was an option you could pass to redirect to make it only redirect to URLs on the current domain.

Care to ticket that in Aspen?

And could you also search for 301, 302, and Location? There may be places where we redirect manually by raising a Response rather than using request.redirect.

[seanlinsley]

Ticketed.

Yeah, looks like we have some code that manually performs redirection. It's just about 1 am here, so that's it for me today 😸

[chadwhitacre]

The only good protection is to respond with full path: Location: http://myhost/ + params[:next]

[chadwhitacre]

I think the way to fix this is to clean up Aspen's redirect helper to prefix a base URL if location.startswith('/').

[chadwhitacre]

IRC

[chadwhitacre]

Done in #3588.