From: Daan De Meyer <daan.j.demeyer@gmail.com>
Date: Sun, 23 Apr 2023 13:46:26 +0000 (+0200)
Subject: Drop safe_tar_extract()
X-Git-Tag: v15~202^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3a6db500cbcf77fbbf2a4082ec5b8fa0f514c715;p=thirdparty%2Fmkosi.git

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 b672f063e..83abab20c 100644
--- 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 281347e4e..b19b28ef9 100644
--- 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 614df2cc6..25df36848 100644
--- 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()
-