Merge pull request #6897 from readthedocs/humitos/yaml-load-safely

Load YAML files safely

readthedocs/config/parser.py

@@ -4,7 +4,6 @@
 
 import yaml
 
-
 __all__ = ('parse', 'ParseError')
 
 

readthedocs/config/tests/test_yaml_loader.py

@@ -0,0 +1,20 @@
+import textwrap
+from readthedocs.doc_builder.backends.mkdocs import yaml_load_safely
+
+
+def test_yaml_load_safely():
+    expected = {
+        'int': 3,
+        'float': 3.0,
+        'unknown': None,
+    }
+
+    content = textwrap.dedent('''
+    int: 3
+    float: !!float 3
+    unknown: !!python/name:unknown_funtion
+    ''')
+    data = yaml_load_safely(content)
+    assert data == expected
+    assert type(data['int']) == int
+    assert type(data['float']) == float

readthedocs/doc_builder/backends/mkdocs.py

@@ -75,7 +75,7 @@ def get_final_doctype(self):
         https://www.mkdocs.org/user-guide/configuration/#use_directory_urls
         """
         with open(self.yaml_file, 'r') as f:
-            config = yaml.safe_load(f)
+            config = yaml_load_safely(f)
             use_directory_urls = config.get('use_directory_urls', True)
             return MKDOCS if use_directory_urls else MKDOCS_HTML
 
@@ -96,7 +96,7 @@ def load_yaml_config(self):
         :raises: ``MkDocsYAMLParseError`` if failed due to syntax errors.
         """
         try:
-            config = yaml.safe_load(open(self.yaml_file, 'r'))
+            config = yaml_load_safely(open(self.yaml_file, 'r'))
 
             if not config:
                 raise MkDocsYAMLParseError(
@@ -324,3 +324,29 @@ class MkdocsJSON(BaseMkdocs):
     type = 'mkdocs_json'
     builder = 'json'
     build_dir = '_build/json'
+
+
+class SafeLoaderIgnoreUnknown(yaml.SafeLoader):  # pylint: disable=too-many-ancestors
+
+    """
+    YAML loader to ignore unknown tags.
+
+    Borrowed from https://stackoverflow.com/a/57121993
+    """
+
+    def ignore_unknown(self, node):  # pylint: disable=no-self-use, unused-argument
+        return None
+
+
+SafeLoaderIgnoreUnknown.add_constructor(None, SafeLoaderIgnoreUnknown.ignore_unknown)
+
+
+def yaml_load_safely(content):
+    """
+    Uses ``SafeLoaderIgnoreUnknown`` loader to skip unknown tags.
+
+    When a YAML contains ``!!python/name:int`` it will complete ignore it an
+    return ``None`` for those fields instead of failing. We need this to avoid
+    executing random code, but still support these YAML files.
+    """
+    return yaml.load(content, Loader=SafeLoaderIgnoreUnknown)

readthedocs/rtd_tests/tests/test_doc_builder.py

@@ -12,7 +12,7 @@
 from unittest.mock import patch
 
 from readthedocs.builds.models import Version
-from readthedocs.doc_builder.backends.mkdocs import MkdocsHTML
+from readthedocs.doc_builder.backends.mkdocs import MkdocsHTML, yaml_load_safely
 from readthedocs.doc_builder.backends.sphinx import BaseSphinx
 from readthedocs.doc_builder.exceptions import MkDocsYAMLParseError
 from readthedocs.doc_builder.python_environments import Virtualenv
@@ -355,7 +355,7 @@ def test_append_conf_create_yaml(self, checkout_path, run):
         # There is a mkdocs.yml file created
         generated_yaml = os.path.join(tmpdir, 'mkdocs.yml')
         self.assertTrue(os.path.exists(generated_yaml))
-        config = yaml.safe_load(open(generated_yaml))
+        config = yaml_load_safely(open(generated_yaml))
         self.assertEqual(
             config['docs_dir'],
             os.path.join(tmpdir, 'docs'),
@@ -412,7 +412,7 @@ def test_append_conf_existing_yaml_on_root(self, checkout_path, run):
 
         run.assert_called_with('cat', 'mkdocs.yml', cwd=mock.ANY)
 
-        config = yaml.safe_load(open(yaml_file))
+        config = yaml_load_safely(open(yaml_file))
         self.assertEqual(
             config['docs_dir'],
             'docs',
@@ -502,7 +502,7 @@ def test_dont_override_theme(self, checkout_path, run):
 
         run.assert_called_with('cat', 'mkdocs.yml', cwd=mock.ANY)
 
-        config = yaml.safe_load(open(yaml_file))
+        config = yaml_load_safely(open(yaml_file))
         self.assertEqual(
             config['theme_dir'],
             'not-readthedocs',
@@ -606,7 +606,7 @@ def test_append_conf_existing_yaml_with_extra(self, checkout_path, run):
 
         run.assert_called_with('cat', 'mkdocs.yml', cwd=mock.ANY)
 
-        config = yaml.safe_load(open(yaml_file))
+        config = yaml_load_safely(open(yaml_file))
 
         self.assertEqual(
             config['extra_css'],