[Zope-dev] TTW leaking of annotions in Plone 4

[Roel Bruggink]

AFAIK this is still open, moved to Zope-Dev.

On Sun, Apr 24, 2011 at 12:18 PM, Martin Aspeli <optilude at gmx.net> wrote:
> Hi,
>
> On 23 April 2011 19:10, David Glick <dglick at gmail.com> wrote:
>> On 4/21/11 9:57 AM, Laurence Rowe wrote:
>>>
>>> On 21 April 2011 10:19, Roel Bruggink<roel at fourdigits.nl>  wrote:
>>>>
>>>> Hi,
>>>> We noticed that calling @@ on an url shows the annotations of that
>>>> object.
>>>> This works for anonymous, but only in Plone 4.
>>>> See http://plone-demo.sixfeetup.com/@@ .
>>>
>>> Ok, this is due to the standard view lookup code in
>>> ZPublisher.BaseRequest.BaseRequest.traverseName:
>>>
>>>     ob2 = namespaceLookup(ns, nm, ob, self)
>>>
>>> It's getting the adapter registered in zope.annotations:
>>>
>>>   <adapter
>>>       for=".interfaces.IAttributeAnnotatable *"
>>>       provides=".interfaces.IAnnotations"
>>>       factory=".attribute.AttributeAnnotations"
>>>       />
>>>
>>> I'm not quite sure what this 2-way adapter is for.
>>> AttributeAnnotations.__init__ takes an optional context argument which
>>> is promptly thrown away.
>>>
>>> Two ways to fix this:
>>>
>>> 1. Register a more specific adapter for (IAttributeAnnotatble,
>>> IRequest) which would raise a KeyError or something to provoke a
>>> NotFound response.
>>>
>>> 2. Add a __str__ method to AttributeAnnotations so no information is
>>> leaked when it is published.
>>>
>>> 3. Special case traverseName to check that the namespace name is not
>>> the empty string.
>>>
>>> I guess I can see the use-case for allowing @@ in page templates and
>>> such, but this is actually prevented by code in
>>> OFS.Traversable.Traversable.unrestrictedTraverse which specifically
>>> checks for nsParse(name)[1], the namespace name.
>>>
>>> Perhaps we would be better moving this discussion to zope-dev. It
>>> doesn't seem too serious from a security perspective.
>>
>> A possible long-term solution (involving an API change) would be to make the
>> view registration directives register views as adapters to an IBrowserPage
>> interface, and then make the view traverser look up adapters to that
>> interface instead of (implicitly) to Interface. Then it couldn't find random
>> (context,request) adapters that weren't intended to be views and that didn't
>> get security declarations.
>
> I agree this would be much better, but I think it's too big a change
> and something we'll need to live with: a lot of code does
> getMultiAdapter((context, request,), name="...") to get a view.
>
> A slightly more clunky but compatible idea may be to just explicitly
> check IView.providedBy(obj) during publication?
>
> Martin



-- 
Roel Bruggink
http://www.fourdigits.nl/mensen/roel-bruggink

Four Digits BV
http://www.fourdigits.nl tel: +31(0)26 4422700