Merge pull request #989 from thunderstore-io/improve-validation

Bolster zip file validation

django/thunderstore/repository/package_upload.py

@@ -16,6 +16,10 @@
 from thunderstore.repository.validation.icon import validate_icon
 from thunderstore.repository.validation.manifest import validate_manifest
 from thunderstore.repository.validation.markdown import validate_markdown
+from thunderstore.repository.validation.zip import (
+    check_relative_paths,
+    check_zero_offset,
+)
 
 MAX_PACKAGE_SIZE = 1024 * 1024 * settings.REPOSITORY_MAX_PACKAGE_SIZE_MB
 MIN_PACKAGE_SIZE = 1  # Honestly impossible, but need to set some value
@@ -123,6 +127,14 @@ def clean_file(self):
                 if unzip.testzip():
                     raise ValidationError("Corrupted zip file")
 
+                if check_relative_paths(unzip.infolist()):
+                    raise ValidationError("Relative paths inside a zip are not allowed")
+
+                if not check_zero_offset(unzip.infolist()):
+                    raise ValidationError(
+                        "The zip includes bogus data at the beginning of the file."
+                    )
+
                 try:
                     manifest = unzip.read("manifest.json")
                     self.validate_manifest(manifest)

django/thunderstore/repository/validation/tests/test_zip.py

@@ -0,0 +1,36 @@
+from io import BytesIO
+from zipfile import ZipFile
+
+import pytest
+
+from thunderstore.repository.validation.zip import (
+    check_relative_paths,
+    check_zero_offset,
+)
+
+
+@pytest.mark.parametrize(
+    ("path", "expected"),
+    (("foo.txt", False), ("foo/../foo.txt", True), ("../foo.txt", True)),
+)
+def test_zip_check_relative_paths(path: str, expected: bool):
+    buffer = BytesIO()
+
+    with ZipFile(buffer, "w") as zf:
+        zf.writestr(path, "foo")
+
+    with ZipFile(buffer, "r") as zf:
+        assert check_relative_paths(zf.infolist()) is expected
+
+
+@pytest.mark.parametrize(
+    ("bogus_bytes", "expected"), ((0, True), (1000, False), (1, False))
+)
+def test_zip_check_zero_offset(bogus_bytes: int, expected: bool):
+    buffer = BytesIO(b"a" * bogus_bytes)
+
+    with ZipFile(buffer, "a") as zf:
+        zf.writestr("bar.txt", "foo")
+
+    with ZipFile(buffer, "r") as zf:
+        assert check_zero_offset(zf.infolist()) is expected

django/thunderstore/repository/validation/zip.py

@@ -0,0 +1,16 @@
+from typing import List
+from zipfile import ZipInfo
+
+
+def check_relative_paths(infolist: List[ZipInfo]) -> bool:
+    for entry in infolist:
+        if entry.filename.startswith("..") or "/.." in entry.filename:
+            return True
+    return False
+
+
+def check_zero_offset(infolist: List[ZipInfo]) -> bool:
+    for entry in infolist:
+        if entry.header_offset == 0:
+            return True
+    return False