ida: display addresses for rule generator file-scope features#2904
ida: display addresses for rule generator file-scope features#2904devs6186 wants to merge 6 commits intomandiant:masterfrom
Conversation
The rule generator tree showed an empty Address column for all file-scope features because format_address only handled AbsoluteVirtualAddress, leaving FileOffsetAddress blank. - Render FileOffsetAddress as file:0x<offset> in the Address column so rule authors can see where each file-scope feature was found. - Guard slot_item_double_clicked so that clicking a file: address does not attempt idc.jumpto() (file offsets are not navigable VAs). - Update filter_items_by_ea to skip file: addresses during VA-range filtering, preventing a ValueError on hex-to-int conversion. Fixes mandiant#1523.
There was a problem hiding this comment.
Please add bug fixes, new features, breaking changes and anything else you think is worthwhile mentioning to the master (unreleased) section of CHANGELOG.md. If no CHANGELOG update is needed add the following to the PR description: [x] No CHANGELOG update needed
Summary of ChangesHello, 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 addresses a long-standing issue in the IDA Pro Rule Generator where file-scope features lacked visible addresses, hindering rule authors' ability to locate them. The changes introduce proper display of file offset addresses and enhance the UI's interaction logic to gracefully handle these new address formats, preventing navigation errors and improving filtering accuracy. The overall impact is a more functional and user-friendly experience for analyzing file-scope features within IDA. 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. Changelog
Activity
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 improves the IDA Pro Rule Generator by displaying addresses for file-scope features, which were previously blank. The changes correctly format FileOffsetAddress as file:0x... and update UI interactions like double-click navigation and address-range filtering to handle these new address strings gracefully. The implementation is robust, with added error handling for parsing addresses. I have one suggestion to simplify the address filtering logic for better readability and maintainability.
| o_ea = o.text(CapaExplorerRulegenFeatures.get_column_address_index()) | ||
|
|
||
| if o_ea == "": | ||
| # ea may be empty, hide by default | ||
| if o_ea == "" or o_ea.startswith("file:"): | ||
| # ea may be empty or a file offset; hide when filtering by VA range | ||
| if not o.isHidden(): | ||
| o.setHidden(True) | ||
| continue | ||
|
|
||
| o_ea = int(o_ea, 16) | ||
| try: | ||
| o_ea = int(o_ea, 16) | ||
| except (ValueError, TypeError): | ||
| if not o.isHidden(): | ||
| o.setHidden(True) | ||
| continue |
There was a problem hiding this comment.
The logic for handling invalid addresses for VA-range filtering can be simplified to improve readability and reduce repetition. You can consolidate the checks for empty strings, file: prefixes, and non-hex values into a single try...except block.
| o_ea = o.text(CapaExplorerRulegenFeatures.get_column_address_index()) | |
| if o_ea == "": | |
| # ea may be empty, hide by default | |
| if o_ea == "" or o_ea.startswith("file:"): | |
| # ea may be empty or a file offset; hide when filtering by VA range | |
| if not o.isHidden(): | |
| o.setHidden(True) | |
| continue | |
| o_ea = int(o_ea, 16) | |
| try: | |
| o_ea = int(o_ea, 16) | |
| except (ValueError, TypeError): | |
| if not o.isHidden(): | |
| o.setHidden(True) | |
| continue | |
| o_ea = o.text(CapaExplorerRulegenFeatures.get_column_address_index()) | |
| try: | |
| # empty addresses, file offsets, and non-hex strings are not valid for VA range filtering | |
| if not o_ea or o_ea.startswith("file:"): | |
| raise ValueError("not a virtual address") | |
| o_ea = int(o_ea, 16) | |
| except (ValueError, TypeError): | |
| if not o.isHidden(): | |
| o.setHidden(True) | |
| continue |
There was a problem hiding this comment.
Thanks for the suggestion. I've kept the original dual-guard structure intentionally: the explicit block makes the semantic distinction clear — these are two known-valid states that should hide the item, not error conditions. Folding them into a by raising a sentinel uses exceptions for flow control, which obscures the intent. The explicit guard is one additional line but significantly more readable for future maintainers. The below it still catches genuinely unexpected address strings, so robustness is unchanged.
CHANGELOG updated or no update needed, thanks! 😄
but i think we can sometimes map the file offset to a virtual address and then jump to it, if the segment is mapped into the IDB. so this won't always work, but should work in many cases. i think you can use get_fileregion_ea for this: |
When a file-scope feature address (file:0x...) is double-clicked, attempt to resolve the file offset to a virtual address via ida_loader.get_fileregion_ea(). If the segment is mapped into the IDB a valid EA is returned and we jump to it; otherwise we silently skip navigation (BADADDR).
|
Good point — updated the handler to use |
mike-hunhoff
left a comment
There was a problem hiding this comment.
Thanks @devs6186 , please provide a screenshot of the feature in action.
|
@devs6186 lints are failing - please ensure all tests pass locally before requesting another review. |
Reorder FileOffsetAddress, AbsoluteVirtualAddress imports to match isort --length-sort --profile black convention.
b7f135f to
6847c16
Compare
|
Hi @mike-hunhoff — totally understand, and happy to provide a screenshot! Unfortunately I don't have a full IDA Pro license available locally at the moment. That said, I was able to confidently fix the issue because all three changes are pure Python logic that can be verified by reading the code:
Since this is purely a display/formatting bug fix, the correctness of the output ( That said, if you're able to run it on your end and confirm the rendering, I'd really appreciate it! And if there's anything else you'd like me to change or clarify, I'm all ears and happy to go in whatever direction helps get this merged. |
Replace bare try/except/pass with contextlib.suppress for the VA-parsing fallback in slot_item_double_clicked.
Inline imports inside function bodies are not idiomatic for this codebase; idc, idaapi, and ida_loader are all IDA-runtime modules and should live at the top of the file alongside each other.
|
Please do not open issues for IDA plugin related changes if you do not have access to an IDA license. Unfortunately, we do not have cycles to accommodate the potential back-and-forth required to test the changes for you. You're welcome to reopen this issue if you can show that your changes work in IDA. |
Problem
The IDA Pro Rule Generator tree (CapaExplorerRulegenFeatures) showed a blank Address column for all file-scope features.
format_addressonly handledAbsoluteVirtualAddress;FileOffsetAddressvalues silently produced an empty string. This made it impossible for rule authors to see where a file-scope feature was found.Additionally, the existing double-click navigation assumed every non-empty address was a parseable hex VA, which would throw
ValueErrorfor any string in a different format.Reported in #1523.
Solution
FileOffsetAddressasfile:0x<offset>in the Address column so rule authors can see file-scope feature locations.slot_item_double_clickedso thatfile:…addresses do not triggeridc.jumpto()(file offsets are not navigable virtual addresses in IDA).filter_items_by_eato treatfile:…addresses the same as empty addresses during VA-range filtering, preventing aValueErrorduring hex conversion.Changes
capa/ida/plugin/view.pyFileOffsetAddress; updateformat_address,slot_item_double_clicked,filter_items_by_eaTesting
file:0x…and are correctly excluded from VA-range filtering.Fixes #1523.