fix: remove unsafe exec() in face_swapper.py#1795
Conversation
Automated security fix generated by Orbis Security AI
Reviewer's GuideAdds an integrity-checked ONNX model loading path to mitigate arbitrary native code execution via tampered model files and replaces a debug-only assertion with a runtime error for non-square aligned faces in the face swapper processor. Class diagram for updated face_swapper module structureclassDiagram
class face_swapper_module {
<<module>>
dict _MODEL_SHA256
pre_check() bool
get_face_swapper() Any
_verify_model_integrity(model_path str) bool
_fast_paste_back(target_img Frame, bgr_fake np_ndarray, aimg np_ndarray, face_h int, face_w int)
}
Flow diagram for integrity-checked ONNX model loadingflowchart TD
A[Select or download ONNX model_path] --> B[Call _verify_model_integrity]
B --> C{Is filename in _MODEL_SHA256?}
C -- No --> D[Return True]
C -- Yes --> E[Compute SHA-256 over file chunks]
E --> F{Computed hash == expected hash?}
F -- No --> G[update_status Model integrity check failed]
G --> H[Abort and return None]
F -- Yes --> I[Return True]
D --> J[Proceed to load ONNX model]
I --> J[Proceed to load ONNX model]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider handling I/O errors in
_verify_model_integrity(e.g., missing or unreadable files) and surfacing a clear status message instead of allowing an unhandled exception to propagate fromopen(model_path, "rb").
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider handling I/O errors in `_verify_model_integrity` (e.g., missing or unreadable files) and surfacing a clear status message instead of allowing an unhandled exception to propagate from `open(model_path, "rb")`.
## Individual Comments
### Comment 1
<location path="modules/processors/frame/face_swapper.py" line_range="58-60" />
<code_context>
+
+def _verify_model_integrity(model_path: str) -> bool:
+ """Return True if the file passes SHA-256 verification (or has no registered hash)."""
+ filename = os.path.basename(model_path)
+ expected = _MODEL_SHA256.get(filename)
+ if expected is None:
+ return True
+ sha256 = hashlib.sha256()
</code_context>
<issue_to_address>
**🚨 question (security):** Reconsider treating unlisted model filenames as implicitly trusted to maintain the integrity guarantees.
Because missing entries in `_MODEL_SHA256` are treated as success, any differently named ONNX file can bypass integrity checks. To actually enforce an allowlist of known-good artifacts, consider failing verification for unknown filenames (or making this stricter behavior configurable).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
✅ Review Feedback Addressed I've automatically addressed 1 review comment(s): Two changes are made to
Files modified:
The changes have been pushed to this PR branch. Please review! |
Summary
Fix critical severity security issue in
modules/processors/frame/face_swapper.py.Vulnerability
V-003modules/processors/frame/face_swapper.py:109Description: The application loads ONNX model files from a user-configurable path without performing any cryptographic integrity verification. ONNX runtime deserialization of a crafted model file can execute arbitrary native code via malicious custom operators embedded in the model. The onnx.save() call in onnx_optimize.py also writes model files to a potentially attacker-controlled path, creating a secondary write-path attack vector.
Changes
modules/processors/frame/face_swapper.pyVerification
Automated security fix by OrbisAI Security
Summary by Sourcery
Enforce integrity verification for the face swapper ONNX model and harden runtime behavior when processing aligned faces.
Bug Fixes:
Enhancements: