diff --git a/Products/CMFPlone/PloneTool.py b/Products/CMFPlone/PloneTool.py index f9503125d4..8deb7e2b52 100644 --- a/Products/CMFPlone/PloneTool.py +++ b/Products/CMFPlone/PloneTool.py @@ -119,12 +119,14 @@ def getSiteEncoding(self): @security.public def portal_utf8(self, str, errors='strict'): - """Transforms an string in portal encoding to utf8.""" + # Transforms an string in portal encoding to utf8. + # Note: no docstring please, to avoid reflected XSS. return utils.portal_utf8(self, str, errors) @security.public def utf8_portal(self, str, errors='strict'): - """Transforms an utf8 string to portal encoding.""" + # Transforms an utf8 string to portal encoding. + # Note: no docstring please, to avoid reflected XSS. return utils.utf8_portal(self, str, errors) @security.private @@ -336,11 +338,9 @@ def getIconFor(self, category, id, default=_marker, context=None): @security.protected(View) def getReviewStateTitleFor(self, obj): - """Utility method that gets the workflow state title for the - object's review_state. - - Returns None if no review_state found. - """ + # Utility method that gets the workflow state title for the + # object's review_state. + # Returns None if no review_state found. wf_tool = getToolByName(self, 'portal_workflow') wfs = () objstate = None @@ -407,18 +407,17 @@ def fixOwnerRole(object, user_id): @security.public def urlparse(self, url): - """Returns the pieces of url in a six-part tuple. - - Since Python 2.6: urlparse now returns a ParseResult object. - We just need the tuple form which is tuple(result). - """ + # Returns the pieces of url in a six-part tuple. + # Since Python 2.6: urlparse now returns a ParseResult object. + # We just need the tuple form which is tuple(result). + # Note: no docstring please, to avoid reflected XSS. return tuple(parse.urlparse(url)) @security.public def urlunparse(self, url_tuple): - """Puts a url back together again, in the manner that - urlparse breaks it. - """ + # Puts a url back together again, in the manner that + # urlparse breaks it. + # Note: no docstring please, to avoid reflected XSS. return parse.urlunparse(url_tuple) # Enable scripts to get the string value of an exception even if the @@ -551,30 +550,32 @@ def getDefaultPage(self, obj, request=None): @security.public def addPortalMessage(self, message, type='info', request=None): - """\ - Call this once or more to add messages to be displayed at the - top of the web page. - - The arguments are: - message: a string, with the text message you want to show, - or a HTML fragment (see type='structure' below) - type: optional, defaults to 'info'. The type determines how - the message will be rendered, as it is used to select - the CSS class for the message. Predefined types are: - 'info' - for informational messages - 'warning' - for warning messages - 'error' - for messages about restricted access or - errors. - - Portal messages are by default rendered by the global_statusmessage.pt - page template. - - It is also possible to add messages from page templates, as - long as they are processed before the portal_message macro is - called by the main template. Example: - - # noqa - """ + # Call this once or more to add messages to be displayed at the + # top of the web page. + + # Note: no docstring please, to avoid reflected XSS. + # This might not be possible, but type="structure" below sounds dangerous, + # although I find no support for it in code. + + # The arguments are: + # message: a string, with the text message you want to show, + # or a HTML fragment (see type='structure' below) + # type: optional, defaults to 'info'. The type determines how + # the message will be rendered, as it is used to select + # the CSS class for the message. Predefined types are: + # 'info' - for informational messages + # 'warning' - for warning messages + # 'error' - for messages about restricted access or + # errors. + + # Portal messages are by default rendered by the global_statusmessage.pt + # page template. + + # It is also possible to add messages from page templates, as + # long as they are processed before the portal_message macro is + # called by the main template. Example: + + # # noqa if request is None: request = self.REQUEST IStatusMessage(request).add(message, type=type) @@ -592,42 +593,43 @@ def showPortalMessages(self, request=None): @security.public def browserDefault(self, obj): - """Sets default so we can return whatever we want instead of index_html. - - This method is complex, and interacts with mechanisms such as - IBrowserDefault (implemented in CMFDynamicViewFTI), LinguaPlone and - various mechanisms for setting the default page. - - The method returns a tuple (obj, [path]) where path is a path to - a template or other object to be acquired and displayed on the object. - The path is determined as follows: - - 0. If we're c oming from WebDAV, make sure we don't return a contained - object "default page" ever - 1. If there is an index_html attribute (either a contained object or - an explicit attribute) on the object, return that as the - "default page". Note that this may be used by things like - File and Image to return the contents of the file, for example, - not just content-space objects created by the user. - 2. If the object implements IBrowserDefault, query this for the - default page. - 3. If the object has a property default_page set and this gives a list - of, or single, object id, and that object is is found in the - folder or is the name of a skin template, return that id - 4. If the property default_page is set in site_properties and that - property contains a list of ids of which one id is found in the - folder, return that id - 5. If the object implements IBrowserDefault, try to get the selected - layout. - 6. If the type has a 'folderlisting' action and no default page is - set, use this action. This permits folders to have the default - 'view' action be 'string:${object_url}/' and hence default to - a default page when clicking the 'view' tab, whilst allowing the - fallback action to be specified TTW in portal_types (this action - is typically hidden) - 7. If nothing else is found, fall back on the object's 'view' action. - 8. If this is not found, raise an AttributeError - """ + # Sets default so we can return whatever we want instead of index_html. + + # Note: no docstring please, to avoid reflected XSS. + + # This method is complex, and interacts with mechanisms such as + # IBrowserDefault (implemented in CMFDynamicViewFTI), LinguaPlone and + # various mechanisms for setting the default page. + + # The method returns a tuple (obj, [path]) where path is a path to + # a template or other object to be acquired and displayed on the object. + # The path is determined as follows: + + # 0. If we're c oming from WebDAV, make sure we don't return a contained + # object "default page" ever + # 1. If there is an index_html attribute (either a contained object or + # an explicit attribute) on the object, return that as the + # "default page". Note that this may be used by things like + # File and Image to return the contents of the file, for example, + # not just content-space objects created by the user. + # 2. If the object implements IBrowserDefault, query this for the + # default page. + # 3. If the object has a property default_page set and this gives a list + # of, or single, object id, and that object is is found in the + # folder or is the name of a skin template, return that id + # 4. If the property default_page is set in site_properties and that + # property contains a list of ids of which one id is found in the + # folder, return that id + # 5. If the object implements IBrowserDefault, try to get the selected + # layout. + # 6. If the type has a 'folderlisting' action and no default page is + # set, use this action. This permits folders to have the default + # 'view' action be 'string:${object_url}/' and hence default to + # a default page when clicking the 'view' tab, whilst allowing the + # fallback action to be specified TTW in portal_types (this action + # is typically hidden) + # 7. If nothing else is found, fall back on the object's 'view' action. + # 8. If this is not found, raise an AttributeError # WebDAV in Zope is odd it takes the incoming verb eg: PROPFIND # and then requests that object, for example for: /, with verb PROPFIND @@ -789,7 +791,8 @@ def isLocalRoleAcquired(self, obj): @security.public def getOwnerName(self, obj): - """ Returns the userid of the owner of an object.""" + # Returns the userid of the owner of an object. + # Note: no docstring please, to avoid reflected XSS. mt = getToolByName(self, 'portal_membership') if not mt.checkPermission(View, obj): raise Unauthorized @@ -797,14 +800,14 @@ def getOwnerName(self, obj): @security.public def normalizeString(self, text): - """Normalizes a title to an id. + # Normalizes a title to an id. + # Note: no docstring please, to avoid reflected XSS. - The relaxed mode was removed in Plone 4.0. You should use either the - url or file name normalizer from the plone.i18n package instead. + # The relaxed mode was removed in Plone 4.0. You should use either the + # url or file name normalizer from the plone.i18n package instead. - normalizeString() converts a whole string to a normalized form that - should be safe to use as in a url, as a css id, etc. - """ + # normalizeString() converts a whole string to a normalized form that + # should be safe to use as in a url, as a css id, etc. return utils.normalizeString(text, context=self) @security.public @@ -963,7 +966,8 @@ def isIDAutoGenerated(self, id): @security.public def getEmptyTitle(self, translated=True): - """Returns string to be used for objects with no title or id.""" + # Returns string to be used for objects with no title or id. + # Note: no docstring please, to avoid reflected XSS. return utils.getEmptyTitle(self, translated) @security.public diff --git a/Products/CMFPlone/patches/publishing.py b/Products/CMFPlone/patches/publishing.py index 1bf50f6e62..598a6c38eb 100644 --- a/Products/CMFPlone/patches/publishing.py +++ b/Products/CMFPlone/patches/publishing.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # From Products.PloneHotfix20160419 # Plus extras for properties. +# Plus Products.PloneHotfix20210518. from OFS.PropertyManager import PropertyManager #from OFS.ZDOM import Document #from OFS.ZDOM import Node @@ -29,6 +30,25 @@ class ATCTBTreeFolder(object): pass +def delete_method_docstring(klass, method_name): + # Delete the docstring from the class method. + # Objects must have a docstring to be published. + # So this avoids them getting published. + method = getattr(klass, method_name, None) + if method is None: + return + if hasattr(method, "im_func"): + # Only Python 2 has im_func. + # Python 3 has __func__, but only on methods of instances, not classes. + if hasattr(method.im_func, "__doc__"): + del method.im_func.__doc__ + else: + # This would fail on Python 2 with an AttributeError: + # "attribute '__doc__' of 'instancemethod' objects is not writable" + if hasattr(method, "__doc__"): + del method.__doc__ + + klasses = ( # Node, # Document, @@ -61,10 +81,7 @@ class ATCTBTreeFolder(object): for klass in klasses: for method_name in methods: - method = getattr(klass, method_name, None) - if (method is not None and hasattr(method, 'im_func') and - hasattr(method.im_func, '__doc__')): - del method.im_func.__doc__ + delete_method_docstring(klass, method_name) property_methods = ( 'getProperty', @@ -79,7 +96,4 @@ class ATCTBTreeFolder(object): ) for method_name in property_methods: - method = getattr(PropertyManager, method_name, None) - if (method is not None and hasattr(method, 'im_func') and - hasattr(method.im_func, '__doc__')): - del method.im_func.__doc__ + delete_method_docstring(PropertyManager, method_name) diff --git a/Products/CMFPlone/tests/testSecurity.py b/Products/CMFPlone/tests/testSecurity.py index 45ae145396..6d8ae3008c 100644 --- a/Products/CMFPlone/tests/testSecurity.py +++ b/Products/CMFPlone/tests/testSecurity.py @@ -1,12 +1,19 @@ # -*- coding: utf-8 -*- -from Products.CMFPlone.tests.PloneTestCase import PloneTestCase -from Testing.makerequest import makerequest +from plone.app.testing import login +from plone.app.testing import logout from plone.app.testing import SITE_OWNER_NAME from plone.app.testing import SITE_OWNER_PASSWORD +from plone.testing.zope import Browser +from Products.CMFCore.utils import getToolByName +from Products.CMFPlone.testing import PRODUCTS_CMFPLONE_FUNCTIONAL_TESTING +from Products.CMFPlone.tests.PloneTestCase import PloneTestCase +from Testing.makerequest import makerequest +from zExceptions import NotFound from zExceptions import Unauthorized import re import six +import transaction import unittest @@ -202,3 +209,120 @@ def test_formatColumns(self): # formatColumns is unused and was removed res = self.publish('/plone/formatColumns?items:list=') self.assertIn(res.status, [403, 404]) + + +class TestFunctional(unittest.TestCase): + # The class above is rather old-style. + # Let's try a more modern approach, with a layer. + layer = PRODUCTS_CMFPLONE_FUNCTIONAL_TESTING + + def get_admin_browser(self): + browser = Browser(self.layer["app"]) + browser.handleErrors = False + browser.addHeader( + "Authorization", + "Basic {0}:{1}".format(SITE_OWNER_NAME, SITE_OWNER_PASSWORD), + ) + return browser + + def test_plonetool(self): + base_url = self.layer["portal"].absolute_url() + "/plone_utils" + browser = self.get_admin_browser() + method_names = ( + "addPortalMessage", + "browserDefault", + "getReviewStateTitleFor", + "portal_utf8", + "urlparse", + "urlunparse", + "utf8_portal", + "getOwnerName", + "normalizeString", + "getEmptyTitle", + ) + for method_name in method_names: + with self.assertRaises(NotFound): + browser.open(base_url + "/" + method_name) + + def test_hotfix_20160419(self): + """Test old hotfix. + + CMFPlone has patches/publishing.py, containing + the publishing patch from Products.PloneHotfix20160419. + This avoids publishing some methods inherited from Zope or CMF, + which upstream does not want to fix, considering it no problem + to have these methods available. I can imagine that. + But in Plone we have decided otherwise. + + Problem: the patch did not work on Python 3. + This was fixed in hotfix 20210518. + """ + portal = self.layer["portal"] + portal.invokeFactory("Document", "doc") + transaction.commit() + portal_url = portal.absolute_url() + doc_url = portal.doc.absolute_url() + browser = self.get_admin_browser() + method_names = ( + "EffectiveDate", + "ExpirationDate", + "getAttributes", + "getChildNodes", + "getFirstChild", + "getLastChild", + "getLayout", + "getNextSibling", + "getNodeName", + "getNodeType", + "getNodeValue", + "getOwnerDocument", + "getParentNode", + "getPhysicalPath", + "getPreviousSibling", + "getTagName", + "hasChildNodes", + "Type", + # From PropertyManager: + "getProperty", + "propertyValues", + "propertyItems", + "propertyMap", + "hasProperty", + "getPropertyType", + "propertyIds", + "propertyLabel", + "propertyDescription", + ) + for method_name in method_names: + with self.assertRaises(NotFound): + browser.open(portal_url + "/" + method_name) + with self.assertRaises(NotFound): + browser.open(doc_url + "/" + method_name) + + def test_quick_installer_security(self): + # Products.CMFQuickInstallerTool has a fix. + # But CMFPlone overrides the tool class, so let's check. + portal = self.layer["portal"] + qi = getToolByName(portal, "portal_quickinstaller", None) + if qi is None: + return + + # Make sure we are anonymous. + logout() + logout() + # Unrestricted traversal should work, restricted not. + qi = portal.unrestrictedTraverse("portal_quickinstaller") + with self.assertRaises(Unauthorized): + portal.restrictedTraverse("portal_quickinstaller") + for obj_id in qi.objectIds(): + qi.unrestrictedTraverse(obj_id) + with self.assertRaises(Unauthorized): + qi.restrictedTraverse(obj_id) + + # Authenticated with role Manager, we can view whatever we want. + login(portal, SITE_OWNER_NAME) + qi = portal.unrestrictedTraverse("portal_quickinstaller") + qi = portal.restrictedTraverse("portal_quickinstaller") + for obj_id in qi.objectIds(): + qi.unrestrictedTraverse(obj_id) + qi.restrictedTraverse(obj_id) diff --git a/news/3274.bugfix b/news/3274.bugfix new file mode 100644 index 0000000000..7d9f538d12 --- /dev/null +++ b/news/3274.bugfix @@ -0,0 +1,3 @@ +Removed the docstring from various methods to avoid making them available via a url. +From `Products.PloneHotfix20210518 `_. +[maurits]