Skip to content

Commit

Permalink
Fix hardening measure in release extraction (freebsd#55)
Browse files Browse the repository at this point in the history
In freebsd#49 a hardening measure was imported from
truenas/iocage#358. This hardening measure limits what
can be extracted (location and attributes). It is implemented by
applying the 'tar' filter from tarfile. That filter does this[0]:

- Strip leading slashes (/ and os.sep) from filenames.
- 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 AbsolutePathError.
- Refuse to extract files whose absolute path (after following symlinks)
  would end up outside the destination. This raises
  OutsideDestinationError.
- Clear high mode bits (setuid, setgid, sticky) and group/other write
  bits (S_IWGRP | S_IWOTH).

While the first three modifications are desirable, the last one
damages the extracted release image, as things like sticky bits and
world writable files are required by a proper FreeBSD (jail)
installation.

Fixes freebsd#54

[0]https://docs.python.org/3/library/tarfile.html#tarfile-extraction-filter
  • Loading branch information
grembo authored Dec 9, 2024
1 parent 71ef181 commit a5066c6
Showing 1 changed file with 24 additions and 4 deletions.
28 changes: 24 additions & 4 deletions iocage_lib/ioc_fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,29 @@
from iocage_lib.pools import Pool
from iocage_lib.dataset import Dataset

# deliberately crash if tarfile doesn't have required filter
tarfile.tar_filter

# taken from tarfile.tar_filter (and _get_filtered_attrs)
# basically the same, but **without**:
# - Clear high mode bits (setuid, setgid, sticky) and
# group/other write bits (S_IWGRP | S_IWOTH).
def untar_release_filter(member, dest_path):
new_attrs = {}
name = member.name
dest_path = os.path.realpath(dest_path)
# Strip leading / (tar's directory separator) from filenames.
# 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 tarfile.AbsolutePathError(member)
# Ensure we stay in the destination
target_path = os.path.realpath(os.path.join(dest_path, name))
if os.path.commonpath([target_path, dest_path]) != dest_path:
raise tarfile.OutsideDestinationError(member, target_path)
if new_attrs:
return member.replace(**new_attrs, deep=False)
return member

class IOCFetch:

Expand Down Expand Up @@ -820,7 +840,7 @@ def fetch_extract(self, f):
# removing them first.
member = self.__fetch_extract_remove__(f)
member = self.__fetch_check_members__(member)
f.extractall(dest, members=member, filter='tar')
f.extractall(dest, members=member, filter=untar_release_filter)

def fetch_update(self, cli=False, uuid=None):
"""This calls 'freebsd-update' to update the fetched RELEASE."""
Expand Down

0 comments on commit a5066c6

Please sign in to comment.