Skip to content

Reject control characters in local redirect URL validation - #4028

Merged
Bogdan Gavril (bgavrilMS) merged 4 commits into
masterfrom
nebharg/fix-664506-redirect-control-char
Sep 14, 2026
Merged

Bogdan Gavril (bgavrilMS) merged 4 commits into
masterfrom
nebharg/fix-664506-redirect-control-char

Conversation

@neha-bhargava

Copy link
Copy Markdown
Contributor

RedirectUriHelper.IsLocalUrl and the AccountController SignIn/Challenge redirect checks accepted paths containing raw ASCII control characters (e.g. tab/CR/LF). Browsers strip these per the WHATWG URL spec, so a value like "//host" resolves to a protocol-relative URL after stripping and was treated as local. Add a HasControlCharacter guard (rejects C0 range and DEL) so the library enforces this independent of the host's ASP.NET Core version. Internal/private helpers only; no public API change.

{PR title}

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Summary of the changes (Less than 80 chars)

Description

{Detail}

Fixes #{bug number} (in this specific format)

Comment thread src/Microsoft.Identity.Web/Internal/RedirectUriHelper.cs Outdated
@iarekk
Iarek Kovtunenko (iarekk) force-pushed the nebharg/fix-664506-redirect-control-char branch from d93ecb0 to d56086c Compare September 14, 2026 11:15
@bgavrilMS
Bogdan Gavril (bgavrilMS) force-pushed the nebharg/fix-664506-redirect-control-char branch from d56086c to 6e73b79 Compare September 14, 2026 13:55
RedirectUriHelper.IsLocalUrl and the AccountController SignIn/Challenge
redirect checks accepted paths containing raw ASCII control characters
(e.g. tab/CR/LF). Browsers strip these per the WHATWG URL spec, so a value
like "/<tab>/host" resolves to a protocol-relative URL after stripping and
was treated as local. Add a HasControlCharacter guard (rejects C0 range and
DEL) so the library enforces this independent of the host's ASP.NET Core
version. Internal/private helpers only; no public API change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e64c3d8c-4f86-4d0b-ac12-0da2e2d44a5b
Combine the AccountController redirect checks, simplify helper documentation, correct the control-character example, and clarify the legacy framework behavior modeled by the tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: e64c3d8c-4f86-4d0b-ac12-0da2e2d44a5b
Document that percent-encoded slash checks use case-insensitive hexadecimal comparison as defined by RFC 3986.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: e64c3d8c-4f86-4d0b-ac12-0da2e2d44a5b
Explain the redirect risk in plain language without relying on standards terminology.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: e64c3d8c-4f86-4d0b-ac12-0da2e2d44a5b
@bgavrilMS
Bogdan Gavril (bgavrilMS) force-pushed the nebharg/fix-664506-redirect-control-char branch from 6e73b79 to 76ff5ff Compare September 14, 2026 14:59
@bgavrilMS
Bogdan Gavril (bgavrilMS) merged commit 46be8bf into master Sep 14, 2026
9 checks passed
@bgavrilMS
Bogdan Gavril (bgavrilMS) deleted the nebharg/fix-664506-redirect-control-char branch September 14, 2026 15:16
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.

3 participants