[Checkins] SVN: zope.publisher/trunk/ Fix #98471: Restrict redirects to current host. This causes a ValueError to be

Christian Theune ct at gocept.com
Sat Jul 4 10:45:47 EDT 2009


Log message for revision 101538:
  Fix #98471: Restrict redirects to current host. This causes a ValueError to be
  raised in the case of redirecting to a different host. If this is intentional,
  the parameter `trusted` can be given.
  

Changed:
  U   zope.publisher/trunk/CHANGES.txt
  U   zope.publisher/trunk/src/zope/publisher/browser.py
  U   zope.publisher/trunk/src/zope/publisher/http.py
  U   zope.publisher/trunk/src/zope/publisher/interfaces/http.py
  U   zope.publisher/trunk/src/zope/publisher/tests/test_http.py

-=-
Modified: zope.publisher/trunk/CHANGES.txt
===================================================================
--- zope.publisher/trunk/CHANGES.txt	2009-07-04 14:44:15 UTC (rev 101537)
+++ zope.publisher/trunk/CHANGES.txt	2009-07-04 14:45:47 UTC (rev 101538)
@@ -4,7 +4,9 @@
 3.8.1 (unreleased)
 ------------------
 
-- ...
+- Fix #98471: Restrict redirects to current host. This causes a ValueError to
+  be raised in the case of redirecting to a different host. If this is
+  intentional, the parameter `trusted` can be given.
 
 3.8.0 (2009-05-23)
 ------------------

Modified: zope.publisher/trunk/src/zope/publisher/browser.py
===================================================================
--- zope.publisher/trunk/src/zope/publisher/browser.py	2009-07-04 14:44:15 UTC (rev 101537)
+++ zope.publisher/trunk/src/zope/publisher/browser.py	2009-07-04 14:45:47 UTC (rev 101538)
@@ -728,7 +728,7 @@
     def setBase(self, base):
         self._base = base
 
-    def redirect(self, location, status=None):
+    def redirect(self, location, status=None, trusted=False):
         base = getattr(self, '_base', '')
         if base and isRelative(str(location)):
             l = base.rfind('/')
@@ -746,7 +746,7 @@
         # if isRelative(str(location)):
         #     raise AssertionError('Cannot determine absolute location')
 
-        return super(BrowserResponse, self).redirect(location, status)
+        return super(BrowserResponse, self).redirect(location, status, trusted)
 
     def reset(self):
         super(BrowserResponse, self).reset()

Modified: zope.publisher/trunk/src/zope/publisher/http.py
===================================================================
--- zope.publisher/trunk/src/zope/publisher/http.py	2009-07-04 14:44:15 UTC (rev 101537)
+++ zope.publisher/trunk/src/zope/publisher/http.py	2009-07-04 14:45:47 UTC (rev 101538)
@@ -15,14 +15,17 @@
 
 $Id$
 """
-import re, time, random
+import logging
+import random
+import re
+import time
+import urlparse
 from cStringIO import StringIO
 from urllib import quote, unquote, splitport
 from types import StringTypes, ClassType
 from cgi import escape
 from Cookie import SimpleCookie
 from Cookie import CookieError
-import logging
 from tempfile import TemporaryFile
 
 from zope import component, interface, event
@@ -877,8 +880,15 @@
         """
         return self.__class__()
 
-    def redirect(self, location, status=None):
+    def redirect(self, location, status=None, trusted=False):
         """Causes a redirection without raising an error"""
+        if not trusted:
+            scheme, target_host, path, query, fragment = (
+                urlparse.urlsplit(location))
+            if target_host and target_host != self._request['HTTP_HOST']:
+                raise ValueError(
+                    "Untrusted redirect to host %r not allowed." % target_host)
+
         if status is None:
             # parse the HTTP version and set default accordingly
             if (self._request.get("SERVER_PROTOCOL","HTTP/1.0") <

Modified: zope.publisher/trunk/src/zope/publisher/interfaces/http.py
===================================================================
--- zope.publisher/trunk/src/zope/publisher/interfaces/http.py	2009-07-04 14:44:15 UTC (rev 101537)
+++ zope.publisher/trunk/src/zope/publisher/interfaces/http.py	2009-07-04 14:45:47 UTC (rev 101538)
@@ -227,11 +227,17 @@
 
 
 class IHTTPApplicationResponse(Interface):
-    """HTTP Response
-    """
+    """HTTP Response"""
 
-    def redirect(location, status=302):
+    def redirect(location, status=302, trusted=False):
         """Causes a redirection without raising an error.
+
+        By default redirects are untrusted which restricts target URLs to the
+        same host that the request was sent to.
+
+        If the `trusted` flag is set, redirects are allowed for any target
+        URL.
+
         """
 
 class IHeaderOutput(Interface):

Modified: zope.publisher/trunk/src/zope/publisher/tests/test_http.py
===================================================================
--- zope.publisher/trunk/src/zope/publisher/tests/test_http.py	2009-07-04 14:44:15 UTC (rev 101537)
+++ zope.publisher/trunk/src/zope/publisher/tests/test_http.py	2009-07-04 14:45:47 UTC (rev 101538)
@@ -282,6 +282,27 @@
         request.response.redirect('http://foobar.com/explicit', 304)
         self.assertEquals(request.response.getStatus(), 304)
 
+
+    def testUntrustedRedirect(self):
+        # Redirects are by default only allowed to target the same host as the
+        # request was directed to. This is to counter fishing.
+        request = self._createRequest({}, '')
+        self.assertRaises(
+            ValueError,
+            request.response.redirect, 'http://phishing-inc.com')
+
+        # Redirects with relative URLs are treated as redirects to the current
+        # host. They aren't really allowed per RFC but the response object
+        # supports them and people are probably using them.
+        location = request.response.redirect('/foo', trusted=False)
+        self.assertEquals('/foo', location)
+
+        # If we pass `trusted` for the redirect, we can redirect the browser
+        # anywhere we want, though.
+        location = request.response.redirect(
+            'http://my-friends.com', trusted=True)
+        self.assertEquals('http://my-friends.com', location)
+
     def testUnregisteredStatus(self):
         # verify we can set the status to an unregistered int value
         request = self._createRequest({}, '')



More information about the Checkins mailing list