feat(security): detect CVE-2024-27318 ONNX nested path traversal bypass#607
Merged
yash2998chhabria merged 15 commits intomainfrom Mar 4, 2026
Merged
feat(security): detect CVE-2024-27318 ONNX nested path traversal bypass#607yash2998chhabria merged 15 commits intomainfrom
yash2998chhabria merged 15 commits intomainfrom
Conversation
Contributor
|
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 nested and direct ONNX external-data path traversal detection (CVE-2024-27318, CVE-2022-25882), a Keras ZIP unsafe-deserialization bypass check (CVE-2025-9906), new tests, a containment helper, and changelog updates; also adds a duplicated explanation function in explanations.py. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@modelaudit/config/explanations.py`:
- Around line 673-702: The get_cve_2024_27318_explanation function is dead
code—defined but never used—so either delete it or make the scanner use it for
consistent explanations; to fix, remove get_cve_2024_27318_explanation from
modelaudit/config/explanations.py if you want to clean up unused code, or modify
the ONNX scanner (the _check_external_data method in onnx_scanner.py) to import
and call get_cve_2024_27318_explanation(vulnerability_type) instead of the
current hardcoded inline descriptions, ensuring the function name and the
vulnerability_type keys ("nested_traversal", "path_traversal", "onnx_version")
match the scanner’s reported vulnerability types.
In `@modelaudit/scanners/onnx_scanner.py`:
- Line 273: The current containment check using
str(external_path).startswith(str(model_dir)) (assigning escapes_model_dir) is
brittle; update it to resolve paths and use a robust containment test such as:
resolve both external_path and model_dir to absolute paths and then use
Path.is_relative_to (or fallback to comparing os.path.commonpath) to determine
containment. Replace the startswith-based assignment for escapes_model_dir with
a resolved-path check using external_path and model_dir (e.g.,
Path(external_path).resolve().is_relative_to(Path(model_dir).resolve()) or the
commonpath fallback) so paths like /foo/barbaz are not misclassified as inside
/foo/bar.
ℹ️ 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/onnx_scanner.pytests/conftest.pytests/scanners/test_onnx_scanner.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@modelaudit/scanners/onnx_scanner.py`:
- Around line 281-285: The raw substring check has_traversal_raw = ".." in
location produces false positives; instead detect actual path-segment traversal
by splitting the path into segments and checking for any segment equal to ".."
(e.g., compute has_traversal_raw = any(part == ".." for part in
Path(location).parts) using pathlib.Path or PurePath), then proceed with the
existing escapes_model_dir check via _is_contained_in(external_path, model_dir);
update imports if needed and replace the substring-based logic in the block
containing has_traversal_raw, _is_contained_in, location, external_path, and
model_dir.
ℹ️ 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 (1)
modelaudit/scanners/onnx_scanner.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@modelaudit/scanners/onnx_scanner.py`:
- Around line 281-336: The scanner currently flags any location containing '..'
(has_traversal_raw) even when the resolved external_path remains inside
model_dir, causing false positives and CVE misattribution; change the logic to
only report a traversal CVE when escapes_model_dir is true (i.e., external_path
is actually outside model_dir), keep has_traversal_raw for diagnostics only, and
refine the direct-vs-nested classification by normalizing the location (strip
leading './' and use path.parts) to detect whether the first path component is
'..' (direct traversal) versus '..' appearing later (nested traversal); update
the block around variables
has_traversal_raw/escapes_model_dir/external_path/model_dir and the
result.add_check calls (including tensor.name) accordingly and add a regression
test case asserting that 'subdir/../weights.bin' does not produce a critical
traversal finding.
ℹ️ 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 (1)
modelaudit/scanners/onnx_scanner.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/scanners/test_onnx_scanner.py`:
- Around line 180-193: Add a short clarifying comment in the
test_normalized_in_dir_path_with_dotdot_no_traversal_flag function explaining
the setup: that weights.bin is created under tmp_path but create_onnx_model is
called with external=True and external_path="subdir/../weights.bin" plus
missing_external=True to simulate an external reference that resolves inside the
model directory; mention why both creating the file and using
missing_external=True are used (to ensure the scanner sees an external reference
while the actual file exists in-dir) so future maintainers understand the intent
around OnnxScanner().scan and create_onnx_model parameters.
ℹ️ 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 (2)
modelaudit/scanners/onnx_scanner.pytests/scanners/test_onnx_scanner.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@modelaudit/scanners/keras_zip_scanner.py`:
- Around line 267-280: The current broad token-based detection (using
_collect_string_tokens, has_enable_unsafe and has_keras_config_context) can
false-positive across unrelated entries; replace it with a scoped recursive
check (e.g. implement a helper like _has_unsafe_deserialization_reference) that
walks the model_config structure (dicts/lists), inspects string values within
the same object entry, and returns True only when it finds either a
fully-qualified reference ("keras.config.enable_unsafe_deserialization" or
"keras.src.config.enable_unsafe_deserialization") or a combination in the same
dict of "enable_unsafe_deserialization" together with "keras.config" /
"keras.src.config"; then call that helper on model_config instead of the current
token-wide any(...) logic to ensure matches are correlated to the same object.
In `@tests/scanners/test_keras_zip_scanner.py`:
- Around line 425-431: The helper _make_keras_zip has an untyped tmp_path
parameter; add a type hint (use pathlib.Path) to tmp_path (e.g., tmp_path:
pathlib.Path or Path) and ensure Path is imported where tests live so the
signature becomes def _make_keras_zip(self, config_str: str, tmp_path: Path) ->
str: to satisfy repo typing rules while leaving behavior unchanged.
ℹ️ 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 (6)
CHANGELOG.mdmodelaudit/config/explanations.pymodelaudit/scanners/keras_zip_scanner.pytests/conftest.pytests/scanners/test_keras_zip_scanner.pytests/scanners/test_onnx_scanner.py
yash2998chhabria
force-pushed
the
feat/cve-2024-27318-detection
branch
from
7115d66 to
7f02fc8
Compare
CVE-2024-27318 bypasses the CVE-2022-25882 fix (which used lstrip) via nested path traversal like "subdir/../../etc/passwd" that starts with a legitimate directory name. - Check raw location for ".." BEFORE file existence to catch traversal even for non-existent targets (fixes logic bug in check ordering) - Attribute nested traversal to CVE-2024-27318, direct to CVE-2022-25882 - Add get_cve_2024_27318_explanation() with nested_traversal/onnx_version - Add 5 tests: nested detection, direct attribution, non-existent target, safe data, CWE/CVSS details Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace str().startswith() with proper Path.relative_to() to prevent path containment bypass where /tmp/model_evil would incorrectly match /tmp/model via string prefix comparison. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace substring check with segment splitting to avoid false positives on legitimate filenames like "..hidden" or "file..backup". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ion config bypass config.json in .keras archives can reference enable_unsafe_deserialization to disable safe_mode from within the loading process, then load malicious Lambda layers. Adds raw string scanning of config.json content. - Add _check_unsafe_deserialization_bypass() to keras_zip_scanner - Flag enable_unsafe_deserialization in config as CRITICAL - Add explanation function for CVE-2025-9906 - 4 new tests (detection, nested value, no false positive, attribution) Co-Authored-By: Claude <noreply@anthropic.com>
Replace `not c.passed` with `c.status == CheckStatus.FAILED` in test_onnx_scanner.py (two occurrences) and import CheckStatus from modelaudit.scanners.base. The Check model has no `passed` attribute; status is tracked via the CheckStatus enum. Co-Authored-By: Claude <noreply@anthropic.com>
The CVE Detection Checklist requires explanation functions in explanations.py for each CVE. This was missing for CVE-2024-27318. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add traversal_files tracking alongside read-CVE checks so the per-file CVE-2025-51480 write-traversal block fires correctly. - Add Path type annotation to _make_keras_zip tmp_path parameter. - Apply ruff formatting fixes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add code comment explaining why unsafe deserialization token detection scans document-wide rather than per-object. This is deliberate: payloads can split indicator tokens across sibling config keys. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
yash2998chhabria
force-pushed
the
feat/cve-2024-27318-detection
branch
from
2ffc87d to
acec47e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
subdir/../../etc/passwd) that evades the lstrip-based CVE-2022-25882 fixget_cve_2024_27318_explanation()for vulnerability contextTest plan
subdir/../../etc/passwd) detected as CVE-2024-27318../../etc/passwd) correctly attributed to CVE-2022-25882weights.bin) not flagged🤖 Generated with Claude Code
Summary by CodeRabbit
Security
Bug Fixes
Tests
Documentation