Skip to content

Commit

Permalink
FIX: Zip slip directory traversal vulnerability in file handling fun…
Browse files Browse the repository at this point in the history
…ctions (#884)

The "Zip Slip" vulnerability occurs when an application extracts files from an archive without validating their paths, allowing attackers to overwrite arbitrary files on the system.

To fix this I checked the following:

    Stripping directory traversal sequences (e.g., ../) from the filenames.
    Refusing to extract files with absolute paths.

I added method to Sanitize the paths before use in Compress and Extract functions. It ensures ensures that any unexpected directory traversal attempts (such as ../) or adding files to unwanted locations are handled correctly.
  • Loading branch information
Unichronic authored Feb 2, 2025
1 parent e2064b0 commit 8b96626
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 9 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ share/python-wheels/
*.egg
MANIFEST

#Sample files
samples/

# PyInstaller
# Usually these files are written by a python script from a template
# before PyInstaller builds the exe, so as to inject date/other infos into it.
Expand Down
51 changes: 42 additions & 9 deletions invesalius/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

import invesalius.constants as const
from invesalius import inv_paths
from invesalius.gui.dialogs import ErrorMessageBox

# from invesalius.data import imagedata_utils
from invesalius.presets import Presets
Expand Down Expand Up @@ -501,30 +502,53 @@ def Compress(
else:
tar = tarfile.open(temp_inv3, "w")
for name in filelist:
sanit_name = os.path.normpath(filelist[name]) # Sanitizing path
if ".." in sanit_name or os.path.isabs(sanit_name):
continue

tar.add(name, arcname=os.path.join(tmpdir_, filelist[name]))
tar.close()
os.close(fd_inv3)
shutil.move(temp_inv3, filename)
# os.chdir(current_dir)


def custom_tar_filter(file: tarfile.TarInfo, path: Union[str, os.PathLike]):
file.name = os.path.normpath(file.name).lstrip(os.sep)
file_path = os.path.abspath(os.path.join(path, file.name))
if not file_path.startswith(os.path.abspath(path)):
dlg = ErrorMessageBox(
None,
"Security Alert",
f"Potential directory traversal detected: {file.name}. Skipping file extraction.",
)
dlg.ShowModal()
dlg.Destroy()
return None
return file


def Extract(filename: Union[str, bytes, os.PathLike], folder: Union[str, bytes, os.PathLike]):
if _has_win32api:
folder = win32api.GetShortPathName(folder)
folder = decode(folder, const.FS_ENCODE)

tar = tarfile.open(filename, "r")
idir = decode(os.path.split(tar.getnames()[0])[0], "utf8")
os.mkdir(os.path.join(folder, idir))
os.makedirs(os.path.join(folder, idir), exist_ok=True)
filelist = []
tar_filter = getattr(tarfile, "tar_filter", None) # For python < 3.12
tar_filter = getattr(tarfile, "tar_filter", None)
for t in tar.getmembers():
try:
tar.extract(t, path=folder, filter=tar_filter)
except TypeError:
tar.extract(t, path=folder)
fname = os.path.join(folder, decode(t.name, "utf-8"))
filelist.append(fname)
fname = os.path.join(folder, decode(t.name, "utf-8"))
filelist.append(fname)
except (TypeError, AttributeError, tarfile.OutsideDestinationError):
filtered = custom_tar_filter(t, folder)
if filtered is not None:
tar.extract(filtered, path=folder)
fname = os.path.join(folder, decode(filtered.name, "utf-8"))
filelist.append(fname)

tar.close()
return filelist

Expand All @@ -533,8 +557,17 @@ def Extract_(
filename: Union[str, bytes, os.PathLike], folder: Union[str, os.PathLike]
) -> List[str]:
tar = tarfile.open(filename, "r:gz")
filelist = []

for member in tar.getmembers():
member_name = os.path.basename(member.name) # Strip directory path
fname = os.path.join(folder, member_name)

if os.path.commonpath([folder, fname]) == folder:
tar.extract(member, path=folder)
filelist.append(fname)
else:
print(f"Skipping potential unsafe file: {member.name}")
# tar.list(verbose=True)
tar.extractall(folder)
filelist = [os.path.join(folder, i) for i in tar.getnames()]
tar.close()
return filelist

0 comments on commit 8b96626

Please sign in to comment.