Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions pooch/processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import lzma
import os
import shutil
import sys
import tarfile
import typing
from tarfile import TarFile
from zipfile import ZipFile
Expand Down Expand Up @@ -256,7 +256,13 @@ def _extract_file(self, fname, extract_dir):
This method receives an argument for the archive to extract and the
destination path.
"""
filter_kwarg = {} if sys.version_info < (3, 12) else {"filter": "data"}
# Extract with the "data" filter to reject unsafe members (symlinks,
# hardlinks, or absolute/``..`` paths pointing outside the destination)
# that would otherwise let a malicious archive write files anywhere on
# disk. The filter was added in Python 3.12 and backported to 3.9.17,
# 3.10.12, and 3.11.4, so detect it by feature instead of minor version
# to protect every interpreter that supports it (see GH #543).
filter_kwarg = {"filter": "data"} if hasattr(tarfile, "data_filter") else {}
with TarFile.open(fname, "r") as tar_file:
if self.members is None:
get_logger().info(
Expand Down
48 changes: 48 additions & 0 deletions pooch/tests/test_processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@
Test the processor hooks
"""

import io
import os
import re
import tarfile
from pathlib import Path
from tempfile import TemporaryDirectory

Expand Down Expand Up @@ -285,3 +288,48 @@ def test_unpacking_wrong_members_then_no_members(processor_class, extension):
processor2 = processor_class()
filenames2 = pup.fetch("store" + extension, processor=processor2)
assert len(filenames2) > 0


@pytest.mark.skipif(
not hasattr(tarfile, "data_filter"),
reason="tar 'data' extraction filter is unavailable on this interpreter",
)
def test_untar_blocks_symlink_path_traversal():
"""
Untar must not let a malicious archive write outside the destination.

A tar entry that is a symlink pointing outside the extraction directory,
followed by a file written *through* that symlink, is a classic
archive path-traversal / arbitrary-write attack. The "data" extraction
filter blocks it, and Pooch must enable the filter whenever the running
``tarfile`` supports it (it was backported to 3.9.17, 3.10.12 and 3.11.4),
not only on Python >= 3.12.
https://github.com/fatiando/pooch/issues/543
"""
with TemporaryDirectory() as tmpdir:
tmp = Path(tmpdir)
outside = tmp / "outside"
outside.mkdir()
extract_dir = tmp / "store.tar.untar"
archive = tmp / "store.tar"

# Build a malicious tar: a symlink that escapes the destination,
# followed by a file written *through* that symlink into ``outside``.
with tarfile.open(archive, "w") as tar:
link = tarfile.TarInfo("sl")
link.type = tarfile.SYMTYPE
link.linkname = os.path.abspath(outside)
tar.addfile(link)

payload = b"owned"
member = tarfile.TarInfo("sl/owned.txt")
member.size = len(payload)
tar.addfile(member, io.BytesIO(payload))

# The "data" filter rejects the unsafe symlink, so extraction raises
# instead of silently writing through it.
with pytest.raises(tarfile.FilterError):
Untar()._extract_file(str(archive), str(extract_dir))

# The attacker-controlled file must NOT have escaped the destination.
assert not (outside / "owned.txt").exists()
Loading