fix(ggml): detect embedded ZIP polyglot payloads#1780
Conversation
mldangelo-oai
added
codex
security
Performance BenchmarksCompared
|
There was a problem hiding this comment.
Pull request overview
Closes a security bypass in the GGUF/GGML scanner by ensuring legacy GGML variants also trigger the existing ZIP-polyglot preflight and embedded-archive inspection path, with regressions to prevent recurrence.
Changes:
- Extend GGUF ZIP polyglot inspection helper to support GGML context labeling and invoke it from GGML scans.
- Add regression tests covering embedded ZIP polyglots across all legacy GGML magic variants plus an invalid-ZIP near match.
- Document the security fix in the Unreleased changelog.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
modelaudit/scanners/gguf_scanner.py |
Parameterizes ZIP polyglot detection labeling and enables it for GGML scans. |
tests/scanners/test_gguf_scanner.py |
Adds malicious polyglot regressions for GGML variants and a benign invalid-ZIP near-match test. |
CHANGELOG.md |
Notes the GGML ZIP/pickle inspection security hardening under Unreleased. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8ee962c4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| """Basic GGML file validation with security checks.""" | ||
| result.metadata["format"] = "ggml" | ||
| result.metadata["magic"] = magic.decode("ascii", "ignore") | ||
| self._scan_zip_polyglot(result, format_name="GGML") |
There was a problem hiding this comment.
When a compressed ZIP member expands beyond the outer file size, this call merges the member scan's bytes_scanned, but _scan_ggml later replaces it with file_size. In a directory containing several GGML polyglots whose individual uncompressed totals are below max_total_size, aggregate accounting can therefore remain below --max-size and continue scanning past the safety budget; retain at least the merged count, as _scan_gguf does, and cover this budget outcome.
AGENTS.md reference: AGENTS.md:L137-L137
Useful? React with 👍 / 👎.
| """Basic GGML file validation with security checks.""" | ||
| result.metadata["format"] = "ggml" | ||
| result.metadata["magic"] = magic.decode("ascii", "ignore") | ||
| self._scan_zip_polyglot(result, format_name="GGML") |
There was a problem hiding this comment.
When a ZIP member is itself a GGUF/GGML model, merge_executable_zip_container_findings() merges that child's metadata into result; because the outer format and magic are assigned before this call, the child can replace them. For example, a GGMF polyglot containing nested.gguf is reported with format == "gguf", while a nested GGML variant can replace the outer magic. Move the outer identity assignments after the merge or restore them so reports describe the scanned outer artifact.
Useful? React with 👍 / 👎.
Summary
Validation
The broader scanner run also reaches one unrelated compressed-wrapper cache baseline failure, and full local macOS mypy reports existing out-of-scope platform/type errors; current-main GitHub Python CI and Type Check are green.