Skip to content

Commit

Permalink
Allow try..except..finally in the new transformer.py
Browse files Browse the repository at this point in the history
Not allowing it would be a big regression, since the old RCompile allowed it
also.
  • Loading branch information
stephan-hof committed Oct 26, 2016
1 parent 746045b commit 33a3e2a
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 3 deletions.
9 changes: 6 additions & 3 deletions src/RestrictedPython/transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,16 +149,19 @@
if version >= (2, 7) and version < (2, 8):
AST_WHITELIST.extend([
ast.Print,
#ast.TryFinally, # TryFinally should not be supported
#ast.TryExcept, # TryExcept should not be supported
ast.Raise,
ast.TryExcept,
ast.TryFinally,
ast.ExceptHandler
])

if version >= (3, 0):
AST_WHITELIST.extend([
ast.Bytes,
ast.Starred,
ast.arg,
#ast.Try, # Try should not be supported
ast.Try,
ast.ExceptHandler
])

if version >= (3, 4):
Expand Down
92 changes: 92 additions & 0 deletions tests/test_transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -658,3 +658,95 @@ def test_transformer__RestrictingNodeTransformer__visit_Assign(compile, mocker):
_getiter_.assert_any_call((1, (2, 3)))
_getiter_.assert_any_call((2, 3))
_getiter_.reset_mock()


TRY_EXCEPT_FINALLY = """
def try_except(m):
try:
m('try')
raise IndentationError('f1')
except IndentationError as error:
m('except')
def try_except_else(m):
try:
m('try')
except:
m('except')
else:
m('else')
def try_finally(m):
try:
m('try')
1 / 0
finally:
m('finally')
return
def try_except_finally(m):
try:
m('try')
1 / 0
except:
m('except')
finally:
m('finally')
def try_except_else_finally(m):
try:
m('try')
except:
m('except')
else:
m('else')
finally:
m('finally')
"""

@pytest.mark.parametrize(*compile)
def test_transformer__RestrictingNodeTransformer__error_handling(compile, mocker):
code, errors = compile.compile_restricted_exec(TRY_EXCEPT_FINALLY)[:2]
assert code != None

glb = {}
six.exec_(code, glb)

trace = mocker.stub()

glb['try_except'](trace)
trace.assert_has_calls([
mocker.call('try'),
mocker.call('except')
])
trace.reset_mock()

glb['try_except_else'](trace)
trace.assert_has_calls([
mocker.call('try'),
mocker.call('else')
])
trace.reset_mock()

glb['try_finally'](trace)
trace.assert_has_calls([
mocker.call('try'),
mocker.call('finally')
])
trace.reset_mock()

glb['try_except_finally'](trace)
trace.assert_has_calls([
mocker.call('try'),
mocker.call('except'),
mocker.call('finally')
])
trace.reset_mock()

glb['try_except_else_finally'](trace)
trace.assert_has_calls([
mocker.call('try'),
mocker.call('else'),
mocker.call('finally')
])
trace.reset_mock()

4 comments on commit 33a3e2a

@icemac
Copy link
Member

@icemac icemac commented on 33a3e2a Oct 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stephan-hof Could you write a test which assures that the __traceback__ attribute of an exception on Python 3 is not accessible: See this post on StackOverflow which explains what I mean. Access to the traceback is a big attack vector.

@stephan-hof
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I could do that, but don't you think it is a duplication of the test 'test_transformer__RestrictingNodeTransformer__visit_Attribute__1' ?

@icemac
Copy link
Member

@icemac icemac commented on 33a3e2a Nov 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is duplicate but I want to be super safe here that it is not possible to access the traceback in the exception due to the security problems.

@stephan-hof
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a test here: 4e90e02

Please sign in to comment.