Review redirects to a URL that is not validated as local

Rule ND3126 Security

Matches methods calling Redirect(url) (or its permanent / preserve-method variants) on a controller, page model, HTTP response or results helper, that take a string parameter or read request data (query, form, headers, route values) and never call Url.IsLocalUrl() nor LocalRedirect(). A returnUrl forwarded as-is is the classic open redirect used in phishing campaigns.

This is CWE-601 URL Redirection to Untrusted Site (review).

Flagged Code
public IActionResult AfterLogin(string returnUrl)
{
   return Redirect(returnUrl);                          // flagged: any absolute URL is accepted
}
Fixed Code
public IActionResult AfterLogin(string returnUrl)
{
   if (Url.IsLocalUrl(returnUrl))
      return LocalRedirect(returnUrl);                  // only paths on this site
   return RedirectToAction("Index");
}

How to fix

Use LocalRedirect(returnUrl) or guard with if (Url.IsLocalUrl(returnUrl)) ... else RedirectToAction("Index"). For legitimate external redirects, validate the host against a whitelist. Suppress the issue when the URL is built from constants only, from the server's own configuration (an administrator-controlled base URL), or as a relative URI computed against the request's own scheme and host (Uri.MakeRelativeUri between two URIs built on the same host): such a target cannot leave the site.

CQLinq source

This rule is written in CQLinq, C# LINQ over the NDepend code model. It ships with every new NDepend project: you can read it, adapt it, or use it as a starting point for your own custom rules.

// <Name>Review redirects to a URL that is not validated as local</Name>
// <Id>ND3126:ReviewRedirectsToUrlNotValidatedAsLocal</Id>
warnif count > 0

// Third-party members are part of the code model only when the application uses them.
let redirectMembers = ThirdParty.Methods.Where(m =>
      m.SimpleNameEqualsAny("Redirect", "RedirectPermanent", "RedirectPreserveMethod", "RedirectPermanentPreserveMethod") &&
      m.Name.Contains("(String") &&
      m.ParentType.FullNameEqualsAny(
        "Microsoft.AspNetCore.Mvc.ControllerBase", "Microsoft.AspNetCore.Mvc.RazorPages.PageModel",
        "Microsoft.AspNetCore.Http.HttpResponse", "Microsoft.AspNetCore.Http.Results", "Microsoft.AspNetCore.Http.TypedResults",
        "System.Web.HttpResponse", "System.Web.HttpResponseBase", "System.Web.Mvc.Controller"))
   .ToHashSetEx()
where redirectMembers.Any()

let localUrlChecks = ThirdParty.Methods.Where(m =>
      m.SimpleNameEqualsAny("IsLocalUrl", "LocalRedirect", "LocalRedirectPermanent", "LocalRedirectPreserveMethod", "LocalRedirectPermanentPreserveMethod"))
   .ToHashSetEx()

// Request data read in the method: query string, form, headers, route values...
let requestReaders = ThirdParty.Methods.Where(m =>
      m.IsPropertyGetter &&
      m.SimpleNameEqualsAny("get_Query", "get_QueryString", "get_Form", "get_Headers", "get_RouteValues", "get_Cookies", "get_Path", "get_Referer", "get_UrlReferrer") &&
      m.ParentType.FullNameEqualsAny("Microsoft.AspNetCore.Http.HttpRequest", "System.Web.HttpRequest", "System.Web.HttpRequestBase", "Microsoft.AspNetCore.Http.Headers.RequestHeaders"))
   .ToHashSetEx()

from m in Application.Methods.UsingAny(redirectMembers)
where !m.MethodsCalled.Intersect(localUrlChecks).Any()
// The redirected URL can be influenced by the caller: a string parameter or request data.
where m.Name.Contains("String") || m.MethodsCalled.Intersect(requestReaders).Any()
select new {
  m,
  redirectMembersUsed = m.MethodsCalled.Intersect(redirectMembers).ToArray(),
  Debt = 10.ToMinutes().ToDebt(),
  Severity = Severity.Medium
}

//<Expl>
// The *{0}* redirects with *{1}* to a URL that may come from a parameter
// or from the request, without checking that it is local. A crafted link
// can bounce the user to a phishing site under the application's own domain.
//</Expl>

//<Desc>
// Matches methods calling *Redirect(url)* (or its permanent /
// preserve-method variants) on a controller, page model, HTTP response or
// results helper, that take a string parameter or read request data
// (query, form, headers, route values) and never call *Url.IsLocalUrl()*
// nor *LocalRedirect()*. A *returnUrl* forwarded as-is is the classic
// *open redirect* used in phishing campaigns.
//
// This is CWE-601 *URL Redirection to Untrusted Site* (review).
//
// **Flagged Code**
// ```csharp
// public IActionResult AfterLogin(string returnUrl)
// {
//    return Redirect(returnUrl);                          // flagged: any absolute URL is accepted
// }
// ```
//
// **Fixed Code**
// ```csharp
// public IActionResult AfterLogin(string returnUrl)
// {
//    if (Url.IsLocalUrl(returnUrl))
//       return LocalRedirect(returnUrl);                  // only paths on this site
//    return RedirectToAction("Index");
// }
// ```
//</Desc>

//<HowToFix>
// Use *LocalRedirect(returnUrl)* or guard with
// *if (Url.IsLocalUrl(returnUrl)) ... else RedirectToAction("Index")*.
// For legitimate external redirects, validate the host against a
// whitelist. Suppress the issue when the URL is built from constants only,
// from the server's own configuration (an administrator-controlled base URL),
// or as a relative URI computed against the request's own scheme and host
// (*Uri.MakeRelativeUri* between two URIs built on the same host): such a
// target cannot leave the site.
//</HowToFix>

Open in the interactive Rules Explorer · All default rules

Security: all 39 items