From 355513fe2e8c929f783de1109bee76c340cedced 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-b3ebd61913c9c4bc.yaml | 9 +++++++++ 4 files changed, 14 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/safeloader-cve-2016-4972-b3ebd61913c9c4bc.yaml diff --git a/murano/engine/yaql_yaml_loader.py b/murano/engine/yaql_yaml_loader.py index 395992995..42061c525 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 e85b24ebe..7214ef675 100644 --- a/murano/tests/functional/common/utils.py +++ b/murano/tests/functional/common/utils.py @@ -239,7 +239,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 bbf3958f3..6e69069f2 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-b3ebd61913c9c4bc.yaml b/releasenotes/notes/safeloader-cve-2016-4972-b3ebd61913c9c4bc.yaml new file mode 100644 index 000000000..f022c5c7e --- /dev/null +++ b/releasenotes/notes/safeloader-cve-2016-4972-b3ebd61913c9c4bc.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.