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

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

Author: erik-krogh

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

@@ -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:
+</p>
+
+<sample src="examples/UrlRedirect.cs"/>
 
-<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.
+<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="UrlRedirect.cs" />
+<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/UrlRedirect.qhelp

@@ -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/UrlRedirect.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/UrlRedirectGood.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());
+        }
+    }
+}