Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v5.2 fails to load serialized exception with load(handle, Loader=yaml.FullLoader #364

Open
sphuber opened this issue Dec 16, 2019 · 1 comment

Comments

@sphuber
Copy link

sphuber commented Dec 16, 2019

Consider the following dump/load round trip where we serialize an exception:

import yaml
data = RuntimeError()
with open('test.yaml', 'w') as handle:
    yaml.dump(data, handle)
with open('test.yaml', 'r') as handle:
    yaml.load(handle, Loader=yaml.FullLoader)

This works fine with pyyaml~=5.1.0 however it fails for v5.2 with the following exception:

ConstructorError                          Traceback (most recent call last)
<ipython-input-1-325076656b95> in <module>()
      4     yaml.dump(data, handle)
      5 with open('test.yaml', 'r') as handle:
----> 6     yaml.load(handle, Loader=yaml.FullLoader)
      7 

/home/sphuber/.virtualenvs/aiida_dev/lib/python3.6/site-packages/yaml/__init__.py in load(stream, Loader)
    112     loader = Loader(stream)
    113     try:
--> 114         return loader.get_single_data()
    115     finally:
    116         loader.dispose()

/home/sphuber/.virtualenvs/aiida_dev/lib/python3.6/site-packages/yaml/constructor.py in get_single_data(self)
     41         node = self.get_single_node()
     42         if node is not None:
---> 43             return self.construct_document(node)
     44         return None
     45 

/home/sphuber/.virtualenvs/aiida_dev/lib/python3.6/site-packages/yaml/constructor.py in construct_document(self, node)
     45 
     46     def construct_document(self, node):
---> 47         data = self.construct_object(node)
     48         while self.state_generators:
     49             state_generators = self.state_generators

/home/sphuber/.virtualenvs/aiida_dev/lib/python3.6/site-packages/yaml/constructor.py in construct_object(self, node, deep)
     90                     constructor = self.__class__.construct_mapping
     91         if tag_suffix is None:
---> 92             data = constructor(self, node)
     93         else:
     94             data = constructor(self, tag_suffix, node)

/home/sphuber/.virtualenvs/aiida_dev/lib/python3.6/site-packages/yaml/constructor.py in construct_undefined(self, node)
    418         raise ConstructorError(None, None,
    419                 "could not determine a constructor for the tag %r" % node.tag,
--> 420                 node.start_mark)
    421 
    422 SafeConstructor.add_constructor(

ConstructorError: could not determine a constructor for the tag 'tag:yaml.org,2002:python/object/apply:builtins.RuntimeError'
  in "test.yaml", line 1, column 1

Is this difference in behavior expected?

@perlpunk
Copy link
Member

Yes it is.
Sorry, maybe we should add a comment about 5.2 to #265
object/apply allows arbitrary code execution, that's why you now (since 5.2) have to use the UnsafeLoader:

yaml.load(handle, Loader=yaml.UnsafeLoader)

frdeso added a commit to frdeso/lttng-ci that referenced this issue Dec 20, 2019
Since PyYAML version 5.2 `load()` errors out when parsing such field:
  !!python/object/apply:collections.OrderedDict [...]

This is done to prevent vulnerabilities exploitable using the
`object/apply` construct. See this Github issue [1]:

I believe the real bug is that Lava produce a yaml file with such a
construct. I believe it's not on purpose because we can see a commit [2]
preventing the use of `object/apply` for another type.

For now, use `unsafe_load()` until lava does not produce `object/apply`
contructs.

[1] yaml/pyyaml#364
[2] Linaro/lava@14b347c

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
frdeso added a commit to frdeso/lttng-ci that referenced this issue Dec 20, 2019
Since PyYAML version 5.2 `load()` errors out when parsing such field:
  !!python/object/apply:collections.OrderedDict [...]

This is done to prevent vulnerabilities exploitable using the
`object/apply` construct. See this Github issue [1]:

I believe the real bug is that Lava produce a yaml file with such a
construct. I believe it's not on purpose because we can see a commit [2]
preventing the use of `object/apply` for another type.

For now, use `unsafe_load()` until lava does not produce `object/apply`
contructs.

[1] yaml/pyyaml#364
[2] Linaro/lava@14b347c

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
PSRCode pushed a commit to lttng/lttng-ci that referenced this issue Dec 20, 2019
Since PyYAML version 5.2 `load()` errors out when parsing such field:
  !!python/object/apply:collections.OrderedDict [...]

This is done to prevent vulnerabilities exploitable using the
`object/apply` construct. See this Github issue [1]:

I believe the real bug is that Lava produce a yaml file with such a
construct. I believe it's not on purpose because we can see a commit [2]
preventing the use of `object/apply` for another type.

For now, use `unsafe_load()` until lava does not produce `object/apply`
contructs.

[1] yaml/pyyaml#364
[2] Linaro/lava@14b347c

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants