Allow absolute symlinks

Upstream unblob inaccurately rewrites all symlinks to be relative.

This is certainly a poor choice from a security perspective, but we'll
only run within docker which limits the potential damage.

unblob/extractor.py

@@ -6,9 +6,8 @@
 
 from structlog import get_logger
 
-from .file_utils import carve, is_safe_path
+from .file_utils import carve
 from .models import Chunk, File, PaddingChunk, TaskResult, UnknownChunk, ValidChunk
-from .report import MaliciousSymlinkRemoved
 
 logger = get_logger()
 
@@ -39,69 +38,17 @@ def fix_permission(path: Path):
     path.chmod(mode)
 
 
-def is_recursive_link(path: Path) -> bool:
-    try:
-        path.resolve()
-    except RuntimeError:
-        return True
-    return False
-
-
-def fix_symlink(path: Path, outdir: Path, task_result: TaskResult) -> Path:
-    """Rewrites absolute symlinks to point within the extraction directory (outdir).
-
-    If it's not a relative symlink it is either removed it it attempts
-    to traverse outside of the extraction directory or rewritten to be
-    fully portable (no mention of the extraction directory in the link
-    value).
-    """
-    if is_recursive_link(path):
-        logger.error("Symlink loop identified, removing", path=path)
-        error_report = MaliciousSymlinkRemoved(
-            link=path.as_posix(), target=os.readlink(path)
-        )
-        task_result.add_report(error_report)
-        path.unlink()
-        return path
-
-    raw_target = os.readlink(path)
-    if not raw_target:
-        logger.error("Symlink with empty target, removing.")
-        path.unlink()
-        return path
-
-    target = Path(raw_target)
-
-    if target.is_absolute():
-        target = Path(target.as_posix().lstrip("/"))
-    else:
-        target = path.resolve()
-
-    safe = is_safe_path(outdir, target)
-
-    if not safe:
-        logger.error("Path traversal attempt through symlink, removing", target=target)
-        error_report = MaliciousSymlinkRemoved(
-            link=path.as_posix(), target=target.as_posix()
-        )
-        task_result.add_report(error_report)
-        path.unlink()
-    else:
-        relative_target = os.path.relpath(outdir.joinpath(target), start=path.parent)
-        path.unlink()
-        path.symlink_to(relative_target)
-    return path
-
-
-def fix_extracted_directory(outdir: Path, task_result: TaskResult):
+def fix_extracted_directory(outdir: Path, task_result: TaskResult):  # noqa: ARG001
     def _fix_extracted_directory(directory: Path):
         if not directory.exists():
             return
         for path in (directory / p for p in os.listdir(directory)):
             try:
                 fix_permission(path)
                 if path.is_symlink():
-                    fix_symlink(path, outdir, task_result)
+                    # Unlike upstream unblob, we allow symlinks to do anything they want. We run in docker so this
+                    # isn't as dangerous as it would be otherwise, but it's still probably
+                    # a questionable decision.
                     continue
                 if path.is_dir():
                     _fix_extracted_directory(path)

unblob/handlers/archive/_safe_tarfile.py

@@ -4,7 +4,7 @@
 
 from structlog import get_logger
 
-from unblob.extractor import is_safe_path
+from unblob.file_utils import is_safe_path
 from unblob.report import ExtractionProblem
 
 logger = get_logger()

unblob/report.py

@@ -121,15 +121,6 @@ class ExtractorTimedOut(ErrorReport):
     timeout: float
 
 
-@attr.define(kw_only=True, frozen=True)
-class MaliciousSymlinkRemoved(ErrorReport):
-    """Describes an error when malicious symlinks have been removed from disk."""
-
-    severity: Severity = Severity.WARNING
-    link: str
-    target: str
-
-
 @attr.define(kw_only=True, frozen=True)
 class MultiFileCollisionReport(ErrorReport):
     """Describes an error when MultiFiles collide on the same file."""