feat(security): detect CVE-2025-1550 Keras safe_mode bypass#599
feat(security): detect CVE-2025-1550 Keras safe_mode bypass#599yash2998chhabria merged 6 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds CVE-2025-1550 detection for Keras Changes
Sequence DiagramsequenceDiagram
participant User
participant KerasZipScanner
participant ConfigExtractor
participant ModuleChecker
participant ExplanationProvider
participant IssueReporter
User->>KerasZipScanner: scan_model(keras_zip)
KerasZipScanner->>ConfigExtractor: extract config.json from ZIP
ConfigExtractor->>KerasZipScanner: return parsed config (layers)
KerasZipScanner->>ModuleChecker: _check_layer_module_references(layer) for each layer
ModuleChecker->>ModuleChecker: normalize module path, check top-level root
alt dangerous module found
ModuleChecker->>IssueReporter: create CRITICAL issue (CVE-2025-1550)
else untrusted module found
ModuleChecker->>IssueReporter: create WARNING issue (CVE-2025-1550)
else no issue
ModuleChecker->>IssueReporter: no action
end
IssueReporter->>ExplanationProvider: get_cve_2025_1550_explanation(issue_type)
ExplanationProvider->>IssueReporter: return explanation text
IssueReporter->>User: return issues with metadata and explanations
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 8-13: The new "## [Unreleased]" block is duplicated; remove the
extra header and merge the security item ("**keras:** detect CVE-2025-1550
arbitrary module references in `.keras` config.json (CVSS 9.8, safe_mode
bypass)") into the existing "## [Unreleased]" section—add it under the existing
"### Security" subsection (or create that subsection if missing) so all
unreleased notes remain in a single Unreleased block.
In `@modelaudit/scanners/keras_zip_scanner.py`:
- Around line 25-32: The allowlist currently uses plain startswith checks
against _SAFE_KERAS_MODULE_PREFIXES which treats names like "mathutils" as safe;
update the matching so each allowed entry is only accepted if the module name is
exactly equal to the entry or equals the entry followed by a dot (e.g. "math" ->
accept "math" or "math.xxx" but not "mathutils"). Locate
_SAFE_KERAS_MODULE_PREFIXES and every place it is consulted (the import-checking
logic in keras_zip_scanner.py that uses startswith) and replace the naive
startswith test with a combined equality or startswith(prefix + ".") check for
each prefix, ensuring the same fix is applied to the other occurrences
referenced (the later matching block around lines 337-340).
- Around line 320-328: The code currently coerces module fields to strings
(str(layer[key]) / str(layer_config[key])) which turns None/other non-strings
into truthy strings like "None" and causes false-positive CVE matches; update
the logic in the loop that builds module_keys_to_check in keras_zip_scanner.py
to append the original value (e.g., layer[key] and layer_config[key]) without
str() and then, in the subsequent loop over module_keys_to_check (where
module_value is used), validate by checking type and presence (for example: skip
if module_value is None or not isinstance(module_value, str) or
module_value.strip() == "" ) before using it for CVE detection so None and
non-string values are not treated as valid module identifiers.
In `@tests/scanners/test_keras_zip_scanner.py`:
- Around line 422-617: Add two regression tests to
TestCVE20251550ModuleReferences: one that ensures a layer with module set to
None (or missing the "module" key) is treated as untrusted and still scanned for
CVE-2025-1550 (use KerasZipScanner().scan on a .keras produced by
_make_keras_zip and assert an appropriate IssueSeverity, e.g., WARNING/CRITICAL
as per policy), and one that asserts allowlist-prefix collisions are not allowed
(create a layer with "module": "mathutils.ops" and verify it is flagged as
untrusted even though "math" is allowlisted—ensure the scanner logic referenced
by KerasZipScanner.scan treats allowlist matches as exact prefix boundaries, not
simple startswith). Ensure tests reference methods/_make_keras_zip,
KerasZipScanner, and CVE-2025-1550 in their assertions.
ℹ️ Review info
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
CHANGELOG.mdmodelaudit/config/explanations.pymodelaudit/scanners/keras_zip_scanner.pytests/conftest.pytests/scanners/test_keras_zip_scanner.py
…g.json module references Keras < 3.9.0 allows arbitrary code execution even with safe_mode=True by specifying dangerous Python modules in config.json layer module/fn_module keys. Adds allowlist-based detection to keras_zip_scanner that checks ALL layers (not just Lambda) for references outside keras/tensorflow/numpy. - Add _check_layer_module_references() with safe module allowlist - Flag dangerous modules (os, subprocess, builtins, etc.) as CRITICAL - Flag unknown modules outside allowlist as WARNING - Add CVE attribution with CVSS 9.8, CWE-502 details - Add explanation function for CVE-2025-1550 - 9 new tests covering dangerous/untrusted/safe/nested scenarios Co-Authored-By: Claude <noreply@anthropic.com>
- Replace startswith prefix matching with exact root module matching to prevent "mathutils" colliding with safe "math" prefix - Skip None/non-string module values to avoid str(None) -> "None" false positives - Add regression tests for None module and prefix collision Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…9655 - Fix broken get_cve_2025_49655_explanation() closing (missing ), }, return) - Fix duplicate import of get_pattern_explanation in keras_zip_scanner.py - Remove extra argument in get_cve_2025_23304_explanation() dict.get() call - Fix duplicate dict keys from merge in test fixtures - Restore missing CVE-2025-49655 tests to correct test class - Remove misplaced CVE-2025-49655 tests from CVE-2025-1550 test class - Fix CVE-2025-1550 default explanation message - Resolve remaining git conflict markers in test file Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
0012c9f to
e6ef1fd
Compare
Fixes prettier formatting check in CI. Co-Authored-By: Claude <noreply@anthropic.com>
Summary
Model.load_modelallows arbitrary code execution even withsafe_mode=Trueby specifying dangerous Python modules inconfig.jsoninside.kerasarchiveskeras_zip_scannerthat scans ALL layer configs (not just Lambda) formodule/fn_modulekeys referencing modules outsidekeras.*,tensorflow.*,numpy.*os,subprocess,builtins, etc.) as CRITICAL; unknown modules as WARNINGChanges
modelaudit/scanners/keras_zip_scanner.py: Add_check_layer_module_references()with safe module allowlist and dangerous module blocklistmodelaudit/config/explanations.py: Addget_cve_2025_1550_explanation()functiontests/scanners/test_keras_zip_scanner.py: 9 new tests (dangerous/untrusted/safe/nested/attribution)tests/conftest.py: Add keras_zip tests to Python 3.12 allowlistCHANGELOG.md: Add unreleased entryTest plan
uv run modelaudit scan /tmp/malicious_cve1550.keraswithosmodule reference → CRITICAL finding🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Documentation