Security: tighten redirect URI matching to prevent prefix-confusion attacks (FireWatch c1bf88bd), Fixes AB#3665141 by wzhipan · Pull Request #3136 · AzureAD/microsoft-authentication-library-common-for-android

wzhipan · 2026-06-03T22:22:52Z

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).

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 (needs reviewer decision)

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. Closing the msauth path likely requires also tightening isInstallRequestUrl / processInstallRequest. Flagging for reviewer input on whether to expand scope here or track as a follow-up.

Tests

Assert the real security property via the completion callback (not just the shouldOverrideUrlLoading return value), using an https:// redirect URI:

Test	Asserts
`testStrictMatching_acceptsLegitimateHttpsRedirect`	Legit redirect -> code delivered once
`testStrictMatching_acceptsTrailingSlashPathDifference`	`/auth` registered, `/auth/` incoming -> still delivered
`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.

Draft because:

Reviewer decision needed on the msauth:// install-path limitation above
Pipelines need to validate against the full test suite

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

AzureActiveDirectoryWebViewClient.isRedirectUrl() used String#startsWith() to identify navigations carrying the OAuth2 authorization response back to the configured redirect URI. Because the match was a prefix check, a URL such as msauth://com.outlook/HASH=.attacker.com/x?code=<AUTH_CODE> would also match the registered redirect URI msauth://com.outlook/HASH= and the auth code would be handed to processRedirectUrl() on a URL controlled by an attacker (in the event of a malicious server response or a MITM'd connection). eSTS performs exact redirect URI validation on the server side and the finding is therefore tagged Moderate / Not Exploitable, but the client should also do an exact check (defense-in-depth, MSRC Ceiling Rule 3). This change parses both the candidate URL and the configured redirect URI and requires that scheme, authority (host[:port]) and path match. Query string and fragment are intentionally ignored because they carry the auth code and state on legitimate redirects. Fixes FireWatch finding c1bf88bd-5fce-454c-a028-cbfe176639e0 Fixes IcM 31000000624712 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-06-03T22:23:12Z

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

Click here to Learn more.

…i-matching

- Add ENABLE_STRICT_REDIRECT_URI_MATCHING CommonFlight (default on) as an ECS kill switch. When disabled, isRedirectUrl falls back to the historical String#startsWith prefix match so the change can be turned off via flighting without a code rollback. - Fix the failing unit tests. The previous spoofing tests asserted shouldOverrideUrlLoading() returns false, which is wrong: for an msauth:// redirect URI a spoofed URL is still caught downstream by isInstallRequestUrl (BROWSER_EXT_INSTALL_PREFIX = "msauth://") whose processInstallRequest also extracts the code, so the isRedirectUrl fix is not observable at the callback for msauth. Rewrite the tests against an https:// redirect URI (which isolates isRedirectUrl) and assert the security property via the completion callback: spoofed URLs are never delivered; the legitimate redirect is. - Add a kill-switch test proving the flag reverts to prefix-match behavior. Note: isInstallRequestUrl matching the entire msauth:// scheme means the isRedirectUrl fix alone does not close the prefix-confusion path for broker (msauth) redirect URIs. Tracked as a follow-up. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-06-15T01:33:07Z

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

Click here to learn more.

github-actions · 2026-06-15T02:16:08Z

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

isRedirectUrl now treats a registered "/auth" and an incoming "/auth/" as the same resource by collapsing a single trailing slash before the path comparison (normalizePath). Only one trailing slash is removed; any other path difference (extra segment, suffix, "/auth//") is still rejected, so the prefix-confusion protection is unchanged. Adds a unit test asserting the trailing-slash redirect is still delivered to the completion callback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…i-matching

- Trim the vNext changelog line to one concise sentence and add the PR number (#3136). - Condense the verbose comments on isRedirectUrl, normalizePath, the CommonFlight kill-switch doc, and the test vector block. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

#3128 was a vNext entry when this branch was created; it has since been released into "Version 24.3.0" on dev. The branch carried the old vNext copy in addition to the released one (merged from dev), producing a duplicate. Remove the stale vNext copy so the changelog matches dev and this PR's changelog diff is just the single new entry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-06-17T18:44:05Z

❌ Invalid work item number: AB#3665141

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

Click here to learn more.

github-actions · 2026-06-17T18:44:06Z

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

github-actions · 2026-06-17T18:46:00Z

❌ 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].

wzhipan · 2026-06-17T22:25:55Z

+        try {
+            final Uri actual = Uri.parse(url);
+            final Uri expected = Uri.parse(mRedirectUrl);
+            final String expectedScheme = expected.getScheme();
+
+            // Scheme-less configured URI: fall back to strict equality.
+            if (expectedScheme == null || expectedScheme.isEmpty()) {
+                return url.equalsIgnoreCase(mRedirectUrl);
+            }
+            if (!expectedScheme.equalsIgnoreCase(actual.getScheme())) {
+                return false;
+            }
+            if (!equalsIgnoreCaseNullSafe(actual.getAuthority(), expected.getAuthority())) {
+                return false;
+            }
+            // Single trailing slash is normalized so "/auth" matches "/auth/".
+            return normalizePath(actual.getPath())
+                    .equalsIgnoreCase(normalizePath(expected.getPath()));
+        } catch (final Throwable t) {
+            // Fail closed on unparseable URLs rather than risk leaking the code.
+            Logger.warn(TAG, "Failed to parse URL for redirect URI comparison: " + t);
+            return false;
+        }


Good catch — confirmed and fixed in 1cb637e. urn:ietf:wg:oauth:2.0:oob is exactly AuthenticationConstants.Broker.BROKER_REDIRECT_URI, and being opaque it returns null authority/path, so the comparison degenerated to scheme-only and would accept any urn:...?code=STOLEN.

Fix: when either URI is opaque, require both to be opaque and compare the scheme-specific part (minus appended query/fragment) instead of authority/path. Also stopped logging the Throwable on the parse-failure path since its message could embed the URL/auth code — now logs a static message.

wzhipan · 2026-06-17T22:25:57Z

+    public void testStrictMatching_rejectsSpoofedRedirectWithPathSuffix() throws ClientException {
+        assertSpoofedRedirectNotDelivered(HTTPS_REDIRECT_SPOOFED_PATH_SUFFIX);
+    }
+


Added in 1cb637e:

testStrictMatching_acceptsLegitimateOpaqueOobRedirect — configured urn:ietf:wg:oauth:2.0:oob, incoming urn:ietf:wg:oauth:2.0:oob?code=AUTH_CODE&state=xyz -> delivered as a COMPLETED auth result.

testStrictMatching_rejectsSpoofedOpaqueOobRedirect — incoming urn:ietf:wg:oauth:2.0:oobstolen?code=STOLEN -> not delivered as a COMPLETED result (it falls through to the SSL-protection error path; asserted via the captured result code).

…i-matching

Addresses Copilot review on PR #3136. - Opaque URIs (e.g. the broker OOB redirect urn:ietf:wg:oauth:2.0:oob, AuthenticationConstants.Broker.BROKER_REDIRECT_URI) have null authority and path, so the scheme+authority+path comparison degenerated to scheme-only and would accept ANY same-scheme urn (e.g. urn:...:oobstolen?code=STOLEN). Now, when either URI is opaque, compare the scheme-specific part (minus appended query/fragment) and require both to be opaque. - Stop logging the Throwable on the parse-failure path; its message could embed the URL (and thus the auth code). Log a static message instead. - Add positive and negative opaque (OOB) unit tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Keeps the exception class name for debuggability while still avoiding logging the message, which could embed the URL/auth code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

wzhipan and others added 2 commits June 14, 2026 18:25

Merge remote-tracking branch 'origin/dev' into fix/secure-redirect-ur…

6e0746f

…i-matching

wzhipan changed the title ~~Security: tighten redirect URI matching to prevent prefix-confusion attacks (FireWatch c1bf88bd)~~ Security: tighten redirect URI matching to prevent prefix-confusion attacks (FireWatch c1bf88bd), Fixes AB#3665141 Jun 15, 2026

wzhipan and others added 4 commits June 17, 2026 11:20

Merge remote-tracking branch 'origin/dev' into fix/secure-redirect-ur…

35886f3

…i-matching

wzhipan marked this pull request as ready for review June 17, 2026 20:25

wzhipan requested review from a team as code owners June 17, 2026 20:25

Copilot AI review requested due to automatic review settings June 17, 2026 20:25

Copilot started reviewing on behalf of wzhipan June 17, 2026 20:26 View session

Copilot AI reviewed Jun 17, 2026

View reviewed changes

wzhipan and others added 3 commits June 17, 2026 15:19

Merge remote-tracking branch 'origin/dev' into fix/secure-redirect-ur…

db36a17

…i-matching

Log exception type (not message) on redirect URI parse failure

6e6235d

Keeps the exception class name for debuggability while still avoiding logging the message, which could embed the URL/auth code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

Security: tighten redirect URI matching to prevent prefix-confusion attacks (FireWatch c1bf88bd), Fixes AB#3665141#3136
wzhipan wants to merge 10 commits into
devfrom
fix/secure-redirect-uri-matching

wzhipan commented Jun 3, 2026 •

edited by azure-boards Bot

Loading

Uh oh!

github-actions Bot commented Jun 3, 2026

Uh oh!

github-actions Bot commented Jun 15, 2026

Uh oh!

github-actions Bot commented Jun 15, 2026

Uh oh!

github-actions Bot commented Jun 17, 2026

Uh oh!

github-actions Bot commented Jun 17, 2026

Uh oh!

github-actions Bot commented Jun 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

wzhipan Jun 17, 2026

Uh oh!

wzhipan Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

wzhipan commented Jun 3, 2026 • edited by azure-boards Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

What changes

⚠️ Known limitation — msauth:// install path (needs reviewer decision)

Tests

References

Validation

Uh oh!

github-actions Bot commented Jun 3, 2026

Uh oh!

github-actions Bot commented Jun 15, 2026

Uh oh!

github-actions Bot commented Jun 15, 2026

Uh oh!

github-actions Bot commented Jun 17, 2026

Uh oh!

github-actions Bot commented Jun 17, 2026

Uh oh!

github-actions Bot commented Jun 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

wzhipan Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

wzhipan Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wzhipan commented Jun 3, 2026 •

edited by azure-boards Bot

Loading