Port MASTG-TEST-0028: Testing for Testing Deep Links (android) (by @appknox) - #3463
Port MASTG-TEST-0028: Testing for Testing Deep Links (android) (by @appknox)#3463ScreaMy7 wants to merge 18 commits into
Conversation
There was a problem hiding this comment.
@ScreaMy7
I see that a lot of the information contained in this test is being lost right now. Please consider porting it all:
- What information is knowledge and belongs in https://mas.owasp.org/MASTG/knowledge/android/MASVS-PLATFORM/MASTG-KNOW-0019/
- What information describes best practices and should be added to a new
MASTG-BEST-XXXX? (each test should have at least one best practice linked) - What techniques are here that don't have a MASTG-TECH-XXXX entry? e.g.
- "Check for Deep Link Usage"
- "Check for Correct Website Association"
- "Monitoring Deep Links"
- "Invoking Deep Links"
- etc
- What tools are used and don't have a MASTG-TOOL-XXXX entry? e.g. "Android App Link Verification Tester"
- What tests are missing?
- "Use of Unverified App Links"
- "Use of Unverified Custom URL Schemes"
- "Use of Verified App Links with Incorrect Website Association"
| @@ -0,0 +1,24 @@ | |||
| --- | |||
| title: Deep Link Intent Filter Missing android:autoVerify | |||
There was a problem hiding this comment.
This should be "Use of Unverified App Links" plus an additional "Use of Unverified Custom URL Schemes" which is now missing.
|
Also please use "fake IDs" (e.g. MASTG-TEST-0x28-1, MASTG-TEST-0x28-2, MASTG-DEMO-00xx-1, MASTG-DEMO-00xx-2) until we're ready to merge, to avoid conflicts. |
cpholguera
changed the title
|
@ScreaMy7 any news on this? |
|
@cpholguera I have added new demos, a tool and the new technique also contain invoking and monitoring deeplinks. Please review. |
There was a problem hiding this comment.
Pull request overview
This PR ports MASTG-TEST-0028 (Testing Deep Links on Android) from the legacy format to MASTG V2 structure. It deprecates the old test and creates three new focused tests covering different deep link security aspects, along with comprehensive supporting documentation.
- Creates three new deep link security tests (MASTG-TEST-XXXA, MASTG-TEST-XXXB, MASTG-TEST-0295) covering WebView URL validation, autoVerify attribute, and custom URL schemes
- Adds new technique document (MASTG-TECH-XXXX) with detailed procedures for deep link testing including verification, monitoring, and invocation
- Provides three complete demos with Semgrep rules, sample code, and expected outputs
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 26 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/android/MASVS-PLATFORM/MASTG-TEST-0028.md | Marks the legacy test as deprecated with references to new replacement tests |
| tests-beta/android/MASVS-PLATFORM/MASTG-TEST-XXXA.md | New test for detecting unvalidated deep link URLs loaded into WebViews |
| tests-beta/android/MASVS-PLATFORM/MASTG-TEST-XXXB.md | New test for detecting missing android:autoVerify attribute in intent filters |
| tests-beta/android/MASVS-PLATFORM/MASTG-TEST-XXXC.md | New test (ID: MASTG-TEST-0295) for detecting use of unverified custom URL schemes |
| techniques/android/MASTG-TECH-XXXX.md | Comprehensive technique document covering deep link detection, validation, monitoring, and invocation |
| tools/android/MASTG-TOOL-014X.md | Documents the App Link Verification command-line tool |
| rules/mastg-android-unvalidated-deeplink-data.yml | Semgrep taint rule for detecting unvalidated deep link data flows to WebView |
| rules/mastg-android-custom-deeplink-scheme.yml | Semgrep rule for detecting custom URL schemes in intent filters |
| rules/mastg-android-autoverify-missing.yml | Semgrep rule for detecting missing autoVerify attribute |
| demos/android/MASVS-PLATFORM/MASTG-DEMO-XXXA/* | Complete demo for WebView deep link vulnerability with Kotlin source, reversed Java, manifest, and Semgrep output |
| demos/android/MASVS-PLATFORM/MASTG-DEMO-XXXB/* | Complete demo for missing autoVerify attribute with manifest and Semgrep output |
| demos/android/MASVS-PLATFORM/MASTG-DEMO-XXXC/* | Complete demo for custom URL scheme vulnerability with Kotlin source, reversed Java, manifest, and Semgrep output |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ## Steps | ||
|
|
||
| 1. Run a static analysis tool such as @MASTG-TOOL-0110 on the `AndroidManifest.xml` to list the `VIEW/BROWSABLE` intent-filters declaring non-HTTP(S) schemes. |
There was a problem hiding this comment.
No tools in tests.
Correct words "intent filter".
|
|
||
| ## Evaluation | ||
|
|
||
| The test fails if the app relies on custom URL schemes for high-risk operations that should use verified App Links (HTTPS) with Digital Asset Links. Mitigations include migrating to App Links for exclusivity or implementing robust in-app validation and user confirmation. |
There was a problem hiding this comment.
No mitigations in tests. Add best-practices in the metadata.
| status: new | ||
| --- | ||
|
|
||
| ### Overview |
There was a problem hiding this comment.
Demos don't have an Overview section
|
@ScreaMy7 please take a look. There are review comments from December and today. |
|
@cpholguera I have added the changes. Please review. |
There was a problem hiding this comment.
Thanks for contributing! I have a couple of remarks:
- Can you link MASTG-KNOW-0019 knowledge page and add some missing parts from the old test:
- the change in Android 12 that impacts non-verifiable (We had it in the old test)
- the reports from the hackerone, they are very illustrative
- Can you add a Best Practise page and guide the user to use App Links and explain why
Additionally: Not a must for this PR. We can create a new issue for it. Currently we only test receiving the deep links in the app. I think we could also create some best practices on how to invoke app links. I didn't give it much thought yet but maybe the payload could be encrytpted so it can't leak sensitive data and only the legit app/website can decrypt it. Or maybe we can somehow else ensure that the URL won't be hijacked by another app and goes directly to the expected app or website?
|
|
||
| The Android version the app runs on also influences the risk. Before Android 12 (API level 31), if the app has any [non-verifiable links](https://developer.android.com/training/app-links/verify-android-applinks#fix-errors) (e.g., missing `autoVerify`, invalid Digital Asset Links file, or custom URL schemes), the system may skip verification for all Android App Links declared by that app—leaving even correctly configured App Links unprotected. Starting with Android 12 (API level 31), a generic web intent resolves to the user's default browser unless the target app is approved for the specific domain, reducing but not eliminating the attack surface. | ||
|
|
||
| This vulnerability occurs when a deep link intent filter in `AndroidManifest.xml` lacks the `android:autoVerify="true"` attribute. Without verification, Android cannot confirm the app's ownership of the declared domain. A malicious app could register the same intent filter and intercept deep links, enabling phishing, credential theft, or hijacking of user actions. |
There was a problem hiding this comment.
I am not an expert in App Links but this got me curious:
A malicious app could register the same intent filter and intercept deep links, enabling phishing, credential theft, or hijacking of user actions.
So on Android 11, if my legit app has autoVerify="true" but my server returns 404 for the verification, my app won't receive events for this link. But a malicious app with autoVerify="false" will receive these events instead? Or you mean that the system will open "Open with..." dialog and a user can choose a malicious app there? Or both scenarios are possible?
There was a problem hiding this comment.
When a legitimate app has autoVerify="true" but verification fails (404), that domain becomes "unverified" for your app. So it falls back to acting like a regular deep link handle. A malicious app with
autoVerify="false" can then intercept those links because both apps are now considered equally "unverified" for that domain. I tested this scenario on Android 12, I received the "Open with" dialog box.
| @@ -0,0 +1,25 @@ | |||
| --- | |||
| title: Use of Unverified Custom URL Schemes | |||
There was a problem hiding this comment.
Aren't Custom URL Schemes always Unverified? Only AppLinks can be Verified / Non Verified, no? I believe we want to detect use of any Custom URL Schemes because they are fundamentally insecure?
There was a problem hiding this comment.
Then maybe we should name this test "Use of Custom URL Schemes"?
| @@ -0,0 +1,25 @@ | |||
| --- | |||
| title: Use of Unverified Custom URL Schemes | |||
There was a problem hiding this comment.
| title: Use of Unverified Custom URL Schemes | |
| title: Use of Custom URL Schemes |
|
Hi, @serek8 is there any other blocker for this PR, I have added the title change as suggested. |
serek8
left a comment
serek8
left a comment
There was a problem hiding this comment.
I briefly had a look and found cosmetic some issues. I still need to run the DEMOs myself
| <action android:name="android.intent.action.VIEW" /> | ||
| <category android:name="android.intent.category.DEFAULT" /> | ||
| <category android:name="android.intent.category.BROWSABLE" /> | ||
| <data android:scheme="https" /> |
There was a problem hiding this comment.
Should we also cover <data android:scheme="http" />?
There was a problem hiding this comment.
Yes, app Links apply to both http and https. I'll update the rule to catch both schemes.
| @@ -0,0 +1,11 @@ | |||
| rules: | |||
| - id: mastg-android-custom-deeplink-intent-filter | |||
There was a problem hiding this comment.
ID doesn't match filename
| - id: mastg-android-custom-deeplink-intent-filter | |
| - id: mastg-android-custom-deeplink-scheme |
| └────────────────┘ | ||
|
|
||
| AndroidManifest_reversed.xml | ||
| ❯❱ android-custom-deeplink-intent-filter |
There was a problem hiding this comment.
ID doesn't match
| ❯❱ android-custom-deeplink-intent-filter | |
| ❯❱ mastg-android-custom-deeplink-scheme |
| - pattern: | | ||
| <intent-filter> | ||
| <action android:name="android.intent.action.VIEW" /> | ||
| <category android:name="android.intent.category.DEFAULT" /> | ||
| <category android:name="android.intent.category.BROWSABLE" /> | ||
| <data android:scheme="..." /> | ||
| </intent-filter> |
|
Thanks @serek8 for review addressed all three changes, added http to demo and rule is also updated. |
There was a problem hiding this comment.
This demo doesn't work. Nothing happens when running the adb command.
There was a problem hiding this comment.
Demos must be fully functional. In this case, the link should perform an action so that we can demonstrate how any app can trigger that sensitive action due to a lack of verification.
There was a problem hiding this comment.
Addressed. The demo was manifest-only with no runtime action so I have added App Link and that performs a sensitive action.
The demo's vulnerable sink processDeepLinkAndLoad() was never invoked: the vulnerable-app://deeplink VIEW filter sat on MainActivity, which only reads intent data via the Start button and ignores the launch intent, so `adb shell am start ... VIEW` did nothing. Route the deep link to a dedicated exported DeepLinkActivity whose onCreate reads intent.data and calls processDeepLinkAndLoad(), loading the attacker-controlled url into the WebView. Verified on a physical device: the adb command now renders the URL in the WebView. Regenerated MastgTest_reversed.java and AndroidManifest_reversed.xml via tools/extract-code-for-mastg-demo.sh. The getQueryParameter -> loadUrl data flow is unchanged, so the semgrep finding in output.txt still holds.
…nverified App Link Addresses review feedback (r3448625913): demos must be fully functional, and the link should perform an action demonstrating how an unverified App Link can be triggered by any app. Previously DEMO-0x02 was manifest-only: it documented the missing android:autoVerify but never demonstrated the consequence, and the filter sat on MainActivity (which ignores the launch intent), so the link did nothing at runtime. Add MastgTest.kt with an exported DeepLinkActivity that handles the http(s)://deeplink.example.com App Link (registered WITHOUT android:autoVerify) and performs a sensitive action - disabling two-factor authentication - with no verification or user confirmation. The App Link filter is moved from MainActivity to DeepLinkActivity; MainActivity keeps the LAUNCHER filter. Regenerated AndroidManifest_reversed.xml and MastgTest_reversed.java via tools/extract-code-for-mastg-demo.sh and refreshed output.txt by running run.sh; the autoVerify-missing semgrep finding still holds. Documented the adb trigger command in the Evaluation. Verified on a physical device: opening the link stores two_factor_enabled=false (set_by=deeplink), confirming the sensitive action fires.
d2b4d08 to
cfda51e
Compare
There was a problem hiding this comment.
This rule appears to be too brittle for MASTG coverage. It assumes a specific child order inside the intent filter, VIEW, then DEFAULT, then BROWSABLE, then data. That means it can miss valid manifests where the same elements are present but ordered differently. It also requires DEFAULT, so any app that omits that category is excluded entirely.
Since MASTG rules are meant to work against arbitrary real world apps, the rule should not depend on manifest element ordering unless ordering is semantically required. In this case, the current pattern can produce false negatives for structurally equivalent intent filters, which goes against the intended coverage goals.
There was a problem hiding this comment.
This rule is brittle because it relies on a raw regex over the manifest text rather than XML semantics. It only matches a literal <intent-filter> opening tag, so it can miss valid filters with attributes, whitespace, or formatting differences, such as <intent-filter android:autoVerify="true"> or <intent-filter >.
It also only matches double quoted android:scheme values, even though XML allows single quoted attributes. Since the rule is not XML aware, it can also match text in comments or otherwise behave unexpectedly across formatting edge cases.
|
@ScreaMy7 Thank you for the contribution and for the effort you’ve put into this PR. Due to the complexity of the changes, including demos that do not work as expected or are not fully functional, a brittle Semgrep rule, a large number of unaddressed comments, and the fact that the actual risks are not properly represented in the tests and demos, we had to work on a separate branch and PR for this port. We will move forward with that PR, so we will close this one. Thanks again for your work and for addressing some of the comments over the last couple of hours. However, in order to transition to MASTG v2 before the MAScon, our current goal, we need a fully compliant PR. Your feedback will be appreciated over there. The OWASP MAS team. |
closes #2980