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
This commit is contained in:
Mark Goddard 2019-03-18 09:11:20 +00:00
parent 14bfce8a72
commit 7eedd8f416
4 changed files with 11 additions and 7 deletions

View File

@ -78,7 +78,7 @@ def read_allocations(module):
filename = module.params['allocation_file'] filename = module.params['allocation_file']
try: try:
with open(filename, 'r') as f: with open(filename, 'r') as f:
content = yaml.load(f) content = yaml.safe_load(f)
except IOError as e: except IOError as e:
if e.errno == errno.ENOENT: if e.errno == errno.ENOENT:
# Ignore ENOENT - we will create the file. # Ignore ENOENT - we will create the file.
@ -87,7 +87,7 @@ def read_allocations(module):
except yaml.YAMLError as e: except yaml.YAMLError as e:
module.fail_json(msg="Failed to parse allocation file %s as YAML" % filename) module.fail_json(msg="Failed to parse allocation file %s as YAML" % filename)
if content is None: 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 = {} content = {}
return content return content

View File

@ -89,7 +89,7 @@ def read_allocations(module):
filename = module.params['allocation_file'] filename = module.params['allocation_file']
try: try:
with open(filename, 'r') as f: with open(filename, 'r') as f:
content = yaml.load(f) content = yaml.safe_load(f)
except IOError as e: except IOError as e:
if e.errno == errno.ENOENT: if e.errno == errno.ENOENT:
# Ignore ENOENT - we will create the file. # Ignore ENOENT - we will create the file.
@ -98,7 +98,7 @@ def read_allocations(module):
except yaml.YAMLError as e: except yaml.YAMLError as e:
module.fail_json(msg="Failed to parse allocation file %s as YAML" % filename) module.fail_json(msg="Failed to parse allocation file %s as YAML" % filename)
if content is None: 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 = {} content = {}
return content return content

View File

@ -16,6 +16,7 @@ import subprocess
import unittest import unittest
import mock import mock
import yaml
from kayobe import utils from kayobe import utils
@ -63,14 +64,17 @@ class TestCase(unittest.TestCase):
"/path/to/roles") "/path/to/roles")
@mock.patch.object(utils, "read_file") @mock.patch.object(utils, "read_file")
def test_read_yaml_file(self, mock_read): @mock.patch.object(yaml, "safe_load", wraps=yaml.safe_load)
mock_read.return_value = """--- def test_read_yaml_file(self, mock_load, mock_read):
config = """---
key1: value1 key1: value1
key2: value2 key2: value2
""" """
mock_read.return_value = config
result = utils.read_yaml_file("/path/to/file") result = utils.read_yaml_file("/path/to/file")
self.assertEqual(result, {"key1": "value1", "key2": "value2"}) self.assertEqual(result, {"key1": "value1", "key2": "value2"})
mock_read.assert_called_once_with("/path/to/file") mock_read.assert_called_once_with("/path/to/file")
mock_load.assert_called_once_with(config)
@mock.patch.object(utils, "read_file") @mock.patch.object(utils, "read_file")
def test_read_yaml_file_open_failure(self, mock_read): def test_read_yaml_file_open_failure(self, mock_read):

View File

@ -100,7 +100,7 @@ def read_yaml_file(path):
(path, repr(e))) (path, repr(e)))
sys.exit(1) sys.exit(1)
try: try:
return yaml.load(content) return yaml.safe_load(content)
except yaml.YAMLError as e: except yaml.YAMLError as e:
print("Failed to decode config dump YAML file %s: %s" % print("Failed to decode config dump YAML file %s: %s" %
(path, repr(e))) (path, repr(e)))