From b03c4759aa40d66bd4fcf62c96e352c117bdf4b9 Mon Sep 17 00:00:00 2001 From: Kirill Zaitsev Date: Fri, 27 May 2016 00:42:38 +0300 Subject: [PATCH] Use SafeLoader to load yaml files Before this patch yaml.Loader was used by the engine to create custom yaql-enabled yaml loader. It is unsafe do to so, because yaml.Loader is capable of creating custom python objects from specifically constructed yaml files. After this patch all yaml load operations are performed with safe loaders instead. Also use SafeConstructor instead of Constructor. Change-Id: I61a3c42d73608b5d013285f015a45f4774d264e3 Closes-Bug: #1586079 --- murano/engine/yaql_yaml_loader.py | 6 +++--- murano/tests/functional/common/utils.py | 2 +- murano/tests/unit/policy/test_congress_rules.py | 2 +- .../notes/safeloader-cve-2016-4972-4e7e42b9257c5628.yaml | 9 +++++++++ 4 files changed, 14 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/safeloader-cve-2016-4972-4e7e42b9257c5628.yaml diff --git a/murano/engine/yaql_yaml_loader.py b/murano/engine/yaql_yaml_loader.py index bc12e58d7..7678fe224 100644 --- a/murano/engine/yaql_yaml_loader.py +++ b/murano/engine/yaql_yaml_loader.py @@ -43,7 +43,7 @@ def get_loader(version): node.end_mark.line + 1, node.end_mark.column + 1) - class MuranoPlYamlConstructor(yaml.constructor.Constructor): + class MuranoPlYamlConstructor(yaml.constructor.SafeConstructor): def construct_yaml_map(self, node): data = MuranoPlDict() data.source_file_position = build_position(node) @@ -51,7 +51,7 @@ def get_loader(version): value = self.construct_mapping(node) data.update(value) - class YaqlYamlLoader(yaml.Loader, MuranoPlYamlConstructor): + class YaqlYamlLoader(yaml.SafeLoader, MuranoPlYamlConstructor): pass YaqlYamlLoader.add_constructor( @@ -60,7 +60,7 @@ def get_loader(version): # workaround for PyYAML bug: http://pyyaml.org/ticket/221 resolvers = {} - for k, v in yaml.Loader.yaml_implicit_resolvers.items(): + for k, v in yaml.SafeLoader.yaml_implicit_resolvers.items(): resolvers[k] = v[:] YaqlYamlLoader.yaml_implicit_resolvers = resolvers diff --git a/murano/tests/functional/common/utils.py b/murano/tests/functional/common/utils.py index 41744f284..e02853795 100644 --- a/murano/tests/functional/common/utils.py +++ b/murano/tests/functional/common/utils.py @@ -247,7 +247,7 @@ class DeployTestMixin(zip_utils.ZipUtilsMixin): """ component = service.to_dict() component = json.dumps(component) - return yaml.load(component) + return yaml.safe_load(component) @classmethod def get_service_id(cls, service): diff --git a/murano/tests/unit/policy/test_congress_rules.py b/murano/tests/unit/policy/test_congress_rules.py index 6a8c7cb8d..ad9bbb573 100644 --- a/murano/tests/unit/policy/test_congress_rules.py +++ b/murano/tests/unit/policy/test_congress_rules.py @@ -87,7 +87,7 @@ class TestCongressRules(unittest.TestCase): os.path.dirname(inspect.getfile(self.__class__)), file_name) with open(model_file) as stream: - return yaml.load(stream) + return yaml.safe_load(stream) def _create_rules_str(self, model_file, package_loader=None): model = self._load_file(model_file) diff --git a/releasenotes/notes/safeloader-cve-2016-4972-4e7e42b9257c5628.yaml b/releasenotes/notes/safeloader-cve-2016-4972-4e7e42b9257c5628.yaml new file mode 100644 index 000000000..f022c5c7e --- /dev/null +++ b/releasenotes/notes/safeloader-cve-2016-4972-4e7e42b9257c5628.yaml @@ -0,0 +1,9 @@ +--- +security: + - cve-2016-4972 has been addressed. In ceveral places + Murano used loaders inherited directly from yaml.Loader + when parsing MuranoPL and UI files from packages. + This is unsafe, because this loader is capable of creating + custom python objects from specifically constructed + yaml files. With this change all yaml loading operations are done + using safe loaders instead.