Flagged as insecure / outdated root detection (OWASP MASTG-KNOW-0027) and unreliable on modern Android devices (refs #5525, #2495)#8217
Conversation
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 the 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 counterproductive. 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. |
📝 PRs merging into main branchOur main branch should always be in a releasable state. If you are working on a larger change, or if you don't want this change to see the light of the day just yet, consider using a feature branch first, and only merge into the main branch when the code complete and ready to be released. |
| * <p> | ||
| * @return true if any rule is met. | ||
| */ | ||
| public static boolean isRooted(Context context) { |
There was a problem hiding this comment.
We used to have more checks, like the ones you added here, but they were ineffective at detecting rooted devices. Modern Android versions won't let one app do anything to detect if another app exists, otherwise you could easily make spyware. Also modern Android locks down the file system, so you can't just see if files exist in specific system folders. Also, any modern technique to root a device will do things to hide su to try to be undetectable
Instead of adding more heuristics to detect root, can we just make the existing root detention heuristics not trigger the security scan? Crashlytics only uses this check to mark events as possibly from rooted devices, not for anything critical. So it's ok to have a less reliable root detection, but it shouldn't get flagged on security scans
There was a problem hiding this comment.
We can easily mitigate the "Static file path checks (/system/xbin/su, etc.)" warning by obfuscating the string literals. You could use base64 encode decode, or some other method. This will stop the scanner from flagging it, but won't do anything to improve the reliability
2 participants
Fixes: #8099
Refactoring root detection logic to align with MASTG-KNOW-0027
Context reference: https://mas.owasp.org/MASTG/knowledge/android/MASVS-RESILIENCE/MASTG-KNOW-0027/
NOTE: Validation of active processes is intentionally excluded from this implementation. As this method support analytics during the initialization phase, scanning the process list may trigger performance regressions or potential deadlock scenarios.