fix: cap how much an SLA archive may unpack

.github/workflows/python-tests.yml

@@ -24,7 +24,10 @@ jobs:
     - name: Install dependencies
       run: |
         python -m pip install --upgrade pip
-        pip install -U flake8 ruff pytest pytest-doctestplus pytest-pylint pytest-mypy
+        # The linters are pinned: their rule sets grow between minor
+        # releases, so an unpinned install turns a green build red with no
+        # commit.
+        pip install -U flake8 "ruff==0.15.*" "pylint==3.3.*" pytest pytest-doctestplus pytest-mypy
         pip install -U types-setuptools
         pip install .
     - name: Lint with flake8
@@ -33,9 +36,12 @@ jobs:
     - name: Lint with ruff
       run: |
         ruff check .
-    - name: Lint with pylit
+    - name: Lint with pylint
       run: |
-        pytest -v --mypy --pylint --doctest-plus --doctest-rst gcode_metadata
+        pylint gcode_metadata tests
+    - name: Type check and doctests
+      run: |
+        pytest -v --mypy --doctest-plus --doctest-rst gcode_metadata
     - name: Tests
       run: |
-        pytest -v --mypy --pylint --doctest-plus --doctest-rst tests
+        pytest -v --mypy --doctest-plus --doctest-rst tests

gcode_metadata/metadata.py

@@ -864,6 +864,34 @@ class SLMetaData(MetaData):
     THUMBNAIL_NAME_PAT = re.compile(
         r".*?(?P<dim>\d+x\d+)\.(?P<format>qoi|jpg|png)")
 
+    # Caps on unpacked bytes, against archives that deflate kilobytes into
+    # gigabytes. The thumbnail budget covers all members together, since the
+    # archive picks how many there are. Real files: ~3 kB config, <150 kB
+    # thumbnails.
+    MAX_UNPACKED = 4 * 1024 * 1024
+    MAX_CONFIG = 256 * 1024
+
+    @staticmethod
+    def read_capped(zip_file: zipfile.ZipFile, info: zipfile.ZipInfo,
+                    limit: int) -> bytes:
+        """Read `info`, refusing it if it unpacks to more than `limit` bytes.
+
+        The declared size is only an early reject: zipfile truncates to it
+        after decompressing, so the read itself is what bounds memory. The
+        member is read by `info`, not name, as a name resolves to the last
+        duplicate.
+        """
+        if info.file_size > limit:
+            raise ValueError(
+                f"{info.filename} declares {info.file_size} B, over the "
+                f"remaining {limit} B budget")
+        with zip_file.open(info) as member:
+            data = member.read(limit + 1)
+        if len(data) > limit:
+            raise ValueError(
+                f"{info.filename} unpacks past the {limit} B budget")
+        return data
+
     def set_attr(self, name, value):
         """A helper function that saves attributes to `self.data`"""
         if value is None:
@@ -896,8 +924,8 @@ def load_from_file(self, path: str):
 
         self.thumbnails = self.extract_thumbnails(path)
 
-    @staticmethod
-    def extract_metadata(path: str) -> Dict[str, str]:
+    @classmethod
+    def extract_metadata(cls, path: str) -> Dict[str, str]:
         """Extract metadata from `path`.
 
         :param path: zip file
@@ -910,26 +938,29 @@ def extract_metadata(path: str) -> Dict[str, str]:
         with zipfile.ZipFile(path, "r") as zip_file:
             if file_name not in zip_file.namelist():
                 return data
-            data = json.loads(zip_file.read(file_name))
+            info = zip_file.getinfo(file_name)
+            data = json.loads(cls.read_capped(zip_file, info, cls.MAX_CONFIG))
         return data
 
-    @staticmethod
-    def extract_thumbnails(path: str) -> Dict[str, bytes]:
+    @classmethod
+    def extract_thumbnails(cls, path: str) -> Dict[str, bytes]:
         """Extract thumbnails from `path`.
 
         :param path: zip file
         :returns Dictionary with thumbnail dimensions as key and base64
             encoded image as value.
         """
         thumbnails: Dict[str, bytes] = {}
+        budget = cls.MAX_UNPACKED
         with zipfile.ZipFile(path, "r") as zip_file:
             for info in zip_file.infolist():
                 if info.filename.startswith("thumbnail/"):
-                    match = SLMetaData.THUMBNAIL_NAME_PAT.match(info.filename)
-                    if match:
+                    match = cls.THUMBNAIL_NAME_PAT.match(info.filename)
+                    if match and not info.is_dir():
                         img_format = match.group("format").upper()
                         img_dim = match.group("dim")
-                        data = zip_file.read(info.filename)
+                        data = cls.read_capped(zip_file, info, budget)
+                        budget -= len(data)
                         data = base64.b64encode(data)
                         thumbnails[f"{img_dim}_{img_format}"] = data
         return thumbnails

tests/test_metadata.py

@@ -1,6 +1,9 @@
 """Tests for gcode-metadata tool for g-code files."""
 import json
 import os
+import tracemalloc
+import zipfile
+import zlib
 import tempfile
 import shutil
 from importlib.metadata import version
@@ -10,10 +13,15 @@
 
 from gcode_metadata import (get_metadata, UnknownGcodeFileType, MetaData,
                             get_meta_class)
+from gcode_metadata.metadata import SLMetaData
 
 gcodes_dir = os.path.join(os.path.dirname(os.path.realpath(__file__)),
                           "gcodes")
 
+# Smallest config.json that still yields metadata, for synthetic archives
+SL1_CONFIG = {"printerModel": "SL1S", "layerHeight": 0.05}
+PNG_HEADER = b"\x89PNG\r\n\x1a\n"
+
 # pylint: disable=redefined-outer-name
 
 
@@ -421,3 +429,116 @@ def test_set_attr_same_as_set_data(self):
         for attr, value in raw_meta.items():
             meta_cls_for_set_attr.set_attr(attr, value)
         assert meta_cls_for_set_attr.data == meta_from_set_data.data
+
+
+class TestSLMetaDataBudget:
+    """Refusal of SL archives that unpack past the budget.
+
+    Asserted on the extractors: `MetaData.load` swallows any parse failure.
+    """
+
+    @staticmethod
+    def archive(tmp_dir, declared=None, stored=None):
+        """Build an .sl1 with `stored` bytes that claims `declared` sizes."""
+        path = os.path.join(tmp_dir, "bomb.sl1")
+        declared = declared or {}
+        stored = stored or {}
+        config = json.dumps(SL1_CONFIG)
+        with zipfile.ZipFile(path, "w", zipfile.ZIP_DEFLATED) as archive:
+            archive.writestr("config.json", config)
+            for name in {**stored, **declared}:
+                if name != "config.json":
+                    archive.writestr(name, stored.get(name, PNG_HEADER))
+            # The central directory is written on close with these sizes.
+            for info in archive.infolist():
+                if info.filename in declared:
+                    info.file_size = declared[info.filename]
+        return path
+
+    def test_oversized_thumbnail_is_refused(self, tmp_dir):
+        """A single thumbnail may not exceed the whole budget."""
+        fname = self.archive(
+            tmp_dir,
+            {"thumbnail/thumbnail400x400.png": SLMetaData.MAX_UNPACKED + 1})
+        with pytest.raises(ValueError, match="thumbnail400x400.png"):
+            SLMetaData.extract_thumbnails(fname)
+
+    def test_oversized_config_is_refused(self, tmp_dir):
+        """config.json has a tighter cap of its own."""
+        fname = self.archive(tmp_dir,
+                             {"config.json": SLMetaData.MAX_CONFIG + 1})
+        with pytest.raises(ValueError, match="config.json"):
+            SLMetaData.extract_metadata(fname)
+
+    def test_many_small_thumbnails_exhaust_the_budget(self, tmp_dir):
+        """Members that are individually harmless still add up."""
+        chunk = SLMetaData.MAX_UNPACKED // 4
+        fname = self.archive(tmp_dir,
+                             stored={
+                                 f"thumbnail/t{index}_100x100.png":
+                                 b"\0" * chunk
+                                 for index in range(8)
+                             })
+        with pytest.raises(ValueError, match="budget"):
+            SLMetaData.extract_thumbnails(fname)
+
+    def test_layer_images_are_not_charged(self, tmp_dir):
+        """Layer images are never read, so their size cannot exhaust it."""
+        fname = self.archive(
+            tmp_dir, {"Shape-Box00001.png": SLMetaData.MAX_UNPACKED * 100})
+        assert not SLMetaData.extract_thumbnails(fname)
+        assert SLMetaData.extract_metadata(fname)["printerModel"] == "SL1S"
+
+    def test_a_subclass_may_tighten_the_budget(self, tmp_dir):
+        """A service sets its own ceiling by overriding the attribute."""
+
+        class StrictSL(SLMetaData):
+            """SL metadata for a service with less memory to spare."""
+            MAX_UNPACKED = 1024
+
+        fname = self.archive(tmp_dir, {"thumbnail/thumbnail400x400.png": 2048})
+        assert SLMetaData.extract_thumbnails(fname)
+        with pytest.raises(ValueError, match="thumbnail400x400.png"):
+            StrictSL.extract_thumbnails(fname)
+
+    @staticmethod
+    def peak_of(call):
+        """Return the peak Python allocation, in bytes, made by `call`."""
+        tracemalloc.start()
+        try:
+            call()
+        finally:
+            peak = tracemalloc.get_traced_memory()[1]
+            tracemalloc.stop()
+        return peak
+
+    def test_a_member_lying_downward_cannot_overrun_memory(self, tmp_dir):
+        """A small declared size must not let a huge payload reach memory."""
+        payload = b"\0" * (32 * 1024 * 1024)
+        path = os.path.join(tmp_dir, "bomb.sl1")
+        with zipfile.ZipFile(path, "w", zipfile.ZIP_DEFLATED) as archive:
+            archive.writestr("config.json", json.dumps(SL1_CONFIG))
+            archive.writestr("thumbnail/thumbnail400x400.png", payload)
+            for info in archive.infolist():
+                if info.filename.startswith("thumbnail/"):
+                    info.file_size = 100
+                    info.CRC = zlib.crc32(payload[:100])
+
+        peak = self.peak_of(lambda: SLMetaData.extract_thumbnails(path))
+        assert peak < 12 * 1024 * 1024
+
+    @pytest.mark.filterwarnings("ignore:Duplicate name")
+    def test_a_duplicate_name_cannot_smuggle_a_payload(self, tmp_dir):
+        """The bytes read must come from the member that was checked."""
+        path = os.path.join(tmp_dir, "bomb.sl1")
+        name = "thumbnail/thumbnail400x400.png"
+        with zipfile.ZipFile(path, "w", zipfile.ZIP_DEFLATED) as archive:
+            archive.writestr("config.json", json.dumps(SL1_CONFIG))
+            archive.writestr(name, PNG_HEADER)
+            archive.writestr(name, b"\0" * (16 * 1024 * 1024))
+
+        def refuse():
+            with pytest.raises(ValueError, match="thumbnail400x400.png"):
+                SLMetaData.extract_thumbnails(path)
+
+        assert self.peak_of(refuse) < 8 * 1024 * 1024