Merged from 2.8 branch:

r39647 | jim | 2005-10-26 13:12:39 -0400 (Wed, 26 Oct 2005) | 6 lines

Fixed a bug in getting source that prevented tests from being used if
    there were pyc files around. Sigh.

Added tests for restrictions on augmented assignment and for handling
    generator expressions.

------------------------------------------------------------------------
    r39646 | jim | 2005-10-26 13:12:37 -0400 (Wed, 26 Oct 2005) | 2 lines

Added tests for handling generator expressions.

------------------------------------------------------------------------
    r39645 | jim | 2005-10-26 13:12:35 -0400 (Wed, 26 Oct 2005) | 3 lines

Added protection against the (small) risk that someone could mitate an
    object through an augmented assignment (aka inplace) operator.

------------------------------------------------------------------------
    r39644 | jim | 2005-10-26 13:12:32 -0400 (Wed, 26 Oct 2005) | 5 lines

Added protection against the (small) risk that someone could mitate an
    object through an augmented assignment (aka inplace) operator.

Also added handling for generator expressions.

------------------------------------------------------------------------
    r39643 | jim | 2005-10-26 13:12:30 -0400 (Wed, 26 Oct 2005) | 4 lines

Added notes on how restricted python works.  I hope I
    never need these again, but that's what I said the last time I had to
    rediscover how this worked. :)

doc/CHANGES.txt

@@ -71,6 +71,13 @@ Zope Changes
 
     Bugs Fixed
 
+      - If a content object implemented any in-place numeric operators, 
+        untrusted code could call them, thus modifying the content.
+
+      - If Python 2.4 is used, despite the fact that Python 2.4 is
+        unsupported, untrusted code could use generator expressions to
+        gain access to container items.
+
       - Collector #1895: testrunner: omitting the 'var' from recursive
         directory walking
 

lib/python/AccessControl/ZopeGuards.py

@@ -375,6 +375,112 @@ def _metaclass(name, bases, dict):
     ob._guarded_writes = 1
     return ob
 
+try:
+    valid_inplace_types = list, set
+except NameError:
+    # Python 2.3
+    valid_inplace_types = list
+
+inplace_slots = {
+    '+=': '__iadd__',
+    '-=': '__isub__',
+    '*=': '__imul__',
+    '/=': (1/2 == 0) and '__idiv__' or '__itruediv__',
+    '//=': '__ifloordiv__',
+    '%=': '__imod__',
+    '**=': '__ipow__',
+    '<<=': '__ilshift__',
+    '>>=': '__irshift__',
+    '&=': '__iand__',
+    '^=': '__ixor__',
+    '|=': '__ior_',
+    }
+
+
+def __iadd__(x, y):
+    x += y
+    return x
+
+def __isub__(x, y):
+    x -= y
+    return x
+
+def __imul__(x, y):
+    x *= y
+    return x
+
+def __idiv__(x, y):
+    x /= y
+    return x
+ 
+def __ifloordiv__(x, y):
+    x //= y
+    return x
+
+def __imod__(x, y):
+    x %= y
+    return x
+
+def __ipow__(x, y):
+    x **= y
+    return x
+
+def __ilshift__(x, y):
+    x <<= y
+    return x
+
+def __irshift__(x, y):
+    x >>= y
+    return x
+
+def __iand__(x, y):
+    x &= y
+    return x
+
+def __ixor__(x, y):
+    x ^= y
+    return x
+
+def __ior__(x, y):
+    x |= y
+    return x
+
+
+inplace_ops = {
+    '+=': __iadd__,
+    '-=': __isub__,
+    '*=': __imul__,
+    '/=': __idiv__,
+    '//=': __ifloordiv__,
+    '%=': __imod__,
+    '**=': __ipow__,
+    '<<=': __ilshift__,
+    '>>=': __irshift__,
+    '&=': __iand__,
+    '^=': __ixor__,
+    '|=': __ior__,
+    }
+
+
+def protected_inplacevar(op, var, expr):
+    """Do an inplace operation
+
+    If the var has an inplace slot, then disallow the operation
+    unless the var is a list.
+    """
+    if (hasattr(var, inplace_slots[op])
+        and not isinstance(var, valid_inplace_types)
+        ):
+        try:
+            cls = var.__class__
+        except AttributeError:
+            cls = type(var)
+        raise TypeError(
+            "Augmented assignment to %s objects is not allowed"
+            " in untrusted code" % cls.__name__
+            )
+    return inplace_ops[op](var, expr)
+
 # AccessControl clients generally need to set up a safe globals dict for
 # use by restricted code.  The get_safe_globals() function returns such
 # a dict, containing '__builtins__' mapped to our safe bulitins, and
@@ -394,6 +500,7 @@ _safe_globals = {'__builtins__': safe_builtins,
                  '_getiter_':    guarded_iter,
                  '_print_':      RestrictedPython.PrintCollector,
                  '_write_':      full_write_guard,
+                 '_inplacevar_': protected_inplacevar,
                  # The correct implementation of _getattr_, aka
                  # guarded_getattr, isn't known until
                  # AccessControl.Implementation figures that out, then

lib/python/AccessControl/tests/actual_python.py

@@ -157,3 +157,9 @@ f9()
 def f10():
     assert iter(enumerate(iter(iter(range(9))))).next() == (0, 0)
 f10()
+
+def f11():
+    x = 1
+    x += 1
+f11()
+

lib/python/AccessControl/tests/testZopeGuards.py

@@ -20,6 +20,7 @@ $Id$
 
 import os, sys
 import unittest
+from zope.testing import doctest
 import ZODB
 import AccessControl.SecurityManagement
 from AccessControl.SimpleObjectPolicies import ContainerAssertions
@@ -647,8 +648,90 @@ print foo(**kw)
             if callable(v) and v is not getattr(__builtin__, k, None):
                 d[k] = FuncWrapper(k, v)
 
+def test_inplacevar():
+    """
+Verify the correct behavior of protected_inplacevar.
+
+    >>> from AccessControl.ZopeGuards import protected_inplacevar
+
+Basic operations on objects without inplace slots work as expected:
+
+    >>> protected_inplacevar('+=', 1, 2)
+    3
+    >>> protected_inplacevar('-=', 5, 2)
+    3
+    >>> protected_inplacevar('*=', 5, 2)
+    10
+    >>> protected_inplacevar('/=', 6, 2)
+    3
+    >>> protected_inplacevar('%=', 5, 2)
+    1
+    >>> protected_inplacevar('**=', 5, 2)
+    25
+    >>> protected_inplacevar('<<=', 5, 2)
+    20
+    >>> protected_inplacevar('>>=', 5, 2)
+    1
+    >>> protected_inplacevar('&=', 5, 2)
+    0
+    >>> protected_inplacevar('^=', 7, 2)
+    5
+    >>> protected_inplacevar('|=', 5, 2)
+    7
+
+Inplace operations are allowed on lists:
+
+    >>> protected_inplacevar('+=', [1], [2])
+    [1, 2]
+
+    >>> protected_inplacevar('*=', [1], 2)
+    [1, 1]
+
+But not on custom objects:
+
+    >>> class C:
+    ...     def __iadd__(self, other):
+    ...         return 42
+    >>> protected_inplacevar('+=', C(), 2)    # doctest: +NORMALIZE_WHITESPACE
+    Traceback (most recent call last):
+    ...
+    TypeError: Augmented assignment to C objects is not allowed in
+    untrusted code
+"""
+
+if sys.version_info[:2] >= (2, 4):
+    def test_inplacevar_for_py24():
+        """
+protected_inplacevar allows inplce ops on sets:
+
+    >>> from AccessControl.ZopeGuards import protected_inplacevar
+    >>> s = set((1,2,3,4))
+    >>> sorted(protected_inplacevar('-=', s, set((1, 3))))
+    [2, 4]
+    >>> sorted(s)
+    [2, 4]
+    
+    >>> sorted(protected_inplacevar('|=', s, set((1, 3, 9))))
+    [1, 2, 3, 4, 9]
+    >>> sorted(s)
+    [1, 2, 3, 4, 9]
+
+    >>> sorted(protected_inplacevar('&=', s, set((1, 2, 3, 9))))
+    [1, 2, 3, 9]
+    >>> sorted(s)
+    [1, 2, 3, 9]
+
+    >>> sorted(protected_inplacevar('^=', s, set((1, 3, 7, 8))))
+    [2, 7, 8, 9]
+    >>> sorted(s)
+    [2, 7, 8, 9]
+
+"""
+
 def test_suite():
-    suite = unittest.TestSuite()
+    suite = unittest.TestSuite([
+        doctest.DocTestSuite(),
+        ])
     for cls in (TestGuardedGetattr,
                 TestDictGuards,
                 TestBuiltinFunctionGuards,

lib/python/RestrictedPython/RestrictionMutator.py

@@ -47,6 +47,7 @@ _getitem_name = ast.Name("_getitem_")
 _getiter_name = ast.Name("_getiter_")
 _print_target_name = ast.Name("_print")
 _write_name = ast.Name("_write_")
+_inplacevar_name = ast.Name("_inplacevar_")
 
 # Constants.
 _None_const = ast.Const(None)
@@ -239,9 +240,9 @@ class RestrictionMutator:
         #   for x in expr:
         # to
         #   for x in _getiter(expr):
+        #        # Note that visitListCompFor is the same thing.
         #
-        # Note that visitListCompFor is the same thing.  Exactly the same
-        # transformation is needed to convert
+        # Also for list comprehensions:
         #   [... for x in expr ...]
         # to
         #   [... for x in _getiter(expr) ...]
@@ -251,6 +252,15 @@ class RestrictionMutator:
 
     visitListCompFor = visitFor
 
+    def visitGenExprFor(self, node, walker):
+        # convert
+        #   (... for x in expr ...)
+        # to
+        #   (... for x in _getiter(expr) ...)
+        node = walker.defaultVisitNode(node)
+        node.iter = ast.CallFunc(_getiter_name, [node.iter])
+        return node
+
     def visitGetattr(self, node, walker):
         """Converts attribute access to a function call.
 
@@ -365,6 +375,21 @@ class RestrictionMutator:
         This could be a problem if untrusted code got access to a
         mutable database object that supports augmented assignment.
         """
+        if node.node.__class__.__name__ == 'Name':
+            node = walker.defaultVisitNode(node)
+            newnode = ast.Assign(
+                [ast.AssName(node.node.name, OP_ASSIGN)],
+                ast.CallFunc(
+                    _inplacevar_name,
+                    [ast.Const(node.op),
+                     ast.Name(node.node.name),
+                     node.expr,
+                     ]
+                    ),
+                )
+            newnode.lineno = node.lineno
+            return newnode
+        else:
             node.node.in_aug_assign = True
             return walker.defaultVisitNode(node)
 

lib/python/RestrictedPython/notes.txt

+How it works
+============
+
+Every time I see this code, I have to relearn it.  These notes will
+hopefully make this a little easier. :)
+
+- The important module is RCompile.  The entry points are the
+  compile_restricted_* functions.
+
+  + compile_restricted_function is used by Python scripts.
+
+  + compile_restricted_eval is used by ZPT
+
+    and by DTML indirectly through Eval.RestrictionCapableEval.
+
+- OK, so lets see how this works by following the logic of
+  compile_restricted_eval. 
+
+  - First, we create an RExpression, passing the source and a 
+    "file name", to be used in tracebacks.
+
+    Now, an RExpression is just:
+
+    + a subclass of RestrictedCompileMode and Expression.
+
+      Expression is a subclass of AbstractCompileMode that sets it's
+      mode to 'eval' and everided compile. Sigh.
+
+    + RestrictedCompileMode is a subclass of AbstractCompileMode
+      that changes a bunch of things. :)  These include compile, so we 
+      can ignore the compile we got from Expression.  It would have
+      been simpler to just set the dang mode in RExpression. Sigh.
+
+      RestrictedCompileMode seem to be the interestng base class. I
+      assume it implements the interesting functionality. We'll see
+      below...
+
+  - Next, we call compileAndTuplize.
+
+    + This calls compile on the RExpression.  It has an error
+      handler that does something that I hope I don't care about. :)
+
+    + It then calls the genCode method on the RExpression.  This is
+      boring, so we'll not worry about it.
+
+  - The compile method provided by RestrictedCompileMode is
+    interesting. 
+
+    + First it calls _get_tree.
+
+      * It uses compiler.parse to parse the source
+
+      * it uses MutatingWalker.walk to mutaate the tree using the
+        RestrictedCompileMode's 'rm' attr, which is a
+        RestrictionMutator.
+
+        The RestrictionMutator has the recipies for mutating the parse
+        tree.  (Note, for comparison, that Zope3's
+        zope.security.untrustedpython.rcompile module an alternative
+        RestrictionMutator that provides a much smaller set of
+        changes.)  
+
+        A mutator has visit method for different kinds of AST
+        nodes. These visit methods may mutate nodes or return new
+        nodes that replace the originally visited nodes.  There is a
+        default visitor that visits a node's children and replaces the
+        children who's visitors returned new nodes.
+
+        The walk function just calls the visitor for the root node of
+        the given tree.  Note _get_tree ignores the walk return value,
+        thus assuming that the visitor for the root node doesn't
+        return a new node.  This is a theoretical bug that we can
+        ignore. 
+
+    + Second, it generates the code. This too is boring. 
+
+- So this seems simple enough. ;)  When we want to add a check, we
+  need to update or add a visit function in RestrictionMutator.
+
+  How does a visit function work.
+
+  - First, we usually call walker.defaultVisitNode(node). This
+    transforms the node's child nodes.  
+
+  - Then we hack the node, or possibly return the node. To do this, we
+    have to know how the node works.
+
+  - The hack often involved changing the code to call some checker
+    function. These have names like _name_.  These are names that
+    would be illegal in the input source.
+
+    If this is a new function, we have to provide it in
+    AccessControl.ZopeGuards._safe_globals.
+
+- Don't forget to add a test case to tests.before_and_after.

lib/python/RestrictedPython/tests/before_and_after.py

@@ -244,3 +244,18 @@ def lambda_with_getattr_in_defaults_before():
 
 def lambda_with_getattr_in_defaults_after():
     f = lambda x=_getattr_(y, "z"): x
+
+
+# augmented operators
+# Note that we don't have to worry about item, attr, or slice assignment,
+# as they are disallowed. Yay!
+
+## def inplace_id_add_before():
+##     x += y+z
+
+## def inplace_id_add_after():
+##     x = _inplacevar_('+=', x, y+z)
+
+
+
+    

lib/python/RestrictedPython/tests/before_and_after24.py

+##############################################################################
+#
+# Copyright (c) 2003 Zope Corporation and Contributors.
+# All Rights Reserved.
+#
+# This software is subject to the provisions of the Zope Public License,
+# Version 2.0 (ZPL).  A copy of the ZPL should accompany this distribution.
+# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED
+# WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS
+# FOR A PARTICULAR PURPOSE.
+#
+##############################################################################
+"""Restricted Python transformation examples
+
+This module contains pairs of functions. Each pair has a before and an
+after function.  The after function shows the source code equivalent
+of the before function after it has been modified by the restricted
+compiler.
+
+These examples are actually used in the testRestrictions.py
+checkBeforeAndAfter() unit tests, which verifies that the restricted compiler
+actually produces the same output as would be output by the normal compiler
+for the after function.
+
+$Id$
+"""
+
+def simple_generator_expression_before():
+    x = (y**2 for y in whatever if y > 3)
+
+def simple_generator_expression_after():
+    x = (y**2 for y in _getiter_(whatever) if y > 3)
+
+def nested_generator_expression_before():
+    x = (x**2 + y**2 for x in whatever1 if x >= 0
+                     for y in whatever2 if y >= x)
+
+def nested_generator_expression_after():
+    x = (x**2 + y**2 for x in _getiter_(whatever1) if x >= 0
+                     for y in _getiter_(whatever2) if y >= x)

lib/python/RestrictedPython/tests/restricted_module.py

@@ -40,6 +40,10 @@ def try_apply():
     print f(*(300, 20), **{'z': 1}),
     return printed
 
+def try_inplace():
+    x = 1
+    x += 3
+
 def primes():
     # Somewhat obfuscated code on purpose
     print filter(None,map(lambda y:y*reduce(lambda x,y:x*y!=0,

lib/python/RestrictedPython/tests/security_in_syntax.py

@@ -54,3 +54,16 @@ def except_using_bad_name():
 def keyword_arg_with_bad_name():
     def f(okname=1, __badname=2):
         pass
+
+def no_augmeneted_assignment_to_sub():
+    a[b] += c
+
+def no_augmeneted_assignment_to_attr():
+    a.b += c
+
+def no_augmeneted_assignment_to_slice():
+    a[x:y] += c
+
+def no_augmeneted_assignment_to_slice2():
+    a[x:y:z] += c
+

lib/python/RestrictedPython/tests/testRestrictions.py

@@ -12,7 +12,7 @@ import unittest
 
 from RestrictedPython import compile_restricted, PrintCollector
 from RestrictedPython.Eval import RestrictionCapableEval
-from RestrictedPython.tests import before_and_after, restricted_module, verify
+from RestrictedPython.tests import restricted_module, verify
 from RestrictedPython.RCompile import RModule, RFunction
 
 try:
@@ -51,8 +51,13 @@ def find_source(fn, func):
     return fn, msg
 
 def get_source(func):
-    """Less silly interface to find_source""" # Sheesh
-    return find_source(func.func_globals['__file__'], func.func_code)[1]
+    """Less silly interface to find_source"""
+    file = func.func_globals['__file__']
+    if file.endswith('.pyc'):
+        file = file[:-1]
+    source = find_source(file, func.func_code)[1]
+    assert source.strip(), "Source should not be empty!"
+    return source
 
 def create_rmodule():
     global rmodule
@@ -175,6 +180,14 @@ def apply_wrapper(func, *args, **kws):
     apply_wrapper_called.append('yes')
     return func(*args, **kws)
 
+inplacevar_wrapper_called = {}
+def inplacevar_wrapper(op, x, y):
+    inplacevar_wrapper_called[op] = x, y
+    # This is really lame.  But it's just a test. :)
+    globs = {'x': x, 'y': y}
+    exec 'x'+op+'y' in globs
+    return globs['x']
+
 class RestrictionTests(unittest.TestCase):
     def execFunc(self, name, *args, **kw):
         func = rmodule[name]
@@ -191,6 +204,7 @@ class RestrictionTests(unittest.TestCase):
         # work for everything.
                                   '_getiter_': list,
                                   '_apply_': apply_wrapper,
+                                  '_inplacevar_': inplacevar_wrapper,
                                   })
         return func(*args, **kw)
 
@@ -243,6 +257,11 @@ class RestrictionTests(unittest.TestCase):
         self.assertEqual(apply_wrapper_called, ["yes"])
         self.assertEqual(res, "321")
 
+    def checkInplace(self):
+        inplacevar_wrapper_called.clear()
+        res = self.execFunc('try_inplace')
+        self.assertEqual(inplacevar_wrapper_called['+='], (1, 3))
+
     def checkDenied(self):
         for k in rmodule.keys():
             if k[:6] == 'denied':
@@ -314,7 +333,7 @@ class RestrictionTests(unittest.TestCase):
 
     def checkBeforeAndAfter(self):
         from RestrictedPython.RCompile import RModule
-
+        from RestrictedPython.tests import before_and_after
         from compiler import parse
 
         defre = re.compile(r'def ([_A-Za-z0-9]+)_(after|before)\(')
@@ -339,6 +358,34 @@ class RestrictionTests(unittest.TestCase):
             rm.compile()
             verify.verify(rm.getCode())
 
+    if sys.version_info[:2] >= (2, 4):
+        def checkBeforeAndAfter24(self):
+            from RestrictedPython.RCompile import RModule
+            from RestrictedPython.tests import before_and_after24
+            from compiler import parse
+
+            defre = re.compile(r'def ([_A-Za-z0-9]+)_(after|before)\(')
+
+            beforel = [name for name in before_and_after24.__dict__
+                       if name.endswith("_before")]
+
+            for name in beforel:
+                before = getattr(before_and_after24, name)
+                before_src = get_source(before)
+                before_src = re.sub(defre, r'def \1(', before_src)
+                rm = RModule(before_src, '')
+                tree_before = rm._get_tree()
+
+                after = getattr(before_and_after24, name[:-6]+'after')
+                after_src = get_source(after)
+                after_src = re.sub(defre, r'def \1(', after_src)
+                tree_after = parse(after_src)
+
+                self.assertEqual(str(tree_before), str(tree_after))
+
+                rm.compile()
+                verify.verify(rm.getCode())
+
     def _compile_file(self, name):
         path = os.path.join(_HERE, name)
         f = open(path, "r")
@@ -355,7 +402,7 @@ class RestrictionTests(unittest.TestCase):
         def getiter(seq):
             calls.append(seq)
             return list(seq)
-        globals = {"_getiter_": getiter}
+        globals = {"_getiter_": getiter, '_inplacevar_': inplacevar_wrapper}
         exec co in globals, {}
         # The comparison here depends on the exact code that is
         # contained in unpack.py.