Merge pull request #460 from onekey-sec/459-fix-tar-extractor

Resolve path traversal and unhandled permission error in tar handler

Author: vlaci

tests/integration/archive/tar/__input__/traversal.tar

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:0c0558a0d64c93f706175b7a38158a2909c07bf174ed54572e9f31896ddb20a6
+size 260

tests/integration/archive/tar/__output__/traversal.tar_extract/traversal

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:9d4ab5759508af8d62e4686b236f896210a4145a146ab399797993dcddcc9b22
+size 10240

tests/integration/archive/tar/__output__/traversal.tar_extract/traversal_extract/.gitkeep

@@ -0,0 +1,29 @@
+import os
+from pathlib import Path
+from tarfile import TarFile
+
+from structlog import get_logger
+
+from unblob.extractor import is_safe_path
+
+logger = get_logger()
+
+RUNNING_AS_ROOT = os.getuid() == 0
+
+
+class SafeTarFile(TarFile):
+    def extract(self, member, path="", set_attrs=True, *, numeric_owner=False):
+        path_as_path = Path(str(path))
+        member_name_path = Path(str(member.name))
+
+        if not RUNNING_AS_ROOT and (member.ischr() or member.isblk()):
+            logger.warn(
+                "missing elevated permissions, skipping block and character device creation",
+                path=member_name_path,
+            )
+            return
+        if not is_safe_path(path_as_path, member_name_path):
+            logger.warn("traversal attempt", path=member_name_path)
+            return
+
+        super().extract(member, path, set_attrs, numeric_owner=numeric_owner)

unblob/handlers/archive/_safe_tarfile.py

@@ -1,12 +1,12 @@
 import tarfile
+from pathlib import Path
 from typing import Optional
 
 from structlog import get_logger
 
-from unblob.extractors.command import Command
-
 from ...file_utils import OffsetFile, decode_int, round_up, snull
-from ...models import File, HexString, StructHandler, ValidChunk
+from ...models import Extractor, File, HexString, StructHandler, ValidChunk
+from ._safe_tarfile import SafeTarFile as safe_tarfile
 
 logger = get_logger()
 
@@ -66,6 +66,12 @@ def _find_end_of_padding(file, *, find_from: int) -> int:
     return find_from + padding_blocks * BLOCK_SIZE
 
 
+class TarExtractor(Extractor):
+    def extract(self, inpath: Path, outdir: Path):
+        tf = safe_tarfile.open(inpath.as_posix())
+        tf.extractall(outdir.as_posix())
+
+
 class TarHandler(StructHandler):
     NAME = "tar"
 
@@ -102,7 +108,7 @@ class TarHandler(StructHandler):
     """
     HEADER_STRUCT = "posix_header_t"
 
-    EXTRACTOR = Command("python3", "-m", "tarfile", "-e", "{inpath}", "{outdir}")
+    EXTRACTOR = TarExtractor()
 
     def calculate_chunk(self, file: File, start_offset: int) -> Optional[ValidChunk]:
         file.seek(start_offset)