Skip to content

Commit

Permalink
various security fixes based on PloneHotfix20121106
Browse files Browse the repository at this point in the history
  • Loading branch information
davisagli committed Nov 26, 2012
1 parent 24cad2d commit c3a98f4
Show file tree
Hide file tree
Showing 11 changed files with 293 additions and 3 deletions.
1 change: 1 addition & 0 deletions Products/CMFPlone/PloneControlPanel.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ def addAction(self,
return self.manage_editActionsForm(
REQUEST, manage_tabs_message='Added.')

security.declareProtected(ManagePortal, 'registerConfiglet')
registerConfiglet = addAction

security.declareProtected(ManagePortal, 'manage_editActionsForm')
Expand Down
2 changes: 2 additions & 0 deletions Products/CMFPlone/PloneTool.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from zope.lifecycleevent import ObjectModifiedEvent

from AccessControl import ClassSecurityInfo, Unauthorized
from AccessControl import getSecurityManager
from Acquisition import aq_base
from Acquisition import aq_inner
from Acquisition import aq_parent
Expand Down Expand Up @@ -1301,6 +1302,7 @@ def renameObjectsByPaths(self, paths, new_ids, new_titles,
change_title = new_title and title != new_title
changed = False
if change_title:
getSecurityManager().validate(obj, obj, 'setTitle', obj.setTitle)
obj.setTitle(new_title)
notify(ObjectModifiedEvent(obj))
changed = True
Expand Down
2 changes: 2 additions & 0 deletions Products/CMFPlone/patches/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,7 @@
import iso8601 # use `DateTime.ISO8601` for `DateTime.ISO`
iso8601.applyPatches()

import security # misc security fixes

import sendmail
sendmail.applyPatches()
42 changes: 42 additions & 0 deletions Products/CMFPlone/patches/security.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# 1. make sure allow_module can't be called from restricted code
import AccessControl
AccessControl.allow_module.__roles__ = ()

# 2. make sure /@@ doesn't traverse to annotations
from zope.traversing import namespace
from zope.traversing.interfaces import TraversalError
old_traverse = namespace.view.traverse
def traverse(self, name, ignored):
if not name:
raise TraversalError(self.context, name)
return old_traverse(self, name, ignored)
namespace.view.traverse = traverse

# 3. be sure to check Access contents information permission for FTP users
from AccessControl import getSecurityManager
from zExceptions import Unauthorized
from OFS.ObjectManager import ObjectManager
ObjectManager.__old_manage_FTPlist = ObjectManager.manage_FTPlist
def manage_FTPlist(self, REQUEST):
"""Returns a directory listing consisting of a tuple of
(id,stat) tuples, marshaled to a string. Note, the listing it
should include '..' if there is a Folder above the current
one.
In the case of non-foldoid objects it should return a single
tuple (id,stat) representing itself."""

if not getSecurityManager().checkPermission('Access contents information', self):
raise Unauthorized('Not allowed to access contents.')

return self.__old_manage_FTPlist(REQUEST)
ObjectManager.manage_FTPlist = manage_FTPlist

# 4. Make sure z3c.form widgets don't get declared as public
from Products.Five.metaconfigure import ClassDirective
old_require = ClassDirective.require
def require(self, *args, **kw):
if self._ClassDirective__class.__module__.startswith('z3c.form.browser'):
return
return old_require(self, *args, **kw)
ClassDirective.require = require
4 changes: 3 additions & 1 deletion Products/CMFPlone/skins/plone_scripts/createObject.cpy
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ response.setHeader('Expires', 'Sat, 01 Jan 2000 00:00:00 GMT')
response.setHeader('Cache-Control', 'no-cache')

if id is None:
id=context.generateUniqueId(type_name)
id = context.generateUniqueId(type_name)
else:
id = id.replace('$', '$$')

if type_name is None:
raise Exception, 'Type name not specified'
Expand Down
4 changes: 4 additions & 0 deletions Products/CMFPlone/skins/plone_scripts/formatColumns.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@

# returns a list of lists of items

from zExceptions import Forbidden
if container.REQUEST.get('PUBLISHED') is script:
raise Forbidden('Script may not be published.')

rows = []

i = 0
Expand Down
4 changes: 4 additions & 0 deletions Products/CMFPlone/skins/plone_scripts/getFolderContents.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
# NOTE: This script is obsolete, use the browser view
# @@folderListing in plone.app.contentlisting

from zExceptions import Forbidden
if container.REQUEST.get('PUBLISHED') is script:
raise Forbidden('Script may not be published.')

mtool = context.portal_membership
cur_path = '/'.join(context.getPhysicalPath())
path = {}
Expand Down
7 changes: 6 additions & 1 deletion Products/CMFPlone/skins/plone_scripts/translate.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@
##bind subpath=traverse_subpath
##parameters=msgid, mapping={}, default=None, domain='plone', target_language=None, escape_for_js=False

# handle the possible "nothing" condition in folder_contents.pt ln 21 gracefully
from zExceptions import Forbidden
if container.REQUEST.get('PUBLISHED') is script:
raise Forbidden('Script may not be published.')

# handle the possible "nothing" condition in folder_contents.pt ln 21
# gracefully
if msgid == None:
return None

Expand Down
7 changes: 6 additions & 1 deletion Products/CMFPlone/skins/plone_scripts/utranslate.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@
##bind subpath=traverse_subpath
##parameters=msgid, mapping={}, default=None, domain='plone', target_language=None, escape_for_js=False

# handle the possible "nothing" condition in folder_contents.pt ln 21 gracefully
from zExceptions import Forbidden
if container.REQUEST.get('PUBLISHED') is script:
raise Forbidden('Script may not be published.')

# handle the possible "nothing" condition in folder_contents.pt ln 21
# gracefully
if msgid == None:
return None

Expand Down
220 changes: 220 additions & 0 deletions Products/CMFPlone/tests/testSecurity.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,220 @@
import re
import unittest
from urllib import urlencode
from Testing.makerequest import makerequest
from Products.PloneTestCase import PloneTestCase as ptc
from Products.Five.testbrowser import Browser
from zExceptions import Unauthorized

ptc.setupPloneSite()


class TestAttackVectorsUnit(unittest.TestCase):

def test_gtbn_funcglobals(self):
from Products.CMFPlone.utils import getToolByName
try:
getToolByName(self.assertTrue,'func_globals')['__builtins__']
except TypeError:
pass
else:
self.fail('getToolByName should block access to non CMF tools')

def test_setHeader_drops_LF(self):
from ZPublisher.HTTPResponse import HTTPResponse
response = HTTPResponse()
response.setHeader('Location',
'http://www.ietf.org/rfc/\nrfc2616.txt')
self.assertEqual(response.headers['location'],
'http://www.ietf.org/rfc/rfc2616.txt')

def test_PT_allow_module_not_available_in_RestrictedPython_1(self):
src = '''
from AccessControl import Unauthorized
try:
import Products.PlacelessTranslationService
except (ImportError, Unauthorized):
raise AssertionError("Failed to import Products.PTS")
Products.PlacelessTranslationService.allow_module('os')
'''
from Products.PythonScripts.PythonScript import PythonScript
script = makerequest(PythonScript('script'))
script._filepath = 'script'
script.write(src)
self.assertRaises((ImportError, Unauthorized), script)

def test_PT_allow_module_not_available_in_RestrictedPython_2(self):
src = '''
from Products.PlacelessTranslationService import allow_module
allow_module('os')
'''
from Products.PythonScripts.PythonScript import PythonScript
script = makerequest(PythonScript('script'))
script._filepath = 'script'
script.write(src)
self.assertRaises((ImportError, Unauthorized), script)

def test_get_request_var_or_attr_disallowed(self):
import App.Undo
self.assertFalse(hasattr(App.Undo.UndoSupport, 'get_request_var_or_attr'))


class TestAttackVectorsFunctional(ptc.FunctionalTestCase):

def test_widget_traversal_1(self):
res = self.publish('/plone/@@discussion-settings/++widget++moderator_email')
self.assertEqual(302, res.status)
self.assertTrue(res.headers['location'].startswith('http://nohost/plone/acl_users/credentials_cookie_auth/require_login'))

def test_widget_traversal_2(self):
res = self.publish('/plone/@@discussion-settings/++widget++captcha/terms/field/interface/setTaggedValue?tag=cake&value=lovely')
self.assertEqual(302, res.status)
self.assertTrue(res.headers['location'].startswith('http://nohost/plone/acl_users/credentials_cookie_auth/require_login'))

def test_registerConfiglet_1(self):
VECTOR = "/plone/portal_controlpanel/registerConfiglet?id=cake&name=Cakey&action=woo&permission=View&icon_expr="
res = self.publish(VECTOR)
self.assertTrue(res.headers['location'].startswith('http://nohost/plone/acl_users/credentials_cookie_auth/require_login'))

def test_registerConfiglet_2(self):
VECTOR = "/plone/portal_controlpanel/registerConfiglet?id=cake&name=Cakey&action=woo&permission=View&icon_expr="
self.publish(VECTOR)
action_ids = [action.id for action in self.portal.portal_controlpanel._actions]
self.assertTrue('cake' not in action_ids)

def _get_authenticator(self, basic=None):
url = '/plone/#_password'
res = self.publish(url, basic=basic)
m = re.search('name="_authenticator" value="([^"]*)"', res.body)
if m:
return m.group(1)
return ''

def test_renameObjectsByPaths(self):
PAYLOAD = {
'paths:list': '/plone/news',
# id must stay the same
'new_ids:list': 'news',
'new_titles:list': 'EVIL',
# Set orig_template to 'view'. Otherwise folder_rename "success" redirects
# to folder_contents, which will raise Unauthorized.
'orig_template': 'view',
}

browser = Browser()
csrf_token = self._get_authenticator()

PAYLOAD['_authenticator'] = csrf_token
# Call folder_rename anywhere
browser.open('http://nohost/plone/folder_rename',
urlencode(PAYLOAD))
self.assertTrue('The following item(s) could not be renamed: /plone/news.' in browser.contents)
self.assertEqual('News', self.portal.news.Title())

def test_renameObjectByPaths_postonly(self):
from Products.PythonScripts.PythonScript import PythonScript
script = PythonScript('script')
script._filepath = 'script'
src = """context.plone_utils.renameObjectsByPaths(paths=['/plone/news'], new_ids=['news'], new_titles=['EVIL'], REQUEST=context.REQUEST)"""
script.write(src)
self.portal.evil = script
csrf_token = self._get_authenticator()

self.publish('/plone/evil', extra={'_authenticator': csrf_token}, request_method='POST')
self.assertEqual('News', self.portal.news.Title())

owner_basic = ptc.portal_owner + ':' + ptc.default_password
csrf_token = self._get_authenticator(owner_basic)
self.publish('/plone/evil', extra={'_authenticator': csrf_token}, basic=owner_basic)
self.assertEqual('News', self.portal.news.Title())
self.publish('/plone/evil', request_method='POST', extra={'_authenticator': csrf_token}, basic=owner_basic)
self.assertEqual('EVIL', self.portal.news.Title())

self.setRoles(['Manager'])
self.portal.news.setTitle('News')
self.portal.plone_utils.renameObjectsByPaths(paths=['/plone/news'], new_ids=['news'], new_titles=['EVIL'])
self.assertEqual('EVIL', self.portal.news.Title())
self.portal.news.setTitle('News')

self.setRoles(['Member'])
self.portal.plone_utils.renameObjectsByPaths(paths=['/plone/news'], new_ids=['news'], new_titles=['EVIL'])
self.assertEqual('News', self.portal.news.Title())

def test_gtbn_faux_archetypes_tool(self):
from Products.CMFPlone.FactoryTool import FauxArchetypeTool
from Products.CMFPlone.utils import getToolByName
self.portal.portal_factory.archetype_tool = FauxArchetypeTool(self.portal.archetype_tool)
self.assertEqual(self.portal.portal_factory.archetype_tool, getToolByName(self.portal.portal_factory, 'archetype_tool'))

def test_searchForMembers(self):
res = self.publish('/plone/portal_membership/searchForMembers')
self.assertEqual(302, res.status)
self.assertTrue(res.headers['location'].startswith('http://nohost/plone/acl_users/credentials_cookie_auth/require_login'))

def test_getMemberInfo(self):
res = self.publish('/plone/portal_membership/getMemberInfo?id=admin')
self.assertEqual(404, res.status)

def test_queryCatalog(self):
res = self.publish('/plone/news/aggregator/queryCatalog')
self.assertEqual(404, res.status)

def test_resolve_url(self):
res = self.publish("/plone/uid_catalog/resolve_url?path=/evil")
self.assertEqual(302, res.status)
self.assertTrue(res.headers['location'].startswith('http://nohost/plone/acl_users/credentials_cookie_auth/require_login'))

def test_at_download(self):
self.setRoles(['Manager'])
self.portal.portal_workflow.setChainForPortalTypes(['File'], 'plone_workflow')
self.portal.invokeFactory('File', 'test')
self.portal.portal_workflow.doActionFor(self.portal.test, 'publish')

# give it a more restricted read_permission
self.portal.test.Schema()['file'].read_permission = 'Manage portal'

# make sure at_download disallows even though the user has View permission
res = self.publish('/plone/test/at_download/file')
self.assertEqual(res.status, 302)
self.assertTrue(res.headers['location'].startswith('http://nohost/plone/acl_users/credentials_cookie_auth/require_login'))

def test_ftp(self):
self.setRoles(['Manager', 'Owner'])
self.portal.REQUEST.PARENTS = [self.app]
res = self.portal.news.manage_FTPlist(self.portal.REQUEST)
self.assertTrue(isinstance(res, basestring))
self.portal.portal_workflow.doActionFor(self.portal.news, 'hide')
self.setRoles(['Member'])
from zExceptions import Unauthorized
self.assertRaises(Unauthorized, self.portal.news.manage_FTPlist, self.portal.REQUEST)

def test_atat_does_not_return_anything(self):
res = self.publish('/plone/@@')
self.assertEqual(404, res.status)

def test_go_back(self):
res = self.publish('/plone/front-page/go_back?last_referer=http://${request}',
basic=ptc.portal_owner + ':' + ptc.default_password)
self.assertEqual(302, res.status)
self.assertEqual('http://${request}', res.headers['location'][:17])

def test_getFolderContents(self):
res = self.publish('/plone/getFolderContents')
self.assertEqual(403, res.status)

def test_translate(self):
res = self.publish('/plone/translate?msgid=foo')
self.assertEqual(403, res.status)

def test_utranslate(self):
res = self.publish('/plone/utranslate?msgid=foo')
self.assertEqual(403, res.status)

def test_createObject(self):
res = self.publish('/plone/createObject?type_name=File&id=${foo}')
self.assertEqual(302, res.status)
self.assertEqual('http://nohost/plone/portal_factory/File/${foo}/edit', res.headers['location'])

def test_formatColumns(self):
res = self.publish('/plone/formatColumns?items:list=')
self.assertEqual(403, res.status)
3 changes: 3 additions & 0 deletions docs/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ Changelog
4.2.3 (unreleased)
-----------------

- Add various security fixes based on PloneHotfix20121106.
[davisagli]

- Pass minute_step to date_components_support_view.result(). See
https://dev.plone.org/ticket/11251
[gbastien]
Expand Down

0 comments on commit c3a98f4

Please # to comment.