Skip to content

Commit

Permalink
Test that str.format checks the security for attributes that are ac…
Browse files Browse the repository at this point in the history
…cessed.

Part of PloneHotfix20170117.

This needs zopefoundation/AccessControl#23
This was merged, but not released yet, so we add AccessControl to auto-checkout for now.
  • Loading branch information
mauritsvanrees committed Feb 1, 2017
1 parent ecfd7b1 commit 89c2040
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 0 deletions.
1 change: 1 addition & 0 deletions buildout.cfg
Expand Up @@ -22,6 +22,7 @@ parts =
wsgi
sources-dir = develop
auto-checkout =
AccessControl
versions = versions


Expand Down
3 changes: 3 additions & 0 deletions doc/CHANGES.rst
Expand Up @@ -8,6 +8,9 @@ http://docs.zope.org/zope2/
2.13.26 (unreleased)
--------------------

- In ``str.format``, check the security for attributes that are accessed.
Part of PloneHotfix20170117. [maurits]

- Fixed reflective XSS in findResult.
This applies PloneHotfix20170117. [maurits]

Expand Down
2 changes: 2 additions & 0 deletions src/Zope2/App/tests/normal_zope3_page_template.pt
@@ -0,0 +1,2 @@
<p tal:content="python:('%s' % context).lower()" />
<p tal:content="python:(u'%s' % context).upper()" />
95 changes: 95 additions & 0 deletions src/Zope2/App/tests/test_formatter.py
@@ -0,0 +1,95 @@
from Testing.ZopeTestCase import FunctionalTestCase
from zExceptions import Unauthorized

import unittest


BAD_STR = """
<p tal:content="python:'class of {0} is {0.__class__}'.format(context)" />
"""
BAD_UNICODE = """
<p tal:content="python:u'class of {0} is {0.__class__}'.format(context)" />
"""
GOOD_STR = '<p tal:content="python:(\'%s\' % context).lower()" />'
GOOD_UNICODE = '<p tal:content="python:(\'%s\' % context).lower()" />'


def noop(context=None):
return lambda: context


def hack_pt(pt, context=None):
# hacks to avoid getting error in pt_render.
pt.getPhysicalRoot = noop()
pt._getContext = noop(context)
pt._getContainer = noop(context)
pt.context = context


class FormatterTest(unittest.TestCase):

def test_cook_zope2_page_templates_bad_str(self):
from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
pt = ZopePageTemplate('mytemplate', BAD_STR)
hack_pt(pt)
self.assertRaises(Unauthorized, pt.pt_render)

def test_cook_zope2_page_templates_bad_unicode(self):
from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
pt = ZopePageTemplate('mytemplate', BAD_UNICODE)
hack_pt(pt)
self.assertRaises(Unauthorized, pt.pt_render)

def test_cook_zope2_page_templates_good_str(self):
from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
pt = ZopePageTemplate('mytemplate', GOOD_STR)
hack_pt(pt)
self.assertEqual(pt.pt_render().strip(), '<p>none</p>')

def test_cook_zope2_page_templates_good_unicode(self):
from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
pt = ZopePageTemplate('mytemplate', unicode(GOOD_UNICODE))
hack_pt(pt)
self.assertEqual(pt.pt_render().strip(), '<p>none</p>')


class FormatterFunctionalTest(FunctionalTestCase):

def test_access_to_private_content_not_allowed_via_any_attribute(self):
from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
# If access to _delObject would be allowed, it would still only say
# something like 'bound method _delObject', without actually deleting
# anything, because methods are not executed in str.format, but there
# may be @properties that give an attacker secret info.
pt = ZopePageTemplate('mytemplate', '''
<p tal:content="structure python:'access {0._delObject}'.format(context)" />
''')
hack_pt(pt, context=self.app)
self.assertRaises(Unauthorized, pt.pt_render)

# Zope 3 templates are always file system templates. So we actually have
# no problems allowing str.format there.

def test_cook_zope3_page_templates_normal(self):
from Products.Five.browser.pagetemplatefile import ViewPageTemplateFile
pt = ViewPageTemplateFile('normal_zope3_page_template.pt')
hack_pt(pt)
# Need to pass a namespace.
namespace = {'context': self.app}
self.assertEqual(
pt.pt_render(namespace).strip(),
u'<p>&lt;application at &gt;</p>\n'
u'<p>&lt;APPLICATION AT &gt;</p>')

def test_cook_zope3_page_templates_using_format(self):
from Products.Five.browser.pagetemplatefile import ViewPageTemplateFile
pt = ViewPageTemplateFile('using_format_zope3_page_template.pt')
hack_pt(pt)
# Need to pass a namespace.
namespace = {'context': self.app}
self.assertEqual(
pt.pt_render(namespace).strip(),
u"<p>class of &lt;application at &gt; is "
u"&lt;class 'ofs.application.application'&gt;</p>\n"
u"<p>CLASS OF &lt;APPLICATION AT &gt; IS "
u"&lt;CLASS 'OFS.APPLICATION.APPLICATION'&gt;</p>")
2 changes: 2 additions & 0 deletions src/Zope2/App/tests/using_format_zope3_page_template.pt
@@ -0,0 +1,2 @@
<p tal:content="python:'class of {0} is {0.__class__}'.format(context).lower()" />
<p tal:content="python:u'class of {0} is {0.__class__}'.format(context).upper()" />

0 comments on commit 89c2040

Please sign in to comment.