idalib: discover IDA installation via Desktop .desktop entry files#2907
idalib: discover IDA installation via Desktop .desktop entry files#2907devs6186 wants to merge 2 commits intomandiant:masterfrom
Conversation
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 significantly enhances the robustness of IDA Pro installation discovery within the 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
|
CHANGELOG updated or no update needed, thanks! 😄
There was a problem hiding this comment.
Code Review
The pull request introduces a new fallback mechanism to locate IDA Pro installations by parsing .desktop files in user-writable directories like ~/Desktop when a configuration file is not present. However, the _find_install_dir_from_desktop_entries function, which implements this discovery, has been flagged as a high-severity security vulnerability. It automatically trusts and attempts to execute code paths from potentially malicious .desktop files in user-writable locations, which could lead to arbitrary code execution. Mitigation strategies suggested include avoiding automatic scanning of user-writable directories, implementing stricter validation of file ownership and permissions, or requiring user confirmation before adding discovered paths.
| def _find_install_dir_from_desktop_entries() -> Optional[Path]: | ||
| """ | ||
| Fallback for locating an IDA installation on desktop-based systems. | ||
|
|
||
| On some Linux desktops, IDA may be installed and reachable via a .desktop | ||
| launcher placed directly on the user's Desktop, while idalib has not yet | ||
| been installed (and therefore no ida-config.json exists). | ||
|
|
||
| To better support this workflow we: | ||
| - look for *.desktop files in ~/Desktop | ||
| - parse their Exec= entries | ||
| - treat the directory containing the referenced binary as a candidate | ||
| IDA installation directory | ||
| """ | ||
|
|
||
| # Only attempt this on POSIX desktops; Windows uses APPDATA config only. | ||
| if sys.platform not in {"linux", "linux2", "darwin"}: | ||
| return None | ||
|
|
||
| desktop_dir = Path.home() / "Desktop" | ||
| if not desktop_dir.is_dir(): | ||
| return None | ||
|
|
||
| for entry in sorted(desktop_dir.glob("*.desktop")): | ||
| parser = ConfigParser() | ||
| try: | ||
| # ConfigParser gracefully ignores encoding/section quirks for typical .desktop files. | ||
| parser.read(entry, encoding="utf-8") | ||
| except (OSError, UnicodeDecodeError): | ||
| continue | ||
|
|
||
| if not parser.has_option("Desktop Entry", "Exec"): | ||
| continue | ||
|
|
||
| exec_line = parser.get("Desktop Entry", "Exec", fallback="").strip() | ||
| if not exec_line: | ||
| continue | ||
|
|
||
| # Exec line may contain arguments, e.g. "/opt/ida/ida64 %F" | ||
| command = exec_line.split()[0] | ||
| if not command: | ||
| continue | ||
|
|
||
| cmd_path = Path(command) | ||
| if not cmd_path.is_absolute(): | ||
| # For relative/alias commands (e.g. "ida64"), resolving them reliably | ||
| # requires PATH introspection and is out of scope here. | ||
| continue | ||
|
|
||
| ida_install_dir = cmd_path.parent | ||
| if ida_install_dir.exists(): | ||
| return ida_install_dir | ||
|
|
||
| return None |
There was a problem hiding this comment.
The _find_install_dir_from_desktop_entries function automatically scans user-writable directories like ~/Desktop for *.desktop files and uses the Exec path to locate an IDA installation. This introduces a significant security risk: a malicious .desktop file could lead to arbitrary code execution when capa is run, as it trusts these files without sufficient validation or user confirmation. While using shlex.split() (as suggested for robust parsing of Exec lines) would improve handling of paths with spaces, it does not address the fundamental vulnerability of automatically trusting and executing code paths from arbitrary .desktop files in user-writable locations. To mitigate this, avoid automatically scanning user-writable directories. If discovery is necessary, ensure discovered files are owned by the user and not writable by others, and consider prompting for confirmation before adding paths to sys.path. Alternatively, restrict discovery to well-known, secure installation paths.
There was a problem hiding this comment.
Thanks for the security analysis. A few points on why this is acceptable risk:
-
No code from the .desktop file is executed. The function only reads the
Exec=field to extract a directory path, then validates that directory against hard-coded IDA-specific file markers (ida.hlp,libidalib.so,idalib/python/idapro/__init__.py). The binary referenced byExec=is never run. -
Same trust model as the existing config path.
find_idalib()already readsida-config.jsonfrom a user-writable directory (~/.idapro) and trusts theida-install-dirvalue inside it — identical exposure. A maliciousida-config.jsoncould point to an attacker-controlledidalib/python/in exactly the same way. -
Prerequisite: attacker already has user-level write access. If an attacker can plant a file in
~/Desktop, they already control the user's home directory and can modify.bashrc,.profile, Python'ssitecustomize.py, or any number of higher-impact targets. The marginal attack surface added here is negligible.
For these reasons I'm keeping the current implementation.
|
@devs6186 we're you able to test this locally? |
|
i'd like to recommend we move in the opposite direction of this PR (though i'm happy if @devs6186 takes part). for IDA, we can increasingly rely on ida-config.json to specify where to find the active IDA installation. if this isn't found, we can reasonably indicate this is a user error (and give guidance). when the code was first written, the IDA infrastructure for this wasn't solid yet, so we supported more methods of discovery. that's not needed anymore. |
ad4998b to
d226573
Compare
…dance
Remove the .desktop entry fallback for locating IDA installations.
As IDA's tooling has matured, ida-config.json (created by
py-activate-idalib.py) is the canonical way to locate idalib;
additional discovery heuristics are no longer needed.
When ida-config.json is missing or misconfigured, the error messages
now tell the user exactly which file was expected and how to create it
(by running the idalib activation script).
Also extract _get_install_dir_from_config() and
_locate_idalib_in_install_dir() from find_idalib() for clarity, add
robust error handling for corrupt/unreadable config files, handle the
case where ida-install-dir is an empty string (the default created by
pip install idapro), and fix a pre-existing mypy type error with
os.getenv("APPDATA").
Fixes mandiant#2412.
d226573 to
15f97a5
Compare
Hi @mike-hunhoff Switching to rely purely on ida-config.json made local testing much more straightforward. I went ahead and manually verified each error path against real configs: Missing config: It now reports the expected platform-specific path and points the user to py-activate-idalib.py. Also, I refactored the helpers (_get_install_dir_from_config and _locate_idalib_in_install_dir) to take explicit Path arguments, so they're perfectly set up for unit testing down the road if we want to add that. |
@williballenthin Thanks for the guidance on this! It makes total sense that the infrastructure has outgrown the need for those heuristic discoveries. I would love to take part in this, i apologize to overstep but I've reworked the PR based on your feedback and i am open all suggestions and changes:
The code refactors find_idalib() into two helpers for readability and adds error handling for malformed config files (the original would crash on corrupt JSON). |
Problem
find_idalib()relied exclusively onida-config.json, which is only created by thepy-activate-idalib.pyinstallation script. Users who have IDA installed and launch it via a.desktopshortcut on their Linux/macOS Desktop — but have not yet run the idalib installer — were told idalib could not be found even when IDA was fully functional.Reported in #2412.
Solution
Add a fallback discovery path,
_find_install_dir_from_desktop_entries(), that:~/Desktopfor*.desktopfiles (Linux/macOS only; Windows continues to use APPDATA config exclusively).Exec=field withconfigparser.ConfigParser.This candidate is then validated by the new
_locate_idalib_in_install_dir()helper, which checks forida.hlp, the platform library (libidalib.so/idalib.dll/libidalib.dylib), and theidalib/python/idapro/__init__.pypackage — identical criteria to the original code.The refactoring also extracts
_get_install_dir_from_config()from the original monolithicfind_idalib()and adds proper error handling forOSError/UnicodeDecodeError/JSONDecodeErroraround config reads.Lookup order (unchanged for existing users)
ida-config.json(primary, all platforms)~/Desktop/*.desktopExec= path (new fallback, Linux/macOS only)None+ error log if neither yields a valid install dirChanges
capa/features/extractors/ida/idalib.py_get_install_dir_from_config,_find_install_dir_from_desktop_entries,_locate_idalib_in_install_dir; refactoredfind_idalibTesting
ida-config.jsonis absent — no behaviour change on Windows or for users with the config file..desktopspec and againstidalibdiscovery logic in the original implementation.Fixes #2412.