From 36172cbf785093d7dae531015bbff8f1868b992c Mon Sep 17 00:00:00 2001 From: "sarka.faloutova" Date: Mon, 21 Sep 2026 10:42:25 +0200 Subject: [PATCH 1/2] fix: cap how much an SLA archive may unpack SLMetaData read config.json and every thumbnail/ member with no size limit, so a crafted .sl1/.slx deflating a few hundred kB into gigabytes OOM-killed whatever parsed it. A consumer that parses on a queue then crash-loops: the SIGKILL leaves the message unacked and it is redelivered. extract_metadata and extract_thumbnails now go through read_capped, which reads a member with an explicit byte limit. The declared size in the central directory only serves as an early reject; it bounds nothing by itself, because zipfile truncates to it after the decompressor has already produced the data, and the CRC it would be checked against is written by the archive too. Members are read by ZipInfo rather than by name, since a name resolves to the last duplicate, which lets one entry be checked and another read. config.json is capped at 256 kB. Thumbnails share a 4 MB budget across the whole archive, as a per-member cap multiplies by a member count the archive chooses. Real files spend ~3 kB and under 150 kB respectively. A refusal is a ValueError; directory entries under thumbnail/ are skipped instead of yielding an empty thumbnail. The extractors are classmethods so a subclass can override the caps. Calling them on the class or on an instance works as before. Co-Authored-By: Claude Fable 5.1 --- gcode_metadata/metadata.py | 47 +++++++++++--- tests/test_metadata.py | 121 +++++++++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+), 8 deletions(-) diff --git a/gcode_metadata/metadata.py b/gcode_metadata/metadata.py index a26ed99..17e2e15 100644 --- a/gcode_metadata/metadata.py +++ b/gcode_metadata/metadata.py @@ -864,6 +864,34 @@ class SLMetaData(MetaData): THUMBNAIL_NAME_PAT = re.compile( r".*?(?P\d+x\d+)\.(?Pqoi|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,11 +938,12 @@ 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 @@ -922,14 +951,16 @@ def extract_thumbnails(path: str) -> Dict[str, bytes]: 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 diff --git a/tests/test_metadata.py b/tests/test_metadata.py index 213b0e1..15756ad 100644 --- a/tests/test_metadata.py +++ b/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 From 99de2c9cb9b371f894b2c659e2533b4f3e540044 Mon Sep 17 00:00:00 2001 From: "sarka.faloutova" Date: Mon, 21 Sep 2026 11:02:24 +0200 Subject: [PATCH 2/2] ci: repair the pipeline, unpinned linters had rotted it Two failures that arrived without a commit, because every tool is installed with -U and so drifts with time. Ruff's default rule set grew in 0.16.0 to cover UP, SIM, I, RUF, FA and more, and the repo carries no ruff configuration, so it lints against whatever that default happens to be. master passes on 0.15 and reports 41 findings on 0.16 with nothing changed between them. Ruff is pinned to 0.15.*; adopting the wider set is worth doing, but as a reviewed change rather than one that arrives on its own. pytest-pylint declares the pytest_collect_file(path, parent) hook, which pytest 8 removed, so it fails plugin validation before collection starts and both pytest steps exit 3. Its last release is from 2023 and none supports the current hookspec, so the plugin goes and pylint runs as its own step, pinned for the same reason as ruff. 3.3 rather than 4.0 because 4.0 does not install on Python 3.9, which the matrix still covers. The "Lint with pylit" step gains its missing n and splits: pylint now lints both packages, and the pytest run it used to hide inside keeps the type check and doctests over gcode_metadata. flake8 and mypy are installed the same way and carry the same risk; left alone here to keep this to what is currently failing. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/python-tests.yml | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index fddd79a..bef6b16 100644 --- a/.github/workflows/python-tests.yml +++ b/.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