Repository: mkdocs/mkdocs
Pull request #3465 (2023-11-11 15:53:39)

Stop executing YAML tags for mkdocs_theme.yml, warn about third-party projects
@jorgectf 
This is the outcome of https://github.com/mkdocs/mkdocs/issues/3418
Which warned about arbitrary code execution via installing a theme and that theme having malicious YAML tags.
I found that there are a lot more ways for arbitrary code execution if you install an untrusted package and import it.
As such, this change only slightly reduces the attack surface, but at the same time I don't really consider it a security issue in the first place, and I don't think it'd be useful for users to see all versions of MkDocs as having a security vulnerability and then also see that this change fixes it (but actually not really).

But I agree:
* executing YAML tags in `mkdocs_theme.yml` *is* pointless.
* the documentation *could* definitely use a note about the dangers of third-party packages.

So, the changes are in accordance to that:

- Stop allowing arbitrary YAML tags for mkdocs_theme.yml
- Docs: warn about installing third-party plugins

Bonus change:

- Cache the result of the function `get_themes()`


Patch:
diff --git a/docs/dev-guide/plugins.md b/docs/dev-guide/plugins.md
index 8d4a8c63..20218607 100644
--- a/docs/dev-guide/plugins.md
+++ b/docs/dev-guide/plugins.md
@@ -15,6 +15,8 @@ appropriate package name and install it using `pip`:
 pip install mkdocs-foo-plugin
 ```
 
+WARNING: Installing an MkDocs plugin means installing a Python package and executing any code that the author has put in there. So, exercise the usual caution; there's no attempt at sandboxing.
+
 Once a plugin has been successfully installed, it is ready to use. It just needs
 to be [enabled](#using-plugins) in the configuration file. The [Catalog]
 repository has a large ranked list of plugins that you can install and use.
diff --git a/docs/user-guide/choosing-your-theme.md b/docs/user-guide/choosing-your-theme.md
index 8ababf5b..383ce193 100644
--- a/docs/user-guide/choosing-your-theme.md
+++ b/docs/user-guide/choosing-your-theme.md
@@ -196,6 +196,8 @@ theme supports the following options:
 
 A list of third party themes can be found at the [community wiki] page and [the ranked catalog][catalog]. If you have created your own, please add them there.
 
+WARNING: Installing an MkDocs theme means installing a Python package and executing any code that the author has put in there. So, exercise the usual caution; there's no attempt at sandboxing.
+
 [third party themes]: #third-party-themes
 [theme]: configuration.md#theme
 [Bootstrap]: https://getbootstrap.com/
diff --git a/mkdocs/theme.py b/mkdocs/theme.py
index 77134d41..bf587201 100644
--- a/mkdocs/theme.py
+++ b/mkdocs/theme.py
@@ -6,6 +6,12 @@ import warnings
 from typing import Any, Collection, MutableMapping
 
 import jinja2
+import yaml
+
+try:
+    from yaml import CSafeLoader as SafeLoader
+except ImportError:  # pragma: no cover
+    from yaml import SafeLoader  # type: ignore
 
 from mkdocs import localization, utils
 from mkdocs.config.base import ValidationError
@@ -118,12 +124,13 @@ class Theme(MutableMapping[str, Any]):
     def _load_theme_config(self, name: str) -> None:
         """Recursively load theme and any parent themes."""
         theme_dir = utils.get_theme_dir(name)
+        utils.get_themes.cache_clear()
         self.dirs.append(theme_dir)
 
         try:
             file_path = os.path.join(theme_dir, 'mkdocs_theme.yml')
             with open(file_path, 'rb') as f:
-                theme_config = utils.yaml_load(f)
+                theme_config = yaml.load(f, SafeLoader)
         except OSError as e:
             log.debug(e)
             raise ValidationError(
diff --git a/mkdocs/utils/__init__.py b/mkdocs/utils/__init__.py
index b9c034d7..1560dfa1 100644
--- a/mkdocs/utils/__init__.py
+++ b/mkdocs/utils/__init__.py
@@ -258,6 +258,7 @@ def get_theme_dir(name: str) -> str:
     return os.path.dirname(os.path.abspath(theme.load().__file__))
 
 
+@functools.lru_cache(maxsize=None)
 def get_themes() -> dict[str, EntryPoint]:
     """Return a dict of all installed themes as {name: EntryPoint}."""
     themes: dict[str, EntryPoint] = {}


Test patch:
diff --git a/mkdocs/tests/theme_tests.py b/mkdocs/tests/theme_tests.py
index 4d123c68..2b1aea90 100644
--- a/mkdocs/tests/theme_tests.py
+++ b/mkdocs/tests/theme_tests.py
@@ -76,7 +76,7 @@ class ThemeTests(unittest.TestCase):
         self.assertTrue('new' in theme)
         self.assertEqual(theme['new'], 42)
 
-    @mock.patch('mkdocs.utils.yaml_load', return_value={})
+    @mock.patch('yaml.load', return_value={})
     def test_no_theme_config(self, m):
         theme = Theme(name='mkdocs')
         self.assertEqual(m.call_count, 1)
@@ -89,7 +89,7 @@ class ThemeTests(unittest.TestCase):
                 {'static_templates': ['parent.html']},
             ]
         )
-        with mock.patch('mkdocs.utils.yaml_load', m) as m:
+        with mock.patch('yaml.load', m) as m:
             theme = Theme(name='mkdocs')
             self.assertEqual(m.call_count, 2)
             self.assertEqual(
diff --git a/mkdocs/tests/utils/utils_tests.py b/mkdocs/tests/utils/utils_tests.py
index a72044e2..2a39fddd 100644
--- a/mkdocs/tests/utils/utils_tests.py
+++ b/mkdocs/tests/utils/utils_tests.py
@@ -203,84 +203,6 @@ class UtilsTests(unittest.TestCase):
         utils.insort(a, (1, 'a'), key=lambda v: v[0])
         self.assertEqual(a, [(1, 'a'), (1, 'b'), (1, 'a'), (2, 'c')])
 
-    def test_get_themes(self):
-        themes = utils.get_theme_names()
-        self.assertIn('mkdocs', themes)
-        self.assertIn('readthedocs', themes)
-
-    @mock.patch('mkdocs.utils.entry_points', autospec=True)
-    def test_get_theme_dir(self, mock_iter):
-        path = 'some/path'
-
-        theme = mock.Mock()
-        theme.name = 'mkdocs2'
-        theme.dist.name = 'mkdocs2'
-        theme.load().__file__ = os.path.join(path, '__init__.py')
-
-        mock_iter.return_value = [theme]
-
-        self.assertEqual(utils.get_theme_dir(theme.name), os.path.abspath(path))
-
-    def test_get_theme_dir_keyerror(self):
-        with self.assertRaises(KeyError):
-            utils.get_theme_dir('nonexistanttheme')
-
-    @mock.patch('mkdocs.utils.entry_points', autospec=True)
-    def test_get_theme_dir_importerror(self, mock_iter):
-        theme = mock.Mock()
-        theme.name = 'mkdocs2'
-        theme.dist.name = 'mkdocs2'
-        theme.load.side_effect = ImportError()
-
-        mock_iter.return_value = [theme]
-
-        with self.assertRaises(ImportError):
-            utils.get_theme_dir(theme.name)
-
-    @mock.patch('mkdocs.utils.entry_points', autospec=True)
-    def test_get_themes_warning(self, mock_iter):
-        theme1 = mock.Mock()
-        theme1.name = 'mkdocs2'
-        theme1.dist.name = 'mkdocs2'
-        theme1.load().__file__ = "some/path1"
-
-        theme2 = mock.Mock()
-        theme2.name = 'mkdocs2'
-        theme2.dist.name = 'mkdocs3'
-        theme2.load().__file__ = "some/path2"
-
-        mock_iter.return_value = [theme1, theme2]
-
-        with self.assertLogs('mkdocs') as cm:
-            theme_names = utils.get_theme_names()
-        self.assertEqual(
-            '\n'.join(cm.output),
-            "WARNING:mkdocs.utils:A theme named 'mkdocs2' is provided by the Python "
-            "packages 'mkdocs3' and 'mkdocs2'. The one in 'mkdocs3' will be used.",
-        )
-        self.assertCountEqual(theme_names, ['mkdocs2'])
-
-    @mock.patch('mkdocs.utils.entry_points', autospec=True)
-    def test_get_themes_error(self, mock_iter):
-        theme1 = mock.Mock()
-        theme1.name = 'mkdocs'
-        theme1.dist.name = 'mkdocs'
-        theme1.load().__file__ = "some/path1"
-
-        theme2 = mock.Mock()
-        theme2.name = 'mkdocs'
-        theme2.dist.name = 'mkdocs2'
-        theme2.load().__file__ = "some/path2"
-
-        mock_iter.return_value = [theme1, theme2]
-
-        with self.assertRaisesRegex(
-            exceptions.ConfigurationError,
-            "The theme 'mkdocs' is a builtin theme but the package 'mkdocs2' "
-            "attempts to provide a theme with the same name.",
-        ):
-            utils.get_theme_names()
-
     def test_nest_paths(self, j=posixpath.join):
         result = utils.nest_paths(
             [
@@ -528,6 +450,89 @@ class UtilsTests(unittest.TestCase):
         self.assertEqual(meta.get_data(doc), (doc, {}))
 
 
+class ThemeUtilsTests(unittest.TestCase):
+    def setUp(self):
+        utils.get_themes.cache_clear()
+
+    def test_get_themes(self):
+        themes = utils.get_theme_names()
+        self.assertIn('mkdocs', themes)
+        self.assertIn('readthedocs', themes)
+
+    @mock.patch('mkdocs.utils.entry_points', autospec=True)
+    def test_get_theme_dir(self, mock_iter):
+        path = 'some/path'
+
+        theme = mock.Mock()
+        theme.name = 'mkdocs2'
+        theme.dist.name = 'mkdocs2'
+        theme.load().__file__ = os.path.join(path, '__init__.py')
+
+        mock_iter.return_value = [theme]
+
+        self.assertEqual(utils.get_theme_dir(theme.name), os.path.abspath(path))
+
+    def test_get_theme_dir_error(self):
+        with self.assertRaises(KeyError):
+            utils.get_theme_dir('nonexistanttheme')
+
+    @mock.patch('mkdocs.utils.entry_points', autospec=True)
+    def test_get_theme_dir_importerror(self, mock_iter):
+        theme = mock.Mock()
+        theme.name = 'mkdocs2'
+        theme.dist.name = 'mkdocs2'
+        theme.load.side_effect = ImportError()
+
+        mock_iter.return_value = [theme]
+
+        with self.assertRaises(ImportError):
+            utils.get_theme_dir(theme.name)
+
+    @mock.patch('mkdocs.utils.entry_points', autospec=True)
+    def test_get_themes_warning(self, mock_iter):
+        theme1 = mock.Mock()
+        theme1.name = 'mkdocs2'
+        theme1.dist.name = 'mkdocs2'
+        theme1.load().__file__ = "some/path1"
+
+        theme2 = mock.Mock()
+        theme2.name = 'mkdocs2'
+        theme2.dist.name = 'mkdocs3'
+        theme2.load().__file__ = "some/path2"
+
+        mock_iter.return_value = [theme1, theme2]
+
+        with self.assertLogs('mkdocs') as cm:
+            theme_names = utils.get_theme_names()
+        self.assertEqual(
+            '\n'.join(cm.output),
+            "WARNING:mkdocs.utils:A theme named 'mkdocs2' is provided by the Python "
+            "packages 'mkdocs3' and 'mkdocs2'. The one in 'mkdocs3' will be used.",
+        )
+        self.assertCountEqual(theme_names, ['mkdocs2'])
+
+    @mock.patch('mkdocs.utils.entry_points', autospec=True)
+    def test_get_themes_error(self, mock_iter):
+        theme1 = mock.Mock()
+        theme1.name = 'mkdocs'
+        theme1.dist.name = 'mkdocs'
+        theme1.load().__file__ = "some/path1"
+
+        theme2 = mock.Mock()
+        theme2.name = 'mkdocs'
+        theme2.dist.name = 'mkdocs2'
+        theme2.load().__file__ = "some/path2"
+
+        mock_iter.return_value = [theme1, theme2]
+
+        with self.assertRaisesRegex(
+            exceptions.ConfigurationError,
+            "The theme 'mkdocs' is a builtin theme but the package 'mkdocs2' "
+            "attempts to provide a theme with the same name.",
+        ):
+            utils.get_theme_names()
+
+
 class LogCounterTests(unittest.TestCase):
     def setUp(self):
         self.log = logging.getLogger('dummy')