From 7eedd8f41600b227872145546d380d1a3bb4889c Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Mon, 18 Mar 2019 09:11:20 +0000 Subject: [PATCH] Fix YAML code execution issue There is an issue with the default YAML loader, which allows arbitrary code execution, as documented here: https://github.com/yaml/pyyaml/wiki/PyYAML-yaml.load(input)-Deprecation. This can be avoided by using yaml.safe_load. We don't require Python object serialisation, so safe_load is sufficient. Change-Id: I09190766066ab56d04b1317a4022782160d60528 Story: 2005253 Task: 30050 --- .../console-allocation/library/console_allocation.py | 4 ++-- ansible/roles/ip-allocation/library/ip_allocation.py | 4 ++-- kayobe/tests/unit/test_utils.py | 8 ++++++-- kayobe/utils.py | 2 +- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/ansible/roles/console-allocation/library/console_allocation.py b/ansible/roles/console-allocation/library/console_allocation.py index bb4f7bc45..2797a85a3 100644 --- a/ansible/roles/console-allocation/library/console_allocation.py +++ b/ansible/roles/console-allocation/library/console_allocation.py @@ -78,7 +78,7 @@ def read_allocations(module): filename = module.params['allocation_file'] try: with open(filename, 'r') as f: - content = yaml.load(f) + content = yaml.safe_load(f) except IOError as e: if e.errno == errno.ENOENT: # Ignore ENOENT - we will create the file. @@ -87,7 +87,7 @@ def read_allocations(module): except yaml.YAMLError as e: module.fail_json(msg="Failed to parse allocation file %s as YAML" % filename) if content is None: - # If the file is empty, yaml.load() will return None. + # If the file is empty, yaml.safe_load() will return None. content = {} return content diff --git a/ansible/roles/ip-allocation/library/ip_allocation.py b/ansible/roles/ip-allocation/library/ip_allocation.py index 73c9dacb0..0eb3ee944 100644 --- a/ansible/roles/ip-allocation/library/ip_allocation.py +++ b/ansible/roles/ip-allocation/library/ip_allocation.py @@ -89,7 +89,7 @@ def read_allocations(module): filename = module.params['allocation_file'] try: with open(filename, 'r') as f: - content = yaml.load(f) + content = yaml.safe_load(f) except IOError as e: if e.errno == errno.ENOENT: # Ignore ENOENT - we will create the file. @@ -98,7 +98,7 @@ def read_allocations(module): except yaml.YAMLError as e: module.fail_json(msg="Failed to parse allocation file %s as YAML" % filename) if content is None: - # If the file is empty, yaml.load() will return None. + # If the file is empty, yaml.safe_load() will return None. content = {} return content diff --git a/kayobe/tests/unit/test_utils.py b/kayobe/tests/unit/test_utils.py index 47f998b40..d46838e20 100644 --- a/kayobe/tests/unit/test_utils.py +++ b/kayobe/tests/unit/test_utils.py @@ -16,6 +16,7 @@ import subprocess import unittest import mock +import yaml from kayobe import utils @@ -63,14 +64,17 @@ class TestCase(unittest.TestCase): "/path/to/roles") @mock.patch.object(utils, "read_file") - def test_read_yaml_file(self, mock_read): - mock_read.return_value = """--- + @mock.patch.object(yaml, "safe_load", wraps=yaml.safe_load) + def test_read_yaml_file(self, mock_load, mock_read): + config = """--- key1: value1 key2: value2 """ + mock_read.return_value = config result = utils.read_yaml_file("/path/to/file") self.assertEqual(result, {"key1": "value1", "key2": "value2"}) mock_read.assert_called_once_with("/path/to/file") + mock_load.assert_called_once_with(config) @mock.patch.object(utils, "read_file") def test_read_yaml_file_open_failure(self, mock_read): diff --git a/kayobe/utils.py b/kayobe/utils.py index 4bed03d8e..b5f45b717 100644 --- a/kayobe/utils.py +++ b/kayobe/utils.py @@ -100,7 +100,7 @@ def read_yaml_file(path): (path, repr(e))) sys.exit(1) try: - return yaml.load(content) + return yaml.safe_load(content) except yaml.YAMLError as e: print("Failed to decode config dump YAML file %s: %s" % (path, repr(e)))