Assert the path is inside OUTPUT dir before calling shutil.move

Author: humitos

readthedocs/core/utils/filesystem.py

@@ -25,23 +25,28 @@
 
 
 def assert_path_is_inside_docroot(path):
+    """Assert that the given path is inside the DOCROOT directory."""
+    assert_path_is_inside_expected_path(path, Path(settings.DOCROOT).absolute())
+
+
+def assert_path_is_inside_expected_path(path, expected_path):
     """
-    Assert that the given path is inside the DOCROOT directory.
+    Assert that the given path is inside the expected path directory.
 
     Symlinks are resolved before checking, a SuspiciousFileOperation exception
-    will be raised if the path is outside the DOCROOT.
+    will be raised if the path is outside the expected path.
 
     .. warning::
 
        This operation isn't safe to TocTou (Time-of-check to Time-of-use) attacks.
        Users shouldn't be able to change files while this operation is done.
     """
     resolved_path = path.absolute().resolve()
-    docroot = Path(settings.DOCROOT).absolute()
-    if not path.is_relative_to(docroot):
+    if not path.is_relative_to(expected_path):
         log.error(
-            "Suspicious operation outside the docroot directory.",
+            "Suspicious operation outside the expected path directory.",
             path_resolved=str(resolved_path),
+            expected_path=str(expected_path),
         )
         raise SuspiciousFileOperation(path)
 

readthedocs/projects/tasks/builds.py

@@ -10,6 +10,7 @@
 import socket
 import subprocess
 from dataclasses import dataclass, field
+from pathlib import Path
 
 import structlog
 from celery import Task
@@ -41,6 +42,7 @@
 from readthedocs.builds.utils import memcache_lock
 from readthedocs.config.config import BuildConfigV2
 from readthedocs.config.exceptions import ConfigError
+from readthedocs.core.utils.filesystem import assert_path_is_inside_expected_path
 from readthedocs.doc_builder.director import BuildDirector
 from readthedocs.doc_builder.environments import (
     DockerBuildEnvironment,
@@ -630,13 +632,15 @@ def get_valid_artifact_types(self):
 
                 # Rename file as "<project_slug>-<version_slug>.<artifact_type>",
                 # which is the filename that Proxito serves for offline formats.
-                filename = list_dir[0]
-                _, extension = filename.rsplit(".")
+                filename = os.path.join(artifact_directory, list_dir[0])
+                _, extension = filename.rsplit(".", maxsplit=1)
+                output = os.path.join(
+                    self.data.project.checkout_path(self.data.version.slug),
+                    "_readthedocs/",
+                )
+                assert_path_is_inside_expected_path(Path(filename), Path(output))
                 shutil.move(
-                    os.path.join(
-                        artifact_directory,
-                        list_dir[0],
-                    ),
+                    filename,
                     os.path.join(
                         artifact_directory,
                         f"{self.data.project.slug}.{extension}",