Skip to content

Fix false positives caused in Android manifest analysis#2481

Closed
nick-lupien wants to merge 2 commits intoMobSF:masterfrom
nick-lupien:nick-lupien/fix-fps-manifest-analysis
Closed

Fix false positives caused in Android manifest analysis#2481
nick-lupien wants to merge 2 commits intoMobSF:masterfrom
nick-lupien:nick-lupien/fix-fps-manifest-analysis

Conversation

@nick-lupien
Copy link

@nick-lupien nick-lupien commented Jan 7, 2025

Describe the Pull Request

Problem:

  • Recurring false positives coming from the Android manifest analysis module.
  • FPs occur when well-known URLs like assetlinks.json redirect from http->https. In these instances, high-severity findings would appear in the Manifest Analysis section suggesting that the URLs were configured incorrectly, when in fact the 301 redirect is a valid configuration.
  • Ordinarily this would be resolved by setting allow_redirects to True in the requests.get, but this was removed in (f22c584) specifically to mitigate GHSA-m435-9v6r-v5f6.
  • This PR aims to reduce a specific, common false positive while maintaining protections against SSRF.

Solution:

  • Check redirects to see if they're simple TLS upgrades; if so, perform a manual redirect to the TLS version.

Checklist for PR

  • Run MobSF unit tests and lint tox -e lint,test
  • Tested Working on Linux, Mac, Windows, and Docker
  • Add unit test for any new Web API (Refer: StaticAnalyzer/tests.py)
  • Make sure tests are passing on your PR MobSF tests

Additional Comments (if any)

@nick-lupien nick-lupien marked this pull request as ready for review January 8, 2025 01:42
@ajinabraham
Copy link
Member

Thanks for the PR. I will review this and get back.

@ajinabraham
Copy link
Member

Simplified the logic a bit to automatically handle this without requiring a redirect.
Closing in favour of #2484

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants