Possible Zope 2.12 regression - Five page templates use restrictedTraverse for TAL
Hi, Ages ago, I started a thread (I think on this list) about the use of TAL expression in Zope 3-style page templates (i.e. ViewPageTemplateFile's used on views) incorrectly performing security checks when using TAL expressions. I think Tres fixed it at the time (I can't find the original mail), but in Zope 2.12.2 there seems to have been a regression. I'd like to make sure I've not missed something, though. I see the problem in Plone 4.0a2 on Zope 2.12, trying to use plone.app.registry. This package has a class that derives from plone.registry.registry.Registry (a "ZTK-only" version) and SimpleItem (to get Zope 2 security and ZMI visibility). Otherwise, it does nothing. There is a view for this class registered like this: <browser:page name="view" for="plone.registry.interfaces.IRegistry" template="records.pt" permission="cmf.ManagePortal" /> In records.pt, there is the following: <tr tal:repeat="record context/records/values"> ... </tr> This fails with the stack trace below. However, if I change it to use a python: expression, it's all fine: <tr tal:repeat="record python:context.records.values()"> ... </tr> As you can see from the stack trace, the TAL expression has resulted in the use of restrictedTraverse(). I think it should be using unrestrictedTraverse(), since ViewPageTemplateFile's are filesystem code. (In this particular case, it's failing the restrictedTraverse check because 'records' is a property and so doesn't have an aq chain, but nevermind that). Stack trace: 2009-12-13 00:40:13 ERROR Zope.SiteErrorLog 1260636013.580.4115611559 http://localhost:8080/test/portal_registry/@@view Traceback (innermost last): Module ZPublisher.Publish, line 127, in publish Module ZPublisher.mapply, line 77, in mapply Module Products.PDBDebugMode.runcall, line 70, in pdb_runcall Module ZPublisher.Publish, line 47, in call_object Module Products.Five.browser.metaconfigure, line 427, in __call__ Module Products.Five.browser.pagetemplatefile, line 126, in __call__ Module Products.Five.browser.pagetemplatefile, line 60, in __call__ Module zope.pagetemplate.pagetemplate, line 115, in pt_render Module zope.tal.talinterpreter, line 271, in __call__ Module zope.tal.talinterpreter, line 343, in interpret Module zope.tal.talinterpreter, line 888, in do_useMacro Module zope.tal.talinterpreter, line 343, in interpret Module zope.tal.talinterpreter, line 533, in do_optTag_tal Module zope.tal.talinterpreter, line 518, in do_optTag Module zope.tal.talinterpreter, line 513, in no_tag Module zope.tal.talinterpreter, line 343, in interpret Module zope.tal.talinterpreter, line 888, in do_useMacro Module zope.tal.talinterpreter, line 343, in interpret Module zope.tal.talinterpreter, line 533, in do_optTag_tal Module zope.tal.talinterpreter, line 518, in do_optTag Module zope.tal.talinterpreter, line 513, in no_tag Module zope.tal.talinterpreter, line 343, in interpret Module zope.tal.talinterpreter, line 946, in do_defineSlot Module zope.tal.talinterpreter, line 343, in interpret Module zope.tal.talinterpreter, line 533, in do_optTag_tal Module zope.tal.talinterpreter, line 518, in do_optTag Module zope.tal.talinterpreter, line 513, in no_tag Module zope.tal.talinterpreter, line 343, in interpret Module zope.tal.talinterpreter, line 954, in do_defineSlot Module zope.tal.talinterpreter, line 343, in interpret Module zope.tal.talinterpreter, line 533, in do_optTag_tal Module zope.tal.talinterpreter, line 518, in do_optTag Module zope.tal.talinterpreter, line 513, in no_tag Module zope.tal.talinterpreter, line 343, in interpret Module zope.tal.talinterpreter, line 946, in do_defineSlot Module zope.tal.talinterpreter, line 343, in interpret Module zope.tal.talinterpreter, line 819, in do_loop_tal Module zope.tales.tales, line 682, in setRepeat Module zope.tales.tales, line 696, in evaluate - URL: /Users/optilude/Development/Plone/Instances/Clients/Lotterywest/osc/src/plone.app.registry/plone/app/registry/browser/records.pt - Line 33, Column 12 - Expression: <PathExpr standard:u'context/records/values'> - Names: {'args': (), 'container': <Registry at /test/portal_registry>, 'context': <Registry at /test/portal_registry>, 'default': <object object at 0x1002c7b50>, 'here': <Registry at /test/portal_registry>, 'loop': {}, 'nothing': None, 'options': {}, 'repeat': <Products.PageTemplates.Expressions.SafeMapping object at 0x108e8b3b0>, 'request': <HTTPRequest, URL=http://localhost:8080/test/portal_registry/@@view>, 'root': <Application at >, 'template': <Products.Five.browser.pagetemplatefile.ViewPageTemplateFile object at 0x10792b950>, 'traverse_subpath': [], 'user': <PropertiedUser 'admin'>, 'view': <Products.Five.metaclass.SimpleViewClass from /Users/optilude/Development/Plone/Instances/Clients/Lotterywest/osc/src/plone.app.registry/plone/app/registry/browser/records.pt object at 0x10885c810>, 'views': <Products.Five.browser.pagetemplatefile.ViewMapper object at 0x108cbb0d0>} Module zope.tales.expressions, line 217, in __call__ Module Products.PageTemplates.Expressions, line 127, in _eval Module zope.tales.expressions, line 124, in _eval Module Products.PageTemplates.Expressions, line 76, in boboAwareZopeTraverse Module OFS.Traversable, line 312, in restrictedTraverse Module OFS.Traversable, line 247, in unrestrictedTraverse - __traceback_info__: ([], 'records') Unauthorized: You are not allowed to access 'records' in this context
/Users/optilude/.buildout/eggs/Zope2-2.12.1-py2.6-macosx-10.6-i386.egg/OFS/Traversable.py(247)unrestrictedTraverse() Martin -- Author of `Professional Plone Development`, a book for developers who want to work with Plone. See http://martinaspeli.net/plone-book
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Martin Aspeli wrote:
Hi,
Ages ago, I started a thread (I think on this list) about the use of TAL expression in Zope 3-style page templates (i.e. ViewPageTemplateFile's used on views) incorrectly performing security checks when using TAL expressions.
I think Tres fixed it at the time (I can't find the original mail), but in Zope 2.12.2 there seems to have been a regression. I'd like to make sure I've not missed something, though.
I see the problem in Plone 4.0a2 on Zope 2.12, trying to use plone.app.registry. This package has a class that derives from plone.registry.registry.Registry (a "ZTK-only" version) and SimpleItem (to get Zope 2 security and ZMI visibility). Otherwise, it does nothing.
There is a view for this class registered like this:
<browser:page name="view" for="plone.registry.interfaces.IRegistry" template="records.pt" permission="cmf.ManagePortal" />
In records.pt, there is the following:
<tr tal:repeat="record context/records/values"> ... </tr>
This fails with the stack trace below. However, if I change it to use a python: expression, it's all fine:
<tr tal:repeat="record python:context.records.values()"> ... </tr>
As you can see from the stack trace, the TAL expression has resulted in the use of restrictedTraverse(). I think it should be using unrestrictedTraverse(), since ViewPageTemplateFile's are filesystem code.
(In this particular case, it's failing the restrictedTraverse check because 'records' is a property and so doesn't have an aq chain, but nevermind that).
Stack trace:
2009-12-13 00:40:13 ERROR Zope.SiteErrorLog 1260636013.580.4115611559 http://localhost:8080/test/portal_registry/@@view Traceback (innermost last): Module ZPublisher.Publish, line 127, in publish Module ZPublisher.mapply, line 77, in mapply Module Products.PDBDebugMode.runcall, line 70, in pdb_runcall Module ZPublisher.Publish, line 47, in call_object Module Products.Five.browser.metaconfigure, line 427, in __call__ Module Products.Five.browser.pagetemplatefile, line 126, in __call__ Module Products.Five.browser.pagetemplatefile, line 60, in __call__ Module zope.pagetemplate.pagetemplate, line 115, in pt_render Module zope.tal.talinterpreter, line 271, in __call__ Module zope.tal.talinterpreter, line 343, in interpret Module zope.tal.talinterpreter, line 888, in do_useMacro Module zope.tal.talinterpreter, line 343, in interpret Module zope.tal.talinterpreter, line 533, in do_optTag_tal Module zope.tal.talinterpreter, line 518, in do_optTag Module zope.tal.talinterpreter, line 513, in no_tag Module zope.tal.talinterpreter, line 343, in interpret Module zope.tal.talinterpreter, line 888, in do_useMacro Module zope.tal.talinterpreter, line 343, in interpret Module zope.tal.talinterpreter, line 533, in do_optTag_tal Module zope.tal.talinterpreter, line 518, in do_optTag Module zope.tal.talinterpreter, line 513, in no_tag Module zope.tal.talinterpreter, line 343, in interpret Module zope.tal.talinterpreter, line 946, in do_defineSlot Module zope.tal.talinterpreter, line 343, in interpret Module zope.tal.talinterpreter, line 533, in do_optTag_tal Module zope.tal.talinterpreter, line 518, in do_optTag Module zope.tal.talinterpreter, line 513, in no_tag Module zope.tal.talinterpreter, line 343, in interpret Module zope.tal.talinterpreter, line 954, in do_defineSlot Module zope.tal.talinterpreter, line 343, in interpret Module zope.tal.talinterpreter, line 533, in do_optTag_tal Module zope.tal.talinterpreter, line 518, in do_optTag Module zope.tal.talinterpreter, line 513, in no_tag Module zope.tal.talinterpreter, line 343, in interpret Module zope.tal.talinterpreter, line 946, in do_defineSlot Module zope.tal.talinterpreter, line 343, in interpret Module zope.tal.talinterpreter, line 819, in do_loop_tal Module zope.tales.tales, line 682, in setRepeat Module zope.tales.tales, line 696, in evaluate - URL: /Users/optilude/Development/Plone/Instances/Clients/Lotterywest/osc/src/plone.app.registry/plone/app/registry/browser/records.pt - Line 33, Column 12 - Expression: <PathExpr standard:u'context/records/values'> - Names: {'args': (), 'container': <Registry at /test/portal_registry>, 'context': <Registry at /test/portal_registry>, 'default': <object object at 0x1002c7b50>, 'here': <Registry at /test/portal_registry>, 'loop': {}, 'nothing': None, 'options': {}, 'repeat': <Products.PageTemplates.Expressions.SafeMapping object at 0x108e8b3b0>, 'request': <HTTPRequest, URL=http://localhost:8080/test/portal_registry/@@view>, 'root': <Application at >, 'template': <Products.Five.browser.pagetemplatefile.ViewPageTemplateFile object at 0x10792b950>, 'traverse_subpath': [], 'user': <PropertiedUser 'admin'>, 'view': <Products.Five.metaclass.SimpleViewClass from /Users/optilude/Development/Plone/Instances/Clients/Lotterywest/osc/src/plone.app.registry/plone/app/registry/browser/records.pt object at 0x10885c810>, 'views': <Products.Five.browser.pagetemplatefile.ViewMapper object at 0x108cbb0d0>} Module zope.tales.expressions, line 217, in __call__ Module Products.PageTemplates.Expressions, line 127, in _eval Module zope.tales.expressions, line 124, in _eval Module Products.PageTemplates.Expressions, line 76, in boboAwareZopeTraverse Module OFS.Traversable, line 312, in restrictedTraverse Module OFS.Traversable, line 247, in unrestrictedTraverse - __traceback_info__: ([], 'records') Unauthorized: You are not allowed to access 'records' in this context
/Users/optilude/.buildout/eggs/Zope2-2.12.1-py2.6-macosx-10.6-i386.egg/OFS/Traversable.py(247)unrestrictedTraverse()
Doesn't smell like a regression to me: the code there hasn't changed in a good long while. Can you write a test case for it, so that we can test against earlier versions? Tres. - -- =================================================================== Tres Seaver +1 540-429-0999 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkskVvMACgkQ+gerLs4ltQ5yCQCgtBGdNpp8FF2W0N12oAwUGuIN QnEAni6zkD2mLHEGQ3UXKq6vLUBAOUS0 =V1cV -----END PGP SIGNATURE-----
On 13/12/09 10:52, Tres Seaver wrote:
Doesn't smell like a regression to me: the code there hasn't changed in a good long while. Can you write a test case for it, so that we can test against earlier versions?
I'm almost completely sure that this was an issue ages ago, and slightly less sure that you were the one to fix it in the end. :) Google is failing me in trying to dig up the original mail or changeset, though. I'm struggling to understand the interplay between Products.Five.browser.pagetemplatefile, zope.pagetemplate, zope.tal, zope.tales, Products.PageTemplates, and OFS.Traversable, but I'm trying to find a solution. I'll try to get a working test into Zope 2.12 too. Martin -- Author of `Professional Plone Development`, a book for developers who want to work with Plone. See http://martinaspeli.net/plone-book
On 13/12/09 10:52, Tres Seaver wrote:
Doesn't smell like a regression to me: the code there hasn't changed in a good long while. Can you write a test case for it, so that we can test against earlier versions?
Aha! http://codespeak.net/pipermail/z3-five/2007q2/002185.html This is the same problem. You said: "This is becuase 'Products.PageTemplates.Expression.createTrustedZopeEngine' only trusts 'python:' expressions; path traversal is still governed by 'boboAwareZopeTraverse', which uses 'restrictedTraverse'." and then: "As it turns out, it is only "partially trusted." The attached patch should make them "really trusted", at least for path expressions; does it help? I haven't added any tests, although my 2.10 branch checkout does pass all tests with this change" The attachment is here: http://codespeak.net/pipermail/z3-five/attachments/20070506/7f8a9ea8/attachm... I'm going to poke around a Zope 2.12 checkout for a bit to see what sense I can make of this. Martin -- Author of `Professional Plone Development`, a book for developers who want to work with Plone. See http://martinaspeli.net/plone-book
On 13/12/09 16:49, Martin Aspeli wrote:
On 13/12/09 10:52, Tres Seaver wrote:
Doesn't smell like a regression to me: the code there hasn't changed in a good long while. Can you write a test case for it, so that we can test against earlier versions?
Aha! http://codespeak.net/pipermail/z3-five/2007q2/002185.html
This is the same problem.
You said:
"This is becuase 'Products.PageTemplates.Expression.createTrustedZopeEngine' only trusts 'python:' expressions; path traversal is still governed by 'boboAwareZopeTraverse', which uses 'restrictedTraverse'."
and then:
"As it turns out, it is only "partially trusted." The attached patch should make them "really trusted", at least for path expressions; does it help? I haven't added any tests, although my 2.10 branch checkout does pass all tests with this change"
The attachment is here:
http://codespeak.net/pipermail/z3-five/attachments/20070506/7f8a9ea8/attachm...
I'm going to poke around a Zope 2.12 checkout for a bit to see what sense I can make of this.
Okay, so it turns out your patch has gotten lost from Zope 2.10 to Zope 2.12. This is the revision where it went in: http://zope3.pov.lt/trac/changeset/77064/Zope/branches/2.10/lib/python/Produ... I think that by accident this got committed with an unrelated change, since the commit message says "Use Five 1.5.5" and there's a change in svn:externals. Perhaps that's why this wasn't merged to trunk. The latest merge I can see is at r71802. This also makes me worry about http://zope3.pov.lt/trac/browser/Zope/branches/2.10/lib/python/Products/Page... and http://zope3.pov.lt/trac/browser/Zope/branches/2.10/lib/python/Products/Page..., which may not have been merged, but I'm too far down the rabbit hole now to see clearly. Anyway, I re-applied your patch to the Zope 2.12 branch. This broke one test, in Products.Five: self.assertEqual(engine.types['standard'], ZopePathExpr) I'd argue that this test is testing for precisely the wrong thing, so I updated this assertion and the ones to follow to check for: self.assertEqual(engine.types['standard'], TrustedZopePathExpr) This fixes the original issue I was seeing. All Zope 2.12 and Plone 4 tests pass with this as well. I also think the fixed test in Five is now correct and sufficient, since it checks that we get the trusted engine for ViewPageTemplateFile's. Maybe we should have a functional test too, but I'm not sure how to set that up. I've committed this in r106436 and merged to trunk in r106437. If anyone objects, please let me know and I'll back it out. Otherwise, I'm hopeful for a 2.12.2 soon, as this breaks a few things in Plone. :-/ Martin -- Author of `Professional Plone Development`, a book for developers who want to work with Plone. See http://martinaspeli.net/plone-book
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Martin Aspeli wrote:
On 13/12/09 16:49, Martin Aspeli wrote:
On 13/12/09 10:52, Tres Seaver wrote:
Doesn't smell like a regression to me: the code there hasn't changed in a good long while. Can you write a test case for it, so that we can test against earlier versions? Aha! http://codespeak.net/pipermail/z3-five/2007q2/002185.html
This is the same problem.
You said:
"This is becuase 'Products.PageTemplates.Expression.createTrustedZopeEngine' only trusts 'python:' expressions; path traversal is still governed by 'boboAwareZopeTraverse', which uses 'restrictedTraverse'."
and then:
"As it turns out, it is only "partially trusted." The attached patch should make them "really trusted", at least for path expressions; does it help? I haven't added any tests, although my 2.10 branch checkout does pass all tests with this change"
The attachment is here:
http://codespeak.net/pipermail/z3-five/attachments/20070506/7f8a9ea8/attachm...
I'm going to poke around a Zope 2.12 checkout for a bit to see what sense I can make of this.
Okay, so it turns out your patch has gotten lost from Zope 2.10 to Zope 2.12.
This is the revision where it went in:
http://zope3.pov.lt/trac/changeset/77064/Zope/branches/2.10/lib/python/Produ...
I think that by accident this got committed with an unrelated change, since the commit message says "Use Five 1.5.5" and there's a change in svn:externals. Perhaps that's why this wasn't merged to trunk. The latest merge I can see is at r71802.
This also makes me worry about http://zope3.pov.lt/trac/browser/Zope/branches/2.10/lib/python/Products/Page... and http://zope3.pov.lt/trac/browser/Zope/branches/2.10/lib/python/Products/Page..., which may not have been merged, but I'm too far down the rabbit hole now to see clearly.
Anyway, I re-applied your patch to the Zope 2.12 branch. This broke one test, in Products.Five:
self.assertEqual(engine.types['standard'], ZopePathExpr)
I'd argue that this test is testing for precisely the wrong thing, so I updated this assertion and the ones to follow to check for:
self.assertEqual(engine.types['standard'], TrustedZopePathExpr)
This fixes the original issue I was seeing. All Zope 2.12 and Plone 4 tests pass with this as well.
I also think the fixed test in Five is now correct and sufficient, since it checks that we get the trusted engine for ViewPageTemplateFile's. Maybe we should have a functional test too, but I'm not sure how to set that up.
I've committed this in r106436 and merged to trunk in r106437.
OK, sounds fine to me. Can you merge to the 2.11 branch as well? I think Andreas will be releasing 2.9.x through 2.12.x fairly soon.
If anyone objects, please let me know and I'll back it out. Otherwise, I'm hopeful for a 2.12.2 soon, as this breaks a few things in Plone. :-/
Heh, and after you have been just posting about using SVN develop eggs on your blog. ;) Tres. - -- =================================================================== Tres Seaver +1 540-429-0999 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAksmshYACgkQ+gerLs4ltQ4BNwCfctztlQ5F2uVVSPawCQ/sli2X hpYAoNSveWbE+NUx6G6BYxSEDsFjaa2v =wwi6 -----END PGP SIGNATURE-----
On 15/12/09 5:45, Tres Seaver wrote:
I've committed this in r106436 and merged to trunk in r106437.
OK, sounds fine to me. Can you merge to the 2.11 branch as well? I think Andreas will be releasing 2.9.x through 2.12.x fairly soon.
Sure, I'd forgotten about that one.
If anyone objects, please let me know and I'll back it out. Otherwise, I'm hopeful for a 2.12.2 soon, as this breaks a few things in Plone. :-/
Heh, and after you have been just posting about using SVN develop eggs on your blog. ;)
Right. I can run from a develop egg in this project, but I wouldn't want to ask everyone who uses Plone to do that. :) Martin -- Author of `Professional Plone Development`, a book for developers who want to work with Plone. See http://martinaspeli.net/plone-book
it took me hours to find out that i had the same issue in a plone-4.0a3 buildout until i found this thread in the meantime 2.12.2 is released so all one needs to do is to modify http://dist.plone.org/release/4.0a3/versions.cfg to use zope-2.12.2: [buildout] extends = http://download.zope.org/Zope2/index/2.12.2/versions.cfg ... Zope2 = 2.12.2 thanks for fixing this! fRiSi Martin Aspeli wrote:
On 15/12/09 5:45, Tres Seaver wrote:
I've committed this in r106436 and merged to trunk in r106437.
OK, sounds fine to me. Can you merge to the 2.11 branch as well? I think Andreas will be releasing 2.9.x through 2.12.x fairly soon.
Sure, I'd forgotten about that one.
If anyone objects, please let me know and I'll back it out. Otherwise, I'm hopeful for a 2.12.2 soon, as this breaks a few things in Plone. :-/
Heh, and after you have been just posting about using SVN develop eggs on your blog. ;)
Right. I can run from a develop egg in this project, but I wouldn't want to ask everyone who uses Plone to do that. :)
Martin
-- Webmeisterei GmbH - Büro für Netzfragen Tel: +43 5572 908877, Fax: +43 5572 908877-66 Steinebach 18, A-6850 Dornbirn http://www.webmeisterei.com
participants (3)
-
Harald Friessnegger -
Martin Aspeli -
Tres Seaver