Skip to content
Merged
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
20 changes: 15 additions & 5 deletions Doc/library/tarfile.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1120,14 +1120,15 @@ reused in custom filters:
Implements the ``'tar'`` filter.

- Strip leading slashes (``/`` and :data:`os.sep`) from filenames.
- :ref:`Refuse <tarfile-extraction-refuse>` to extract files with absolute
paths (in case the name is absolute
even after stripping slashes, e.g. ``C:/foo`` on Windows).
This raises :class:`~tarfile.AbsolutePathError`.
- Normalize filenames (:attr:`TarInfo.name`) that contain ``..`` components
using :func:`os.path.normpath`.
Note that this removes internal ``..`` components, which may change the
meaning of the name if it traverses symbolic links.
- :ref:`Refuse <tarfile-extraction-refuse>` to extract files with absolute
paths (in case the name is absolute
even after stripping slashes, e.g. ``C:/foo`` on Windows)
or with a drive (e.g. ``C:foo`` on Windows).
This raises :class:`~tarfile.AbsolutePathError`.
- :ref:`Refuse <tarfile-extraction-refuse>` to extract files whose absolute
path (after following symlinks) would end up outside the destination.
This raises :class:`~tarfile.OutsideDestinationError`.
Expand All @@ -1152,6 +1153,10 @@ reused in custom filters:

Filenames containing ``..`` components are now normalized.

.. versionchanged:: next

Filenames with a drive that are not absolute are now refused.

.. function:: data_filter(member, path)

Implements the ``'data'`` filter.
Expand All @@ -1164,7 +1169,8 @@ reused in custom filters:
symbolic links.

- :ref:`Refuse <tarfile-extraction-refuse>` to extract links (hard or soft)
that link to absolute paths, or ones that link outside the destination.
that link to absolute paths or to paths with a drive,
or ones that link outside the destination.

This raises :class:`~tarfile.AbsoluteLinkError` or
:class:`~tarfile.LinkOutsideDestinationError`.
Expand Down Expand Up @@ -1198,6 +1204,10 @@ reused in custom filters:

Link targets are now normalized.

.. versionchanged:: next

Link targets with a drive that are not absolute are now refused.


.. _tarfile-extraction-refuse:

Expand Down
13 changes: 7 additions & 6 deletions Lib/tarfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -839,10 +839,6 @@ def _get_filtered_attrs(member, dest_path, for_data=True):
# Include os.sep (target OS directory separator) as well.
if name.startswith(('/', os.sep)):
name = new_attrs['name'] = member.path.lstrip('/' + os.sep)
if os.path.isabs(name):
# Path is absolute even after stripping.
# For example, 'C:/foo' on Windows.
raise AbsolutePathError(member)
# Ensure we stay in the destination
if '..' in name.replace(os.sep, '/').split('/'):
# Directories are created from the name as given, so a name that
Expand All @@ -851,6 +847,11 @@ def _get_filtered_attrs(member, dest_path, for_data=True):
normalized = os.path.normpath(name)
if normalized != name:
name = new_attrs['name'] = normalized
if os.path.isabs(name) or os.path.splitdrive(name)[0]:
# Path is absolute even after stripping, or has a drive.
# For example, 'C:/foo' or 'C:foo' on Windows.
# os.path.join() ignores a destination without a drive for 'C:foo'.
raise AbsolutePathError(member)
target_path = os.path.realpath(os.path.join(dest_path, name),
strict=os.path.ALLOW_MISSING)
if os.path.commonpath([target_path, dest_path]) != dest_path:
Expand Down Expand Up @@ -894,11 +895,11 @@ def _get_filtered_attrs(member, dest_path, for_data=True):
new_attrs['gname'] = None
# Check link destination for 'data'
if member.islnk() or member.issym():
if os.path.isabs(member.linkname):
raise AbsoluteLinkError(member)
normalized = os.path.normpath(member.linkname)
if normalized != member.linkname:
new_attrs['linkname'] = normalized
if os.path.isabs(normalized) or os.path.splitdrive(normalized)[0]:
raise AbsoluteLinkError(member)
if member.issym():
# The symlink is created at `name` with trailing separators
# stripped, so its target is relative to the directory
Expand Down
42 changes: 42 additions & 0 deletions Lib/test/test_tarfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import sys
import os
import io
import ntpath
from hashlib import sha256
from contextlib import contextmanager, ExitStack
from random import Random
Expand Down Expand Up @@ -4298,6 +4299,47 @@ def test_absolute(self):
tarfile.AbsolutePathError,
"""['"].*escaped.evil['"] has an absolute path""")

def test_drive_relative(self):
# Test names and link targets that have a drive but no root.
# They are not absolute, but os.path.join() does not put them
# below a destination that has no drive or another drive.
# Windows path handling is used on all platforms.
dest = r'C:\outerdir\dest'
with support.swap_attr(os, 'path', ntpath):
for name in 'C:escaped.evil', 'x/../C:escaped.evil':
for filter in tarfile.tar_filter, tarfile.data_filter:
with self.subTest(name=name, filter=filter.__name__):
with self.assertRaises(tarfile.AbsolutePathError):
filter(tarfile.TarInfo(name), dest)

for linkname in 'C:escaped.evil', 'x/../C:escaped.evil', './C:evil':
for type in tarfile.SYMTYPE, tarfile.LNKTYPE:
with self.subTest(linkname=linkname, type=type):
member = tarfile.TarInfo('link')
member.type = type
member.linkname = linkname
with self.assertRaises(tarfile.AbsoluteLinkError):
tarfile.data_filter(member, dest)

member = tarfile.TarInfo('x/../C/benign.txt')
member = tarfile.data_filter(member, dest)
self.assertEqual(member.name, r'C\benign.txt')

@unittest.skipUnless(os.name == 'nt', 'requires Windows drives')
def test_drive_relative_to_cwd(self):
# Test a name like 'C:escaped.evil' with a destination that has no
# drive. The file must not be created in the current directory.
with os_helper.temp_cwd() as cwd:
drive = os.path.splitdrive(cwd)[0]
with ArchiveMaker() as arc:
arc.add(f'{drive}escaped.evil')

for filter in 'tar', 'data':
with self.subTest(filter=filter), arc.open() as tar:
with self.assertRaises(tarfile.AbsolutePathError):
tar.extractall('dest', filter=filter)
self.assertFalse(os.path.lexists('escaped.evil'))

def test_parent_dir_out_and_back(self):
# Test a member that leaves the destination and comes back.
# The containment check looks at the resolved path, which stays
Expand Down

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please record the CVE metadata in the standard format, rather than in the prose?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where do we describe what form should be used?

Our docs compilation accepted the :cve: ReST tag so I assumed that counted. is that not considered "the standard format"? it appears elsewhere in our docs.

can we make forms we don't want fail to pass CI? prevention is the best medicine.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant the:

.. cve: xxx

at the top of the file. That way it’s rendered beside the issue number in the changelog.

It would be a little hard to verify in the CI, there may be false positives…

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
On Windows, the ``'tar'`` and ``'data'`` :mod:`tarfile` extraction filters
now refuse member names and link targets that have a drive but are not
absolute, such as ``C:foo``, with :exc:`~tarfile.AbsolutePathError` or
:exc:`~tarfile.AbsoluteLinkError`. Such members could be extracted outside
the destination directory. Fixes :cve:`2026-107784`.
Loading