Merge pull request #2235 from plone/plone-hotfix20171128-isURLInPortal-5.1

gforcada · web-flow · commit 05a943ecbcdd · 2017-12-01T11:37:37.000+01:00
Improved isURLInPortal according to PloneHotfix20171128. [5.1]
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -35,6 +35,12 @@ New features:
 Bug fixes:
 
 - Show example for expression in actions control panel.
+- Improved isURLInPortal according to PloneHotfix20171128.
+  Accept only http/https, and doubly check escaped urls.  [maurits]
+
+- Fixed Products.CMFPlacefulWorkflow being marked as not installed after upgrade from 4.3.
+  This is true for any package in the Products namespace that was installed.
+  Fixes `issue 2103 <https://github.com/plone/Products.CMFPlone/issues/2103>`_.
   [maurits]
 
 - Fixed add-on listed as uninstalled when the default profile is not the first alphabetically.
diff --git a/Products/CMFPlone/URLTool.py b/Products/CMFPlone/URLTool.py
@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 from AccessControl import ClassSecurityInfo
 from App.class_init import InitializeClass
+from HTMLParser import HTMLParser
 from plone.registry.interfaces import IRegistry
 from posixpath import normpath
 from Products.CMFCore.URLTool import URLTool as BaseTool
@@ -11,6 +12,25 @@
 import re
 
 
+hp = HTMLParser()
+# These schemas are allowed in full urls to consider them in the portal:
+# A mailto schema is an obvious sign of a url that is not in the portal.
+# This is a whitelist.
+ALLOWED_SCHEMAS = [
+    'https',
+    'http',
+]
+# These bad parts are not allowed in urls that are in the portal:
+# This is a blacklist.
+BAD_URL_PARTS = [
+    '\\\\',
+    '<script',
+    '%3cscript',
+    'javascript:',
+    'javascript%3a',
+]
+
+
 class URLTool(PloneBaseTool, BaseTool):
 
     meta_type = 'Plone URL Tool'
@@ -34,16 +54,24 @@ def isURLInPortal(self, url, context=None):
         # sanitize url
         url = re.sub('^[\x00-\x20]+', '', url).strip()
         cmp_url = url.lower()
-        if ('\\\\' in cmp_url or
-                '<script' in cmp_url or
-                '%3cscript' in cmp_url or
-                'javascript:' in cmp_url or
-                'javascript%3a' in cmp_url):
-            return False
+        for bad in BAD_URL_PARTS:
+            if bad in cmp_url:
+                return False
 
         p_url = self()
 
-        _, u_host, u_path, _, _, _ = urlparse(url)
+        schema, u_host, u_path, _, _, _ = urlparse(url)
+        if schema and schema not in ALLOWED_SCHEMAS:
+            # Redirecting to 'data:' may be harmful,
+            # and redirecting to 'mailto:' or 'ftp:' is silly.
+            return False
+
+        # Someone may be doing tricks with escaped html code.
+        unescaped_url = hp.unescape(url)
+        if unescaped_url != url:
+            if not self.isURLInPortal(unescaped_url):
+                return False
+
         if not u_host and not u_path.startswith('/'):
             if context is None:
                 return True  # old behavior
diff --git a/Products/CMFPlone/tests/testURLTool.py b/Products/CMFPlone/tests/testURLTool.py
@@ -136,3 +136,43 @@ def test_double_back_slash(self):
         url_tool = self._makeOne()
         iURLiP = url_tool.isURLInPortal
         self.assertFalse(iURLiP('\\\\www.example.com'))
+
+    def test_regression_absolute_url_in_portal(self):
+        url_tool = self._makeOne()
+        iURLiP = url_tool.isURLInPortal
+        self.assertTrue(iURLiP(url_tool()))
+        self.assertTrue(iURLiP(url_tool() + '/shrubbery?knights=ni#ekki-ekki'))
+
+    def test_mailto_simple_not_in_portal(self):
+        url_tool = self._makeOne()
+        iURLiP = url_tool.isURLInPortal
+        self.assertFalse(iURLiP(
+            'mailto:someone@example.org')
+        )
+
+    def test_mailto_complex_not_in_portal(self):
+        url_tool = self._makeOne()
+        iURLiP = url_tool.isURLInPortal
+        self.assertFalse(iURLiP(
+            'mailto&#58;192&#46;168&#46;163&#46;154&#58;8080&#47;Plone&apos;'
+            '&quot;&gt;&lt;html&gt;&lt;svg&#32;onload&#61;alert&#40;document'
+            '&#46;domain&#41;&gt;&lt;&#47;html&gt;')
+        )
+
+    def test_data_not_in_portal(self):
+        url_tool = self._makeOne()
+        iURLiP = url_tool.isURLInPortal
+        self.assertFalse(iURLiP(
+            'data:text/html%3bbase64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4K')
+        )
+
+    def test_double_slash(self):
+        # I wondered if this might be a problem after reading
+        # https://bugs.python.org/issue23505
+        # Apparently not, but let's test it.
+        url_tool = self._makeOne()
+        iURLiP = url_tool.isURLInPortal
+        self.assertFalse(iURLiP(
+            '//www.google.com'))
+        self.assertFalse(iURLiP(
+            '////www.google.com'))
