Redirect hardening (#1460)

weasyl/controllers/settings.py

@@ -798,10 +798,10 @@ def manage_alias_post_(request):
 @token_checked
 @disallow_api
 def sfw_toggle_(request):
-    form = request.web_input(redirect="/")
+    redirect = request.POST.get("redirect", "/")
 
     currentstate = request.cookies.get('sfwmode', "nsfw")
     newstate = "sfw" if currentstate == "nsfw" else "nsfw"
-    response = HTTPSeeOther(location=form.redirect)
+    response = HTTPSeeOther(location=define.path_redirect(redirect))
     response.set_cookie("sfwmode", newstate, max_age=60 * 60 * 24 * 365)
     return response

weasyl/controllers/user.py

@@ -1,5 +1,3 @@
-from urllib.parse import urlsplit, urlunsplit
-
 import arrow
 from pyramid.httpexceptions import HTTPSeeOther
 from pyramid.response import Response
@@ -35,13 +33,13 @@ def signin_get_(request):
 @guest_required
 @token_checked
 def signin_post_(request):
-    form = request.web_input(username="", password="", referer="", sfwmode="nsfw")
-    form.referer = form.referer or '/'
+    form = request.web_input(username="", password="", sfwmode="nsfw")
+    referer = request.POST.get("referer") or "/"
 
     logid, logerror = login.authenticate_bcrypt(form.username, form.password, request=request, ip_address=request.client_addr, user_agent=request.user_agent)
 
     if logid and logerror is None:
-        response = HTTPSeeOther(location=form.referer)
+        response = HTTPSeeOther(location=define.path_redirect(referer))
         response.set_cookie('WZL', request.weasyl_session.sessionid, max_age=60 * 60 * 24 * 365,
                             secure=request.scheme == 'https', httponly=True)
         if form.sfwmode == "sfw":
@@ -72,7 +70,7 @@ def signin_post_(request):
         response = Response(define.webpage(
             request.userid,
             "etc/signin_2fa_auth.html",
-            [define.get_display_name(logid), form.referer, remaining_recovery_codes, None],
+            [define.get_display_name(logid), referer, remaining_recovery_codes, None],
             title="Sign In - 2FA"
         ))
         response.set_cookie('WZL', sess.sessionid, max_age=60 * 60 * 24 * 365,
@@ -81,7 +79,7 @@ def signin_post_(request):
             response.set_cookie("sfwmode", "sfw", max_age=31536000)
         return response
     elif logerror == "invalid":
-        return Response(define.webpage(request.userid, "etc/signin.html", [True, form.referer]))
+        return Response(define.webpage(request.userid, "etc/signin.html", [True, referer]))
     elif logerror == "banned":
         message = moderation.get_ban_message(logid)
         return Response(define.errorpage(request.userid, message))
@@ -108,7 +106,7 @@ def signin_2fa_auth_get_(request):
         login.signout(request)
         raise WeasylError('TwoFactorAuthenticationAuthenticationTimeout')
     else:
-        ref = request.params["referer"] if "referer" in request.params else "/"
+        ref = "/"
         return Response(define.webpage(
             request.userid,
             "etc/signin_2fa_auth.html",
@@ -136,15 +134,14 @@ def signin_2fa_auth_post_(request):
         # 2FA passed, so login and cleanup.
         login.signout(request)
         login.signin(request, tfa_userid, ip_address=request.client_addr, user_agent=request.user_agent)
-        ref = request.params["referer"] or "/"
         # User is out of recovery codes, so force-deactivate 2FA
         if two_factor_auth.get_number_of_recovery_codes(tfa_userid) == 0:
             two_factor_auth.force_deactivate(tfa_userid)
             raise WeasylError('TwoFactorAuthenticationZeroRecoveryCodesRemaining',
                               links=[["2FA Dashboard", "/control/2fa/status"], ["Return to the Home Page", "/"]])
         # Return to the target page, removing the scheme and domain per urlsplit.
-        urlparts = urlsplit(ref)
-        response = HTTPSeeOther(location=urlunsplit(['', '', urlparts[2], urlparts[3], urlparts[4]]))
+        ref = request.POST["referer"] or "/"
+        response = HTTPSeeOther(location=define.path_redirect(ref))
         response.set_cookie('WZL', request.weasyl_session.sessionid, max_age=60 * 60 * 24 * 365,
                             secure=request.scheme == 'https', httponly=True)
         return response
@@ -159,11 +156,11 @@ def signin_2fa_auth_post_(request):
             sess.additional_data['2fa_pwd_auth_attempts'] += 1
             flag_modified(sess, 'additional_data')
             tx.add(sess)
-        # 2FA failed; redirect to 2FA input page & inform user that authentication failed.
+        # 2FA failed; respond with 2FA input page & inform user that authentication failed.
         return Response(define.webpage(
             request.userid,
             "etc/signin_2fa_auth.html",
-            [define.get_display_name(tfa_userid), request.params["referer"], two_factor_auth.get_number_of_recovery_codes(tfa_userid),
+            [define.get_display_name(tfa_userid), request.POST["referer"], two_factor_auth.get_number_of_recovery_codes(tfa_userid),
              "2fa"], title="Sign In - 2FA"))
 
 

weasyl/define.py

@@ -240,6 +240,13 @@ def is_csrf_valid(request):
     return request.headers.get('origin') == _ORIGIN
 
 
+def path_redirect(path_qs: str) -> str:
+    """
+    Return an absolute URL for an internal redirect within the application’s origin.
+    """
+    return _ORIGIN + path_qs
+
+
 @region.cache_on_arguments(namespace='v3')
 def _get_all_config(userid):
     """

weasyl/test/web/test_redirect.py

@@ -0,0 +1,22 @@
+import pytest
+
+from weasyl import define
+from weasyl.test import db_utils
+
+
+@pytest.mark.usefixtures("db", "cache")
+def test_sfw_toggle(app, monkeypatch):
+    monkeypatch.setattr(define, "_ORIGIN", "http://localhost")
+
+    user = db_utils.create_user()
+    app.set_cookie(*db_utils.create_session(user).split("=", 1))
+
+    resp = app.post("/control/sfwtoggle", {
+        "redirect": "/~referer",
+    }, status=303)
+    assert resp.headers["location"] == "http://localhost/~referer", "SFW toggle should redirect back to original page"
+
+    resp = app.post("/control/sfwtoggle", {
+        "redirect": "//evil.example/",
+    }, status=303)
+    assert resp.headers["location"] == "http://localhost//evil.example/", "SFW toggle shouldn’t act as an open redirect"