Drop safe_tar_extract()

Let's just run tar as a child process instead.

diff --git a/mkosi/distributions/gentoo.py b/mkosi/distributions/gentoo.py
index b672f063ea2a8ced4a29d1fa9f4193d8733c2938..83abab20c25cbc255e41a729fea456c160d6f9a8 100644 (file)
--- a/mkosi/distributions/gentoo.py
+++ b/mkosi/distributions/gentoo.py
@@ -3,7 +3,6 @@
 import logging
 import os
 import re
-import tarfile
 import urllib.parse
 import urllib.request
 from collections.abc import Sequence
@@ -14,9 +13,8 @@ from mkosi.distributions import DistributionInstaller
 from mkosi.install import copy_path, flock
 from mkosi.log import ARG_DEBUG, complete_step, die, log_step
 from mkosi.remove import unlink_try_hard
-from mkosi.run import run_workspace_command
+from mkosi.run import run, run_workspace_command
 from mkosi.state import MkosiState
-from mkosi.util import safe_tar_extract
 
 ARCHITECTURES = {
     "x86_64": ("amd64", "arch/x86/boot/bzImage"),
@@ -235,10 +233,16 @@ class Gentoo:
 
         with flock(stage3_tmp_extract):
             if not stage3_tmp_extract.joinpath(".cache_isclean").exists():
-                with tarfile.open(stage3_tar_path) as tfd:
-                    log_step(f"Extracting {stage3_tar.name} to "
-                             f"{stage3_tmp_extract}")
-                    safe_tar_extract(tfd, stage3_tmp_extract, numeric_owner=True)
+                log_step(f"Extracting {stage3_tar.name} to {stage3_tmp_extract}")
+
+                run([
+                    "tar",
+                    "--numeric-owner",
+                    "-C", stage3_tmp_extract,
+                    "--extract",
+                    "--file", stage3_tar_path,
+                    "--exclude", "./dev",
+                ])
 
                 unlink_try_hard(stage3_tmp_extract.joinpath("dev"))
                 unlink_try_hard(stage3_tmp_extract.joinpath("proc"))

diff --git a/mkosi/util.py b/mkosi/util.py
index 281347e4eeb77f08259e8429aec28f34f09086d4..b19b28ef999ec98246a4e8ab0a92529c17ec0302 100644 (file)
--- a/mkosi/util.py
+++ b/mkosi/util.py
@@ -13,14 +13,11 @@ import re
 import resource
 import shutil
 import sys
-import tarfile
 import tempfile
 from collections.abc import Iterable, Iterator, Sequence
 from pathlib import Path
 from typing import Any, Callable, Optional, TypeVar
 
-from mkosi.log import die
-
 T = TypeVar("T")
 V = TypeVar("V")
 
@@ -207,32 +204,6 @@ def patch_file(filepath: Path, line_rewriter: Callable[[str], str]) -> None:
     shutil.move(temp_new_filepath, filepath)
 
 
-def safe_tar_extract(tar: tarfile.TarFile, path: Path=Path("."), *, numeric_owner: bool=False) -> None:
-    """Extract a tar without CVE-2007-4559.
-
-    Throws an exception if a member of the tar resolves to a path that would
-    be outside of the passed in target path.
-
-    Omits the member argument from TarFile.extractall, since we don't need it at
-    the moment.
-
-    See https://github.com/advisories/GHSA-gw9q-c7gh-j9vm
-    """
-    path = path.resolve()
-    members = []
-    for member in tar.getmembers():
-        target = path / member.name
-        try:
-            if not (member.ischr() or member.isblk()):
-                # a.relative_to(b) throws a ValueError if a is not a subpath of b
-                target.resolve().relative_to(path)
-                members += [member]
-        except ValueError as e:
-            die(f"Attempted path traversal in tar file {tar.name!r}", e)
-
-    tar.extractall(path, members=members, numeric_owner=numeric_owner)
-
-
 def sort_packages(packages: Iterable[str]) -> list[str]:
     """Sorts packages: normal first, paths second, conditional third"""
 

diff --git a/tests/test_util.py b/tests/test_util.py
index 614df2cc6848eb10b0e12423bea69249b2ad74b9..25df36848ebee2eb0340599d4c9ca0663cf4bad7 100644 (file)
--- a/tests/test_util.py
+++ b/tests/test_util.py
@@ -1,16 +1,10 @@
 # SPDX-License-Identifier: LGPL-2.1+
 
 import os
-import secrets
-import tarfile
-from pathlib import Path
-
-import pytest
 
 from mkosi.util import (
     Distribution,
     PackageType,
-    safe_tar_extract,
     set_umask,
 )
 
@@ -31,30 +25,3 @@ def test_set_umask() -> None:
     assert tmp1 == 0o767
     assert tmp2 == 0o757
     assert tmp3 == 0o777
-
-
-def test_safe_tar_extract(tmp_path: Path) -> None:
-    name = secrets.token_hex()
-    testfile = tmp_path / name
-    testfile.write_text("Evil exploit\n")
-
-    safe_tar = tmp_path / "safe.tar.gz"
-    with tarfile.TarFile.open(safe_tar, "x:gz") as t:
-        t.add(testfile, arcname=name)
-
-    evil_tar = tmp_path / "evil.tar.gz"
-    with tarfile.TarFile.open(evil_tar, "x:gz") as t:
-        t.add(testfile, arcname=f"../../../../../../../../../../../../../../tmp/{name}")
-
-    safe_target = tmp_path / "safe_target"
-    with tarfile.TarFile.open(safe_tar) as t:
-        safe_tar_extract(t, safe_target)
-    assert (safe_target / name).exists()
-
-    evil_target = tmp_path / "evil_target"
-    with pytest.raises(ValueError):
-        with tarfile.TarFile.open(evil_tar) as t:
-            safe_tar_extract(t, evil_target)
-    assert not (evil_target / name).exists()
-    assert not (Path("/tmp") / name).exists()
-