Merge pull request #13550 from dkjsone/handle_symlink

Add a function to check the security of symbolic links.

Author: notatallshaw

news/13550.bugfix.rst

@@ -0,0 +1,2 @@
+For Python versions that do not support PEP 706, pip will now raise an installation error for a
+source distribution when it includes a symlink that points outside the source distribution archive.

src/pip/_internal/utils/unpacking.py

@@ -248,13 +248,30 @@ def pip_filter(member: tarfile.TarInfo, path: str) -> tarfile.TarInfo:
         tar.close()
 
 
+def is_symlink_target_in_tar(tar: tarfile.TarFile, tarinfo: tarfile.TarInfo) -> bool:
+    """Check if the file pointed to by the symbolic link is in the tar archive"""
+    linkname = os.path.join(os.path.dirname(tarinfo.name), tarinfo.linkname)
+
+    linkname = os.path.normpath(linkname)
+    linkname = linkname.replace("\\", "/")
+
+    try:
+        tar.getmember(linkname)
+        return True
+    except KeyError:
+        return False
+
+
 def _untar_without_filter(
     filename: str,
     location: str,
     tar: tarfile.TarFile,
     leading: bool,
 ) -> None:
     """Fallback for Python without tarfile.data_filter"""
+    # NOTE: This function can be removed once pip requires CPython ≥ 3.12.​
+    # PEP 706 added tarfile.data_filter, made tarfile extraction operations more secure.
+    # This feature is fully supported from CPython 3.12 onward.
     for member in tar.getmembers():
         fn = member.name
         if leading:
@@ -269,6 +286,14 @@ def _untar_without_filter(
         if member.isdir():
             ensure_dir(path)
         elif member.issym():
+            if not is_symlink_target_in_tar(tar, member):
+                message = (
+                    "The tar file ({}) has a file ({}) trying to install "
+                    "outside target directory ({})"
+                )
+                raise InstallationError(
+                    message.format(filename, member.name, member.linkname)
+                )
             try:
                 tar._extract_member(member, path)
             except Exception as exc:

tests/unit/test_utils_unpacking.py

@@ -10,6 +10,7 @@
 from pathlib import Path
 
 import pytest
+from _pytest.monkeypatch import MonkeyPatch
 
 from pip._internal.exceptions import InstallationError
 from pip._internal.utils.unpacking import is_within_directory, untar_file, unzip_file
@@ -238,6 +239,148 @@ def test_unpack_tar_links(self, input_prefix: str, unpack_prefix: str) -> None:
         with open(os.path.join(unpack_dir, "symlink.txt"), "rb") as f:
             assert f.read() == content
 
+    def test_unpack_normal_tar_link1_no_data_filter(
+        self, monkeypatch: MonkeyPatch
+    ) -> None:
+        """
+        Test unpacking a normal tar with file containing soft links, but no data_filter
+        """
+        if hasattr(tarfile, "data_filter"):
+            monkeypatch.delattr("tarfile.data_filter")
+
+        tar_filename = "test_tar_links_no_data_filter.tar"
+        tar_filepath = os.path.join(self.tempdir, tar_filename)
+
+        extract_path = os.path.join(self.tempdir, "extract_path")
+
+        with tarfile.open(tar_filepath, "w") as tar:
+            file_data = io.BytesIO(b"normal\n")
+            normal_file_tarinfo = tarfile.TarInfo(name="normal_file")
+            normal_file_tarinfo.size = len(file_data.getbuffer())
+            tar.addfile(normal_file_tarinfo, fileobj=file_data)
+
+            info = tarfile.TarInfo("normal_symlink")
+            info.type = tarfile.SYMTYPE
+            info.linkpath = "normal_file"
+            tar.addfile(info)
+
+        untar_file(tar_filepath, extract_path)
+
+        assert os.path.islink(os.path.join(extract_path, "normal_symlink"))
+
+        link_path = os.readlink(os.path.join(extract_path, "normal_symlink"))
+        assert link_path == "normal_file"
+
+        with open(os.path.join(extract_path, "normal_symlink"), "rb") as f:
+            assert f.read() == b"normal\n"
+
+    def test_unpack_normal_tar_link2_no_data_filter(
+        self, monkeypatch: MonkeyPatch
+    ) -> None:
+        """
+        Test unpacking a normal tar with file containing soft links, but no data_filter
+        """
+        if hasattr(tarfile, "data_filter"):
+            monkeypatch.delattr("tarfile.data_filter")
+
+        tar_filename = "test_tar_links_no_data_filter.tar"
+        tar_filepath = os.path.join(self.tempdir, tar_filename)
+
+        extract_path = os.path.join(self.tempdir, "extract_path")
+
+        with tarfile.open(tar_filepath, "w") as tar:
+            file_data = io.BytesIO(b"normal\n")
+            normal_file_tarinfo = tarfile.TarInfo(name="normal_file")
+            normal_file_tarinfo.size = len(file_data.getbuffer())
+            tar.addfile(normal_file_tarinfo, fileobj=file_data)
+
+            info = tarfile.TarInfo("sub/normal_symlink")
+            info.type = tarfile.SYMTYPE
+            info.linkpath = ".." + os.path.sep + "normal_file"
+            tar.addfile(info)
+
+        untar_file(tar_filepath, extract_path)
+
+        assert os.path.islink(os.path.join(extract_path, "sub", "normal_symlink"))
+
+        link_path = os.readlink(os.path.join(extract_path, "sub", "normal_symlink"))
+        assert link_path == ".." + os.path.sep + "normal_file"
+
+        with open(os.path.join(extract_path, "sub", "normal_symlink"), "rb") as f:
+            assert f.read() == b"normal\n"
+
+    def test_unpack_evil_tar_link1_no_data_filter(
+        self, monkeypatch: MonkeyPatch
+    ) -> None:
+        """
+        Test unpacking a evil tar with file containing soft links, but no data_filter
+        """
+        if hasattr(tarfile, "data_filter"):
+            monkeypatch.delattr("tarfile.data_filter")
+
+        tar_filename = "test_tar_links_no_data_filter.tar"
+        tar_filepath = os.path.join(self.tempdir, tar_filename)
+
+        import_filename = "import_file"
+        import_filepath = os.path.join(self.tempdir, import_filename)
+        open(import_filepath, "w").close()
+
+        extract_path = os.path.join(self.tempdir, "extract_path")
+
+        with tarfile.open(tar_filepath, "w") as tar:
+            info = tarfile.TarInfo("evil_symlink")
+            info.type = tarfile.SYMTYPE
+            info.linkpath = import_filepath
+            tar.addfile(info)
+
+        with pytest.raises(InstallationError) as e:
+            untar_file(tar_filepath, extract_path)
+
+        msg = (
+            "The tar file ({}) has a file ({}) trying to install outside "
+            "target directory ({})"
+        )
+        assert msg.format(tar_filepath, "evil_symlink", import_filepath) in str(e.value)
+
+        assert not os.path.exists(os.path.join(extract_path, "evil_symlink"))
+
+    def test_unpack_evil_tar_link2_no_data_filter(
+        self, monkeypatch: MonkeyPatch
+    ) -> None:
+        """
+        Test unpacking a evil tar with file containing soft links, but no data_filter
+        """
+        if hasattr(tarfile, "data_filter"):
+            monkeypatch.delattr("tarfile.data_filter")
+
+        tar_filename = "test_tar_links_no_data_filter.tar"
+        tar_filepath = os.path.join(self.tempdir, tar_filename)
+
+        import_filename = "import_file"
+        import_filepath = os.path.join(self.tempdir, import_filename)
+        open(import_filepath, "w").close()
+
+        extract_path = os.path.join(self.tempdir, "extract_path")
+
+        link_path = ".." + os.sep + import_filename
+
+        with tarfile.open(tar_filepath, "w") as tar:
+            info = tarfile.TarInfo("evil_symlink")
+            info.type = tarfile.SYMTYPE
+            info.linkpath = link_path
+            tar.addfile(info)
+
+        with pytest.raises(InstallationError) as e:
+            untar_file(tar_filepath, extract_path)
+
+        msg = (
+            "The tar file ({}) has a file ({}) trying to install outside "
+            "target directory ({})"
+        )
+        assert msg.format(tar_filepath, "evil_symlink", link_path) in str(e.value)
+
+        assert not os.path.exists(os.path.join(extract_path, "evil_symlink"))
+
 
 def test_unpack_tar_unicode(tmpdir: Path) -> None:
     test_tar = tmpdir / "test.tar"