Commit 24a5d995 authored by Klaus Wölfel's avatar Klaus Wölfel Committed by Tatuya Kamada

Allow python StringIO type to enable seek in restricted python

parent e7024ea4
...@@ -180,11 +180,13 @@ allow_type(type(re.compile(''))) ...@@ -180,11 +180,13 @@ allow_type(type(re.compile('')))
allow_type(type(re.match('x','x'))) allow_type(type(re.match('x','x')))
allow_type(type(re.finditer('x','x'))) allow_type(type(re.finditer('x','x')))
import cStringIO import cStringIO, StringIO
f = cStringIO.StringIO() f_cStringIO = cStringIO.StringIO()
f_StringIO = StringIO.StringIO()
allow_module('cStringIO') allow_module('cStringIO')
allow_module('StringIO') allow_module('StringIO')
allow_type(type(f)) allow_type(type(f_cStringIO))
allow_type(type(f_StringIO))
  • It seems this allow_type allowed too many types:

    >>> import StringIO
    >>> f_StringIO = StringIO.StringIO()
    >>> type(f_StringIO)
    <type 'instance'>
    

    which is same as:

    >>> class OldStyleClass:
    ...   pass
    ... 
    >>> type(OldStyleClass())
    <type 'instance'>

    so old style classes was allowed by default.

    For reference, it's different from:

    >>> class NewStyleClass(object):
    ...   pass
    ... 
    >>> type(NewStyleClass())
    <class '__main__.NewStyleClass'>

    so this did not change behaviour for new-style classes.

    I found this after extending testRestrictedPythonSecurity to run tests from zope with our patches applied, but I don't think it's a critical issue.

    I think we can change this to only allow StringIO instance only without allowing any old style classes instance. I started some patches in !1090 (merged) . ERP5 test suite seems OK now (I'm still waiting for test results).

    I think this may have impact on wendelin - and I believe it would be nice if we can also run this testRestrictedPythonSecurity after applying Wendelin is loaded (because of the changes to security machinery we do here)

    /cc @klaus @tatuya @Tyagov

  • @jerome, I think this should be OK for vanilla Wendelin. Still for customer projects I'd like @klaus to double check how / if this affects (they used a lot of restricted code and need it there).

Please register or sign in to reply
ModuleSecurityInfo('cgi').declarePublic('escape', 'parse_header') ModuleSecurityInfo('cgi').declarePublic('escape', 'parse_header')
allow_module('datetime') allow_module('datetime')
......
  • @jerome , thanks. I check later today.

  • @jerome Regarding the commit 24a5d995, you are absolutely right. Thank you for your great finding.

    f_StringIO = StringIO.StringIO()
    ...(snip)
    allow_type(type(f_StringIO))

    It was surely intended to allow StringIO.StringIO() only. We overlooked the fact that the type was <type 'instance'>.

    If you need some help to fixing the test failures regarding this issue (revert to not allowing 'instance'), please let me know it.

    Again, Thank you.

  • @tatuya thank you. About test failures, it seems ERP5 test suite is now OK in !1090 (merged) . There might be small not tested things that will break, or problems in project tests, but we can fix later I believe.

Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment