openharmony/third_party_pyyaml
1
0
Fork 0
mirror of https://gitee.com/openharmony/third_party_pyyaml synced 2024-11-23 07:20:31 +00:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

Prevents arbitrary code execution during python/object/new constructor (#386)

Browse Source 
* Prevents arbitrary code execution during python/object/new constructor

In FullLoader python/object/new constructor, implemented by
construct_python_object_apply, has support for setting the state of a
deserialized instance through the set_python_instance_state method.
After setting the state, some operations are performed on the instance
to complete its initialization, however it is possible for an attacker
to set the instance' state in such a way that arbitrary code is executed
by the FullLoader.

This patch tries to block such attacks in FullLoader by preventing
set_python_instance_state from setting arbitrary properties. It
implements a blacklist that includes `extend` method (called by
construct_python_object_apply) and all special methods (e.g. __set__,
__setitem__, etc.).

Users who need special attributes being set in the state of a
deserialized object can still do it through the UnsafeLoader, which
however should not be used on untrusted input. Additionally, they can
subclass FullLoader and redefine `get_state_keys_blacklist()` to
extend/replace the list of blacklisted keys, passing the subclassed
loader to yaml.load.

* Make sure python/object/new constructor does not set some properties

* Add test to show how to subclass FullLoader with new blacklist
This commit is contained in:
Riccardo Schirone 2020-03-17 19:09:55 +01:00 committed by GitHub
parent 2f463cf5b0
commit 5080ba5133
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 100 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
lib/yaml/constructor.py
View File

@ -56,6 +56,14 @@ class BaseConstructor(object):
# If there are more documents available?
return self.check_node()
def check_state_key(self, key):
"""Block special attributes/methods from being set in a newly created
object, to prevent user-controlled methods from being called during
deserialization"""
if self.get_state_keys_blacklist_regexp().match(key):
raise ConstructorError(None, None,
"blacklisted key '%s' in instance state found" % (key,), None)
def get_data(self):
# Construct and return the next document.
if self.check_node():
@ -495,6 +503,16 @@ SafeConstructor.add_constructor(None,
SafeConstructor.construct_undefined)
class FullConstructor(SafeConstructor):
# 'extend' is blacklisted because it is used by
# construct_python_object_apply to add `listitems` to a newly generate
# python instance
def get_state_keys_blacklist(self):
return ['^extend$', '^__.*__$']
def get_state_keys_blacklist_regexp(self):
if not hasattr(self, 'state_keys_blacklist_regexp'):
self.state_keys_blacklist_regexp = re.compile('(' + '|'.join(self.get_state_keys_blacklist()) + ')')
return self.state_keys_blacklist_regexp
def construct_python_str(self, node):
return self.construct_scalar(node).encode('utf-8')
@ -590,7 +608,7 @@ class FullConstructor(SafeConstructor):
else:
return cls(*args, **kwds)
def set_python_instance_state(self, instance, state):
def set_python_instance_state(self, instance, state, unsafe=False):
if hasattr(instance, '__setstate__'):
instance.__setstate__(state)
else:
@ -598,10 +616,15 @@ class FullConstructor(SafeConstructor):
if isinstance(state, tuple) and len(state) == 2:
state, slotstate = state
if hasattr(instance, '__dict__'):
if not unsafe and state:
for key in state.keys():
self.check_state_key(key)
instance.__dict__.update(state)
elif state:
slotstate.update(state)
for key, value in slotstate.items():
if not unsafe:
self.check_state_key(key)
setattr(instance, key, value)
def construct_python_object(self, suffix, node):
@ -723,6 +746,10 @@ class UnsafeConstructor(FullConstructor):
return super(UnsafeConstructor, self).make_python_instance(
suffix, node, args, kwds, newobj, unsafe=True)
def set_python_instance_state(self, instance, state):
return super(UnsafeConstructor, self).set_python_instance_state(
instance, state, unsafe=True)
UnsafeConstructor.add_multi_constructor(
u'tag:yaml.org,2002:python/object/apply:',
UnsafeConstructor.construct_python_object_apply)

29
lib3/yaml/constructor.py
View File

@ -31,6 +31,14 @@ class BaseConstructor:
# If there are more documents available?
return self.check_node()
def check_state_key(self, key):
"""Block special attributes/methods from being set in a newly created
object, to prevent user-controlled methods from being called during
deserialization"""
if self.get_state_keys_blacklist_regexp().match(key):
raise ConstructorError(None, None,
"blacklisted key '%s' in instance state found" % (key,), None)
def get_data(self):
# Construct and return the next document.
if self.check_node():
@ -472,6 +480,16 @@ SafeConstructor.add_constructor(None,
SafeConstructor.construct_undefined)
class FullConstructor(SafeConstructor):
# 'extend' is blacklisted because it is used by
# construct_python_object_apply to add `listitems` to a newly generate
# python instance
def get_state_keys_blacklist(self):
return ['^extend$', '^__.*__$']
def get_state_keys_blacklist_regexp(self):
if not hasattr(self, 'state_keys_blacklist_regexp'):
self.state_keys_blacklist_regexp = re.compile('(' + '|'.join(self.get_state_keys_blacklist()) + ')')
return self.state_keys_blacklist_regexp
def construct_python_str(self, node):
return self.construct_scalar(node)
@ -574,7 +592,7 @@ class FullConstructor(SafeConstructor):
else:
return cls(*args, **kwds)
def set_python_instance_state(self, instance, state):
def set_python_instance_state(self, instance, state, unsafe=False):
if hasattr(instance, '__setstate__'):
instance.__setstate__(state)
else:
@ -582,10 +600,15 @@ class FullConstructor(SafeConstructor):
if isinstance(state, tuple) and len(state) == 2:
state, slotstate = state
if hasattr(instance, '__dict__'):
if not unsafe and state:
for key in state.keys():
self.check_state_key(key)
instance.__dict__.update(state)
elif state:
slotstate.update(state)
for key, value in slotstate.items():
if not unsafe:
self.check_state_key(key)
setattr(instance, key, value)
def construct_python_object(self, suffix, node):
@ -711,6 +734,10 @@ class UnsafeConstructor(FullConstructor):
return super(UnsafeConstructor, self).make_python_instance(
suffix, node, args, kwds, newobj, unsafe=True)
def set_python_instance_state(self, instance, state):
return super(UnsafeConstructor, self).set_python_instance_state(
instance, state, unsafe=True)
UnsafeConstructor.add_multi_constructor(
'tag:yaml.org,2002:python/object/apply:',
UnsafeConstructor.construct_python_object_apply)

5
tests/data/myfullloader.subclass_blacklist Normal file
View File

@ -0,0 +1,5 @@
- !!python/object/new:yaml.MappingNode
args:
state:
mymethod: test
wrong_method: test2

5
tests/data/overwrite-state-new-constructor.loader-error Normal file
View File

@ -0,0 +1,5 @@
- !!python/object/new:yaml.MappingNode
args:
state:
extend: test
__test__: test

18
tests/lib/test_constructor.py
View File

@ -17,7 +17,7 @@ def _make_objects():
global MyLoader, MyDumper, MyTestClass1, MyTestClass2, MyTestClass3, YAMLObject1, YAMLObject2, \
AnObject, AnInstance, AState, ACustomState, InitArgs, InitArgsWithState, \
NewArgs, NewArgsWithState, Reduce, ReduceWithState, Slots, MyInt, MyList, MyDict, \
FixedOffset, today, execute
FixedOffset, today, execute, MyFullLoader
class MyLoader(yaml.Loader):
pass
@ -235,6 +235,10 @@ def _make_objects():
def dst(self, dt):
return datetime.timedelta(0)
class MyFullLoader(yaml.FullLoader):
def get_state_keys_blacklist(self):
return super(MyFullLoader, self).get_state_keys_blacklist() + ['^mymethod$', '^wrong_.*$']
today = datetime.date.today()
def _load_code(expression):
@ -289,6 +293,18 @@ def test_constructor_types(data_filename, code_filename, verbose=False):
test_constructor_types.unittest = ['.data', '.code']
def test_subclass_blacklist_types(data_filename, verbose=False):
_make_objects()
try:
yaml.load(open(data_filename, 'rb').read(), MyFullLoader)
except yaml.YAMLError as exc:
if verbose:
print("%s:" % exc.__class__.__name__, exc)
else:
raise AssertionError("expected an exception")
test_subclass_blacklist_types.unittest = ['.subclass_blacklist']
if __name__ == '__main__':
import sys, test_constructor
sys.modules['test_constructor'] = sys.modules['__main__']

18
tests/lib3/test_constructor.py
View File

@ -14,7 +14,7 @@ def _make_objects():
global MyLoader, MyDumper, MyTestClass1, MyTestClass2, MyTestClass3, YAMLObject1, YAMLObject2, \
AnObject, AnInstance, AState, ACustomState, InitArgs, InitArgsWithState, \
NewArgs, NewArgsWithState, Reduce, ReduceWithState, Slots, MyInt, MyList, MyDict, \
FixedOffset, today, execute
FixedOffset, today, execute, MyFullLoader
class MyLoader(yaml.Loader):
pass
@ -222,6 +222,10 @@ def _make_objects():
def dst(self, dt):
return datetime.timedelta(0)
class MyFullLoader(yaml.FullLoader):
def get_state_keys_blacklist(self):
return super().get_state_keys_blacklist() + ['^mymethod$', '^wrong_.*$']
today = datetime.date.today()
def _load_code(expression):
@ -274,6 +278,18 @@ def test_constructor_types(data_filename, code_filename, verbose=False):
test_constructor_types.unittest = ['.data', '.code']
def test_subclass_blacklist_types(data_filename, verbose=False):
_make_objects()
try:
yaml.load(open(data_filename, 'rb').read(), MyFullLoader)
except yaml.YAMLError as exc:
if verbose:
print("%s:" % exc.__class__.__name__, exc)
else:
raise AssertionError("expected an exception")
test_subclass_blacklist_types.unittest = ['.subclass_blacklist']
if __name__ == '__main__':
import sys, test_constructor
sys.modules['test_constructor'] = sys.modules['__main__']