From: Joerg Behrmann <behrmann@physik.fu-berlin.de>
Date: Mon, 31 Oct 2022 09:40:58 +0000 (+0100)
Subject: Fix path traversal in tar extraction
X-Git-Tag: v15~396
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=43b15558a8191e009d1fc51063b87a5f275e8b89;p=thirdparty%2Fmkosi.git

Fix path traversal in tar extraction

This fixes CVE-2007-4559 [1], which was reported in an automated closing
campaign in PR #1253.

Crafted members of a tarfile with .. in their paths could resolve to paths
outside of the target directory to which the members would be extracted to.

[1] https://github.com/advisories/GHSA-gw9q-c7gh-j9vm

Closes: #1253
---

diff --git a/mkosi/backend.py b/mkosi/backend.py
index 4950a2c11..19db7fe30 100644
--- a/mkosi/backend.py
+++ b/mkosi/backend.py
@@ -17,6 +17,7 @@ import shutil
 import signal
 import subprocess
 import sys
+import tarfile
 import uuid
 from pathlib import Path
 from types import FrameType
@@ -988,3 +989,26 @@ def mkdirp_chown_current_user(
             continue
 
         chown_to_running_user(path)
+
+
+def safe_tar_extract(tar: tarfile.TarFile, path: Path=Path("."), *, numeric_owner: bool=False) -> None:
+    """Extract a tar without CVE-2007-4559.
+
+    Throws a MkosiException 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()
+    for member in tar.getmembers():
+        target = path / member.name
+        try:
+            # a.relative_to(b) throws a ValueError if a is not a subpath of b
+            target.resolve().relative_to(path)
+        except ValueError as e:
+            raise MkosiException(f"Attempted path traversal in tar file {tar.name!r}") from e
+
+    tar.extractall(path, numeric_owner=numeric_owner)
diff --git a/mkosi/gentoo.py b/mkosi/gentoo.py
index 8570a8d21..e8fb16960 100644
--- a/mkosi/gentoo.py
+++ b/mkosi/gentoo.py
@@ -23,6 +23,7 @@ from .backend import (
     die,
     root_home,
     run_workspace_command,
+    safe_tar_extract,
 )
 
 ARCHITECTURES = {
@@ -311,7 +312,7 @@ class Gentoo:
                 with tarfile.open(stage3_tar_path) as tfd:
                     MkosiPrinter.print_step(f"Extracting {stage3_tar.name} to "
                                             f"{stage3_tmp_extract}")
-                    tfd.extractall(stage3_tmp_extract, numeric_owner=True)
+                    safe_tar_extract(tfd, stage3_tmp_extract, numeric_owner=True)
 
                 # REMOVEME : pathetic attempt have this merged :)
                 # remove once upstream ships the current *baselayout-999*
diff --git a/tests/test_backend.py b/tests/test_backend.py
index 3e5dd0517..2c5ddcae8 100644
--- a/tests/test_backend.py
+++ b/tests/test_backend.py
@@ -1,9 +1,21 @@
 # SPDX-License-Identifier: LGPL-2.1+
 
 import os
+import secrets
+import tarfile
 from pathlib import Path
 
-from mkosi.backend import Distribution, PackageType, PartitionTable, set_umask, workspace
+import pytest
+
+from mkosi.backend import (
+    Distribution,
+    MkosiException,
+    PackageType,
+    PartitionTable,
+    safe_tar_extract,
+    set_umask,
+    workspace,
+)
 
 
 def test_distribution() -> None:
@@ -109,3 +121,29 @@ def test_disk_size() -> None:
     # When disk_size() cascade upwards to last_partition_offset, if clause.
     table.last_partition_sector = 32
     assert table.disk_size() == 36864
+
+
+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(MkosiException):
+        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()