loader: handle SegmentationViolation for malformed ELF files#2799
loader: handle SegmentationViolation for malformed ELF files#2799mike-hunhoff merged 3 commits intomandiant:masterfrom
Conversation
Summary of ChangesHello @kami922, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the loader's resilience by introducing specific exception handling for Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request effectively addresses a crash when processing malformed ELF files by introducing specific exception handling for envi.exc.SegmentationViolation. The change correctly catches this low-level error and raises a more user-friendly CorruptFile exception, improving the tool's robustness and error reporting. The implementation is clean, follows best practices by chaining exceptions, and includes a corresponding update to the changelog. This is a great improvement.
mike-hunhoff
left a comment
There was a problem hiding this comment.
Thank you @kami922. Please see my review and address the requested changes.
- Move import envi.exc to align with viv_utils lazy imports - Update CHANGELOG to reference PR mandiant#2799 instead of issue mandiant#2794 Addresses feedback from @mike-hunhoff
546a672 to
7133762
Compare
mike-hunhoff
left a comment
There was a problem hiding this comment.
Thanks @kami922 , one last change for your review.
|
Also, @kami922 please sync with the master branch. |
Catch envi.exc.SegmentationViolation raised by vivisect when parsing malformed ELF files with invalid relocations and convert to a user-friendly CorruptFile error message. Follows existing error handling pattern in get_workspace(). Refs mandiant#2799
Catch envi.exc.SegmentationViolation raised by vivisect when parsing malformed ELF files with invalid relocations and convert to a user-friendly CorruptFile error message. Follows existing error handling pattern in get_workspace(). Refs mandiant#2799
|
@mike-hunhoff i had accidently pushed 2nd last commit to wrong branch please check and lemme know |
- Remove old duplicate CHANGELOG entry that was in the wrong version section - Keep only the correct entry in master (unreleased) section with PR mandiant#2799 - Ensure import envi.exc is in lazy import section, not at top level - All pre-commit checks (isort, black, ruff) pass
- CHANGELOG.md: kept both PR mandiant#2800, mandiant#2799, and mandiant#2802 entries - tests/fixtures.py: resolved duplicate and formatting conflicts - .github/workflows/tests.yml: resolved formatting conflict - tests/data: accepted submodule deletion
|
@mike-hunhoff can I please get an update on my PRs I will then shift to CI workflow issue after at least 1 is merged to avoid confusion in future |
|
see the comment regarding the changelog, is there also a test file and test you could add for this? |
Catch envi.exc.SegmentationViolation raised by vivisect when parsing malformed ELF files with invalid relocations and convert to a user-friendly CorruptFile error message. Follows existing error handling pattern in get_workspace(). Refs mandiant#2799
9ad1fa3 to
811999e
Compare
|
@mike-hunhoff @williballenthin @mr-tz Hello same issue in ci pipeline here as on my other 2 pr test_fix262 in tests/test_main.py is failing and completely unrelated to my pr can you please direct me on how to solve this? The CI is showing one failing test (test_fix262 in tests/test_main.py), but this failure is unrelated to the changes in this PR. The failure is caused by upstream rules changes (capa-rules#1099) that made the "send HTTP request" rule more strict to fix false positives. The test expectations haven't been updated yet to match the new rule behavior.
I've verified this locally - the test fails with the same error after rebasing onto latest master with updated submodules. |
Catch envi.exc.SegmentationViolation raised by vivisect when processing malformed ELF files with invalid relocations and convert it to a CorruptFile exception with a descriptive message. Closes mandiant#2794
1c947c4 to
dd5bd8f
Compare
mike-hunhoff
left a comment
There was a problem hiding this comment.
Thanks @kami922 , tests are passing after merging upstream. LGTM 🚀
| with patch("viv_utils.getWorkspace") as mock_workspace: | ||
| mock_workspace.side_effect = envi.exc.SegmentationViolation( | ||
| 0x30A4B8BD60, | ||
| ) | ||
|
|
||
| with pytest.raises(CorruptFile, match="Invalid memory access"): | ||
| get_workspace(fake_path, FORMAT_ELF, []) |
Fixes #2794
When processing malformed ELF files with invalid relocations, vivisect raises envi.exc.SegmentationViolation during relocation processing. This change catches these exceptions and converts them to CorruptFile exceptions with helpful error messages instead of crashing with a traceback.
Summary
When processing malformed ELF files with invalid relocations, vivisect raises
envi.exc.SegmentationViolationduring relocation processing. This PR catches these exceptions and converts them toCorruptFileexceptions with helpful error messages instead of crashing with a traceback.Changes
envi.exc.SegmentationViolationinget_workspace()CorruptFileexception