[1.4.X] Fixed #18856 -- Ensured that redirects can't be poisoned by malicious users.

3 files changed, 108 insertions, 5 deletions

diff --git a/tests/regressiontests/comment_tests/tests/comment_view_tests.py b/tests/regressiontests/comment_tests/tests/comment_view_tests.py
index 5edc58fe2d..5a8269368d 100644
--- a/tests/regressiontests/comment_tests/tests/comment_view_tests.py
+++ b/tests/regressiontests/comment_tests/tests/comment_view_tests.py
@@ -222,6 +222,13 @@ class CommentViewTests(CommentTestCase):
         match = re.search(r"^http://testserver/somewhere/else/\?c=\d+$", location)
         self.assertTrue(match != None, "Unexpected redirect location: %s" % location)
 
+        data["next"] = "http://badserver/somewhere/else/"
+        data["comment"] = "This is another comment with an unsafe next url"
+        response = self.client.post("/post/", data)
+        location = response["Location"]
+        match = post_redirect_re.match(location)
+        self.assertTrue(match != None, "Unsafe redirection to: %s" % location)
+
     def testCommentDoneView(self):
         a = Article.objects.get(pk=1)
         data = self.getValidData(a)
diff --git a/tests/regressiontests/comment_tests/tests/moderation_view_tests.py b/tests/regressiontests/comment_tests/tests/moderation_view_tests.py
index e9d2fb1578..54f3f3a86b 100644
--- a/tests/regressiontests/comment_tests/tests/moderation_view_tests.py
+++ b/tests/regressiontests/comment_tests/tests/moderation_view_tests.py
@@ -29,6 +29,30 @@ class FlagViewTests(CommentTestCase):
         self.assertEqual(c.flags.filter(flag=CommentFlag.SUGGEST_REMOVAL).count(), 1)
         return c
 
+    def testFlagPostNext(self):
+        """
+        POST the flag view, explicitly providing a next url.
+        """
+        comments = self.createSomeComments()
+        pk = comments[0].pk
+        self.client.login(username="normaluser", password="normaluser")
+        response = self.client.post("/flag/%d/" % pk, {'next': "/go/here/"})
+        self.assertEqual(response["Location"],
+            "http://testserver/go/here/?c=1")
+
+    def testFlagPostUnsafeNext(self):
+        """
+        POSTing to the flag view with an unsafe next url will ignore the
+        provided url when redirecting.
+        """
+        comments = self.createSomeComments()
+        pk = comments[0].pk
+        self.client.login(username="normaluser", password="normaluser")
+        response = self.client.post("/flag/%d/" % pk,
+            {'next': "http://elsewhere/bad"})
+        self.assertEqual(response["Location"],
+            "http://testserver/flagged/?c=%d" % pk)
+
     def testFlagPostTwice(self):
         """Users don't get to flag comments more than once."""
         c = self.testFlagPost()
@@ -48,7 +72,7 @@ class FlagViewTests(CommentTestCase):
     def testFlaggedView(self):
         comments = self.createSomeComments()
         pk = comments[0].pk
-        response = self.client.get("/flagged/", data={"c":pk})
+        response = self.client.get("/flagged/", data={"c": pk})
         self.assertTemplateUsed(response, "comments/flagged.html")
 
     def testFlagSignals(self):
@@ -100,6 +124,33 @@ class DeleteViewTests(CommentTestCase):
         self.assertTrue(c.is_removed)
         self.assertEqual(c.flags.filter(flag=CommentFlag.MODERATOR_DELETION, user__username="normaluser").count(), 1)
 
+    def testDeletePostNext(self):
+        """
+        POSTing the delete view will redirect to an explicitly provided a next
+        url.
+        """
+        comments = self.createSomeComments()
+        pk = comments[0].pk
+        makeModerator("normaluser")
+        self.client.login(username="normaluser", password="normaluser")
+        response = self.client.post("/delete/%d/" % pk, {'next': "/go/here/"})
+        self.assertEqual(response["Location"],
+            "http://testserver/go/here/?c=1")
+
+    def testDeletePostUnsafeNext(self):
+        """
+        POSTing to the delete view with an unsafe next url will ignore the
+        provided url when redirecting.
+        """
+        comments = self.createSomeComments()
+        pk = comments[0].pk
+        makeModerator("normaluser")
+        self.client.login(username="normaluser", password="normaluser")
+        response = self.client.post("/delete/%d/" % pk,
+            {'next': "http://elsewhere/bad"})
+        self.assertEqual(response["Location"],
+            "http://testserver/deleted/?c=%d" % pk)
+
     def testDeleteSignals(self):
         def receive(sender, **kwargs):
             received_signals.append(kwargs.get('signal'))
@@ -115,13 +166,13 @@ class DeleteViewTests(CommentTestCase):
     def testDeletedView(self):
         comments = self.createSomeComments()
         pk = comments[0].pk
-        response = self.client.get("/deleted/", data={"c":pk})
+        response = self.client.get("/deleted/", data={"c": pk})
         self.assertTemplateUsed(response, "comments/deleted.html")
 
 class ApproveViewTests(CommentTestCase):
 
     def testApprovePermissions(self):
-        """The delete view should only be accessible to 'moderators'"""
+        """The approve view should only be accessible to 'moderators'"""
         comments = self.createSomeComments()
         pk = comments[0].pk
         self.client.login(username="normaluser", password="normaluser")
@@ -133,7 +184,7 @@ class ApproveViewTests(CommentTestCase):
         self.assertEqual(response.status_code, 200)
 
     def testApprovePost(self):
-        """POSTing the delete view should mark the comment as removed"""
+        """POSTing the approve view should mark the comment as removed"""
         c1, c2, c3, c4 = self.createSomeComments()
         c1.is_public = False; c1.save()
 
@@ -145,6 +196,36 @@ class ApproveViewTests(CommentTestCase):
         self.assertTrue(c.is_public)
         self.assertEqual(c.flags.filter(flag=CommentFlag.MODERATOR_APPROVAL, user__username="normaluser").count(), 1)
 
+    def testApprovePostNext(self):
+        """
+        POSTing the approve view will redirect to an explicitly provided a next
+        url.
+        """
+        c1, c2, c3, c4 = self.createSomeComments()
+        c1.is_public = False; c1.save()
+
+        makeModerator("normaluser")
+        self.client.login(username="normaluser", password="normaluser")
+        response = self.client.post("/approve/%d/" % c1.pk,
+            {'next': "/go/here/"})
+        self.assertEqual(response["Location"],
+            "http://testserver/go/here/?c=1")
+
+    def testApprovePostUnsafeNext(self):
+        """
+        POSTing to the approve view with an unsafe next url will ignore the
+        provided url when redirecting.
+        """
+        c1, c2, c3, c4 = self.createSomeComments()
+        c1.is_public = False; c1.save()
+
+        makeModerator("normaluser")
+        self.client.login(username="normaluser", password="normaluser")
+        response = self.client.post("/approve/%d/" % c1.pk,
+            {'next': "http://elsewhere/bad"})
+        self.assertEqual(response["Location"],
+            "http://testserver/approved/?c=%d" % c1.pk)
+
     def testApproveSignals(self):
         def receive(sender, **kwargs):
             received_signals.append(kwargs.get('signal'))
diff --git a/tests/regressiontests/views/tests/i18n.py b/tests/regressiontests/views/tests/i18n.py
index 877cf964dd..68c3e81c4d 100644
--- a/tests/regressiontests/views/tests/i18n.py
+++ b/tests/regressiontests/views/tests/i18n.py
@@ -16,13 +16,28 @@ class I18NTests(TestCase):
     """ Tests django views in django/views/i18n.py """
 
     def test_setlang(self):
-        """The set_language view can be used to change the session language"""
+        """
+        The set_language view can be used to change the session language.
+
+        The user is redirected to the 'next' argument if provided.
+        """
         for lang_code, lang_name in settings.LANGUAGES:
             post_data = dict(language=lang_code, next='/views/')
             response = self.client.post('/views/i18n/setlang/', data=post_data)
             self.assertRedirects(response, 'http://testserver/views/')
             self.assertEqual(self.client.session['django_language'], lang_code)
 
+    def test_setlang_unsafe_next(self):
+        """
+        The set_language view only redirects to the 'next' argument if it is
+        "safe".
+        """
+        lang_code, lang_name = settings.LANGUAGES[0]
+        post_data = dict(language=lang_code, next='//unsafe/redirection/')
+        response = self.client.post('/views/i18n/setlang/', data=post_data)
+        self.assertEqual(response['Location'], 'http://testserver/')
+        self.assertEqual(self.client.session['django_language'], lang_code)
+
     def test_jsi18n(self):
         """The javascript_catalog can be deployed with language settings"""
         saved_lang = get_language()