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 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