Security: tighten redirect URI matching to prevent prefix-confusion attacks (FireWatch c1bf88bd), Fixes AB#3665141

[wzhipan]

Fixes AB#3665141 — redirect URI prefix-confusion hardening.

Summary

Tighten redirect URI matching in AzureActiveDirectoryWebViewClient.isRedirectUrl to use structured scheme + authority + path comparison instead of String#startsWith, eliminating a prefix-confusion attack vector. Gated by an ECS kill switch.

Why

isRedirectUrl used url.startsWith(mRedirectUrl.toLowerCase(Locale.US)). Because startsWith is a prefix match, a URL such as

msauth://com.outlook/HASH=.attacker.com/x?code=<AUTH_CODE>

passed the check for the registered redirect URI msauth://com.outlook/HASH=, and the OAuth2 auth code was handed to processRedirectUrl() on a URL controlled by an attacker. eSTS performs exact redirect URI validation server-side, but the client check should be exact too (defense-in-depth, MSRC Ceiling Rule 3).

What changes

isRedirectUrl() — replaced the prefix match with Uri.parse-based comparison requiring scheme + authority (host[:port]) + path to all match (case-insensitive). Query and fragment are ignored (they carry code/state on legit redirects). A single trailing slash on the path is normalized, so a registered /auth still matches an incoming /auth/. Any other path difference is rejected. Unparseable URLs fail closed (false).

Opaque redirect URIs (e.g. the broker OOB redirect urn:ietf:wg:oauth:2.0:oob) — opaque URIs have null authority/path, so a naive scheme+authority+path comparison would degenerate to scheme-only and accept any same-scheme urn:*. These are now compared by scheme-specific part (minus any appended ?query/#fragment), and a spoofed URI must be opaque on both sides. Parse-failure logging records the exception type only (never the throwable message, which could embed the URL and thus the auth code).

ECS kill switch — ENABLE_STRICT_REDIRECT_URI_MATCHING CommonFlight (default on). When disabled via ECS, isRedirectUrl reverts to the historical String#startsWith behavior, allowing a config-only rollback without a code change.

⚠️ Known limitation — msauth:// install path (tracked as follow-up)

For msauth:// redirect URIs (the broker case), this fix alone is not sufficient. After isRedirectUrl correctly rejects a spoofed msauth://… URL, the very next branch in handleUrl — isInstallRequestUrl (BROWSER_EXT_INSTALL_PREFIX = "msauth://") — matches any msauth:// URL, and processInstallRequest also calls RawAuthorizationResult.fromRedirectUri(url), delivering the code to the completion callback. So for msauth the prefix-confusion outcome is unchanged at the callback.

The fix is effective for non-msauth (e.g. https://, custom-scheme) redirect URIs, which is why the new tests use an https:// redirect URI to isolate and prove the isRedirectUrl behavior.

Behavior delta for msauth (no regression): only the routing of a spoofed/suffixed msauth:// URL changes — it now falls through from isRedirectUrl to isInstallRequestUrl and is handled by processInstallRequest instead of processRedirectUrl. The auth-code outcome at the callback is identical (both deliver the same COMPLETED result), so no legitimate redirect or install request changes behavior. The only observable difference is for attack/malformed traffic: it is now logged and counted as a broker-install step (STEP_BROKER_INSTALL_PROMPTED) rather than a redirect — cosmetic, and fully reverted by the ENABLE_STRICT_REDIRECT_URI_MATCHING kill switch.

Plan: closing the msauth path (tightening isInstallRequestUrl / processInstallRequest so a generic msauth://…?code=… without app_link/wpj is not treated as an install request) will be handled as a separate follow-up PR, to keep this defense-in-depth change focused and independently shippable.

Tests

Assert the real security property via the completion callback (not just the shouldOverrideUrlLoading return value), using https:// and opaque urn: redirect URIs:

Test	Asserts
testStrictMatching_acceptsLegitimateHttpsRedirect	Legit redirect -> code delivered once
testStrictMatching_acceptsTrailingSlashPathDifference	/auth registered, /auth/ incoming -> still delivered
testStrictMatching_acceptsLegitimateOpaqueOobRedirect	Legit urn:ietf:wg:oauth:2.0:oob?code=… -> code delivered (COMPLETED)
testStrictMatching_rejectsSpoofedOpaqueOobRedirect	urn:…:oobstolen?code=… -> never delivered as COMPLETED
testStrictMatching_rejectsSpoofedRedirectWithSuffixHost	…/auth.attacker.com/x?code=… -> callback never invoked
testStrictMatching_rejectsSpoofedRedirectWithPathSuffix	…/authstolen?code=… -> callback never invoked
testKillSwitch_disablesStrictMatching_revertsToPrefixMatch	Flag off -> prefix match restored, spoof delivered again
testUrlOverrideHandlesRedirectUriString / …WithFragment	msauth legit redirect (query / fragment) still handled

References

• FireWatch finding c1bf88bd-5fce-454c-a028-cbfe176639e0
• IcM 31000000624712
• TSG: https://aka.ms/IDSEC_AMCConfirmedVulnerabilites

Validation

Local Gradle build needs ADO credential setup unavailable here; PR pipelines run the full unit-test suite. Branch is up to date with dev.

---

Opened by Copilot CLI on behalf of @zhipanwang for IcM 31000000624712.

[github-actions]

❌ Work item link check failed. Description does not contain AB#{ID}.

Click here to Learn more.

[github-actions]

❌ Invalid work item number: AB#`. Work item number must be a valid integer.

Click here to learn more.

[github-actions]

✅ Work item link check complete. Description contains link AB#3665141 to an Azure Boards work item.

[github-actions]

❌ Invalid work item number: AB#3665141

##. Work item number must be a valid integer.

Click here to learn more.

[github-actions]

✅ Work item link check complete. Description contains link AB#3665141 to an Azure Boards work item.

[github-actions]

❌ Work item link check failed. Description contains AB#3665141 but the Bot could not link it to an Azure Boards work item.

Click here to learn more.

[Copilot]

Pull request overview

Hardens WebView redirect handling by replacing redirect-URI prefix matching with structured URI comparison (scheme + authority + path) to mitigate redirect URI prefix-confusion attacks, with an ECS kill switch for rollback.

Changes:

• Added CommonFlight.ENABLE_STRICT_REDIRECT_URI_MATCHING (default on) as a kill switch for strict redirect-URI matching.
• Updated AzureActiveDirectoryWebViewClient.isRedirectUrl to use Uri-based scheme/authority/path comparison (instead of startsWith), with trailing-slash normalization.
• Added/updated unit tests to validate strict matching behavior and the kill switch; updated changelog entry.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
common4j/src/main/com/microsoft/identity/common/java/flighting/CommonFlight.java	Adds a new ECS flight to gate strict redirect-URI matching.
common/src/main/java/com/microsoft/identity/common/internal/ui/webview/AzureActiveDirectoryWebViewClient.java	Implements strict redirect-URI matching logic with kill switch.
common/src/test/java/com/microsoft/identity/common/internal/ui/webview/AzureActiveDirectoryWebViewClientTest.java	Adds tests for strict matching, spoof rejection, trailing slash handling, and kill switch behavior.
changelog.txt	Documents the security hardening as a vNext [PATCH].

[mohitc1]

        } else if (isWebsiteRequestUrl(formattedURL)) {

in isSwitchBrowserRequest() case, it's formattedUrl is /switch_browser. Now, isRedirectUrl() check would fail for this right?

You might want to separate branch for isSwitchBrowserRedirect()

---

Refers to: common/src/main/java/com/microsoft/identity/common/internal/ui/webview/AzureActiveDirectoryWebViewClient.java:358 in 9507dc9. [](commit_id = 9507dc9, deletion_comment = False)