From a5066c63f6fd947844f8b57fc66b7c6a58b5d83e Mon Sep 17 00:00:00 2001 From: grembo Date: Mon, 9 Dec 2024 07:25:16 +0100 Subject: [PATCH] Fix hardening measure in release extraction (#55) In freebsd/iocage#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/iocage#54 [0]https://docs.python.org/3/library/tarfile.html#tarfile-extraction-filter --- iocage_lib/ioc_fetch.py | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/iocage_lib/ioc_fetch.py b/iocage_lib/ioc_fetch.py index 9a978f7d..fcf950a3 100644 --- a/iocage_lib/ioc_fetch.py +++ b/iocage_lib/ioc_fetch.py @@ -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: @@ -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."""