ModuleSecurityInfo('os').declarePublic('environ') allows os.system

[stephan-hof]

as mentioned above I have the following line in one of my zope Products.

ModuleSecurityInfo('os').declarePublic('environ')

Unfortunately in a PythonScript inside Zope, which should run under restricted python, I write code like this without getting an Unauthorized exception. Instead the os.system is happily executed, giving a lot of 'fun' to the users of my system :)

import os
os.system('ls /')

I looked a bit through the code and the following function in SecurityInfo.py got my attention.

class _ModuleSecurityInfo(SecurityInfo):
    """Encapsulate security information for modules."""

    __call____roles__ = ACCESS_PRIVATE
    def __call__(self, name, value):
        """Callback for __allow_access_to_unprotected_subobjects__ hook."""
        access = self.names.get(name, _marker)
        if access is not _marker:
            return access == ACCESS_PUBLIC
        return getattr(self, 'access', 0)

The interesting line is getattr(self, 'access', 0). Somehow self takes part in acquisition with the os module as parent.

print Acquisition.aq_chain(self) 
[<AccessControl.SecurityInfo._ModuleSecurityInfo object at 0xa25c28c>, <module 'os' from '/usr/lib/python2.7/os.pyc'>]

So, self having no attribute 'access', Acquisition jumps back to the os module which has an attribute being os.access

Access control evaluates os.access as True which means 'system' is allowed.

I don't have a big clue how AccessControl works, so its hard for me to say what is the 'appropriate' fix for this. Right now I changed the the __call__ method to this:

    __call____roles__ = ACCESS_PRIVATE
    def __call__(self, name, value):
        """Callback for __allow_access_to_unprotected_subobjects__ hook."""
        access = self.names.get(name, _marker)
        if access is not _marker:
            return access == ACCESS_PUBLIC

        me = Acquisition.aq_base(self)
        return getattr(me, 'access', 0)

[tseaver]

Thank you for the report! I believe your analysis is correct, and that acquiring access cannot be intended. Rather than use aq_base, I propose that we update SecurityInfo such that it sets access to False by default at the class level. I will test this change with a full Zope instance to ensure it doesn't break stuff.

[stephan-hof]

Hi,

I'll will apply your proposed change also to my Zope instance and let you know if something breaks.

[tseaver]

After checking: Setting access = False in the SecurityInfo base class causes tests to fail (and likely breaks a bunch of stuff). Setting it in the derived _ModuleSecurityInfo class lets them all pass, but should still fix your issue.

[tseaver]

via e47723e