Skip to content

Commit

Permalink
Improve validation of zip files
Browse files Browse the repository at this point in the history
Fixes #379
  • Loading branch information
rubenwardy committed Jul 8, 2024
1 parent 25da8f5 commit a62f68e
Showing 1 changed file with 30 additions and 4 deletions.
34 changes: 30 additions & 4 deletions app/tasks/importtasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,30 @@ def update_translations(package: Package, tree: PackageTreeNode):
.update(to_update)


def safe_extract_zip(temp_dir: str, archive_path: str) -> bool:
with ZipFile(archive_path, 'r') as zf:
# No more than 256MB
total_size = sum(e.file_size for e in zf.infolist())
if total_size > 256 * 1024 * 1024:
return False

# Check paths
for member in zf.infolist():
# We've had cases of newlines in filenames
if "\n" in member.filename:
return False

file_path = os.path.realpath(os.path.join(temp_dir, member.filename))
if not file_path.startswith(os.path.realpath(temp_dir)):
return False

# Extract all
for member in zf.infolist():
zf.extract(member, temp_dir)

return True


@celery.task(bind=True)
def check_zip_release(self, id, path):
release = PackageRelease.query.get(id)
Expand All @@ -291,8 +315,10 @@ def check_zip_release(self, id, path):
raise TaskError("No package attached to release")

with get_temp_dir() as temp:
with ZipFile(path, 'r') as zip_ref:
zip_ref.extractall(temp)
if not safe_extract_zip(temp, path):
release.approved = False
db.session.commit()
raise Exception(f"Unsafe zip file at {path}")

post_release_check_update(self, release, temp)

Expand All @@ -310,8 +336,8 @@ def import_languages(self, id, path):
raise TaskError("No package attached to release")

with get_temp_dir() as temp:
with ZipFile(path, 'r') as zip_ref:
zip_ref.extractall(temp)
if not safe_extract_zip(temp, path):
raise Exception(f"Unsafe zip file at {path}")

try:
tree: PackageTreeNode = build_tree(temp,
Expand Down

0 comments on commit a62f68e

Please sign in to comment.