Merge pull request #15623 from erik-krogh/cs-url

C#: update the QHelp for `cs/web/unvalidated-url-redirection`

csharp/ql/src/Security Features/CWE-601/UrlRedirect.cs

@@ -1,19 +0,0 @@
-using System;
-using System.Web;
-
-public class UnvalidatedUrlHandler : IHttpHandler
-{
-    private const String VALID_REDIRECT = "http://cwe.mitre.org/data/definitions/601.html";
-
-    public void ProcessRequest(HttpContext ctx)
-    {
-        // BAD: a request parameter is incorporated without validation into a URL redirect
-        ctx.Response.Redirect(ctx.Request.QueryString["page"]);
-
-        // GOOD: the request parameter is validated against a known fixed string
-        if (VALID_REDIRECT == ctx.Request.QueryString["page"])
-        {
-            ctx.Response.Redirect(VALID_REDIRECT);
-        }
-    }
-}

csharp/ql/src/Security Features/CWE-601/UrlRedirect.qhelp

@@ -16,18 +16,43 @@ controlled by the attacker.</p>
 <p>To guard against untrusted URL redirection, it is advisable to avoid putting user input
 directly into a redirect URL. Instead, maintain a list of authorized
 redirects on the server; then choose from that list based on the user input provided.</p>
-
+<p>
+If this is not possible, then the user input should be validated in some other way,
+for example, by verifying that the target URL is on the same host as the current page.
+</p>
 </recommendation>
-<example>
 
-<p>The following example shows an HTTP request parameter being used directly in a URL redirect
-without validating the input, which facilitates phishing attacks.
-It also shows how to remedy the problem by validating the user input against a known fixed string.
+
+<example>
+<p>
+The following example shows an HTTP request parameter being used directly in a URL redirect
+without validating the input, which facilitates phishing attacks:
 </p>
 
-<sample src="UrlRedirect.cs" />
+<sample src="examples/UrlRedirect.cs"/>
+
+<p>
+One way to remedy the problem is to validate the user input against a known fixed string
+before doing the redirection:
+</p>
+
+<sample src="examples/UrlRedirectGood.cs"/>
+
+<p>
+Alternatively, we can check that the target URL does not redirect to a different host
+by checking that the URL is either relative or on a known good host:
+</p>
+
+<sample src="examples/UrlRedirectGoodDomain.cs"/>
+
+<p>
+Note that as written, the above code will allow redirects to URLs on <code>example.com</code>,
+which is harmless but perhaps not intended. You can substitute your own domain (if known) for
+<code>example.com</code> to prevent this.
+</p>
 
 </example>
+
 <references>
 
 <li>

csharp/ql/src/Security Features/CWE-601/examples/UrlRedirect.cs

@@ -0,0 +1,11 @@
+using System;
+using System.Web;
+
+public class UnvalidatedUrlHandler : IHttpHandler
+{
+    public void ProcessRequest(HttpContext ctx)
+    {
+        // BAD: a request parameter is incorporated without validation into a URL redirect
+        ctx.Response.Redirect(ctx.Request.QueryString["page"]);
+    }
+}

csharp/ql/src/Security Features/CWE-601/examples/UrlRedirectGood.cs

@@ -0,0 +1,17 @@
+using System;
+using System.Web;
+using System.Collections.Generic;
+
+public class UnvalidatedUrlHandler : IHttpHandler
+{
+    private List<string> VALID_REDIRECTS = new List<string>{ "http://cwe.mitre.org/data/definitions/601.html", "http://cwe.mitre.org/data/definitions/79.html" };
+
+    public void ProcessRequest(HttpContext ctx)
+    {
+        if (VALID_REDIRECTS.Contains(ctx.Request.QueryString["page"]))
+        {
+            // GOOD: the request parameter is validated against a known list of strings
+            ctx.Response.Redirect(ctx.Request.QueryString["page"]);
+        }
+    }
+}

csharp/ql/src/Security Features/CWE-601/examples/UrlRedirectGoodDomain.cs

@@ -0,0 +1,22 @@
+using System;
+using System.Web;
+
+public class UnvalidatedUrlHandler : IHttpHandler
+{
+    public void ProcessRequest(HttpContext ctx)
+    {
+        var urlString = ctx.Request.QueryString["page"];
+        var url = new Uri(urlString, UriKind.RelativeOrAbsolute);
+
+        var url = new Uri(redirectUrl, UriKind.RelativeOrAbsolute);
+        if (!url.IsAbsoluteUri) {
+            // GOOD: The redirect is to a relative URL
+            ctx.Response.Redirect(url.ToString());
+        }
+
+        if (url.Host == "example.org") {
+            // GOOD: The redirect is to a known host
+            ctx.Response.Redirect(url.ToString());
+        }
+    }
+}