Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Feb 16, 2024

Conversation

erik-krogh
Copy link
Contributor

I used the examples from the JS QHelp, and adapted them to the C# QHelp.

The examples should not give any false positives now that #15596 has been merged.

Copy link
Contributor

QHelp previews:

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

URL redirection from remote source

Directly incorporating user input into a URL redirect request without validating the input can facilitate phishing attacks. In these attacks, unsuspecting users can be redirected to a malicious site that looks very similar to the real site they intend to visit, but which is controlled by the attacker.

Recommendation

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.

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.

Example

The following example shows an HTTP request parameter being used directly in a URL redirect without validating the input, which facilitates phishing attacks:

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"]);
    }
}

One way to remedy the problem is to validate the user input against a known fixed string before doing the redirection:

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"]);
        }
    }
}

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:

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());
        }
    }
}

Note that as written, the above code will allow redirects to URLs on example.com, which is harmless but perhaps not intended. You can substitute your own domain (if known) for example.com to prevent this.

References

@erik-krogh erik-krogh marked this pull request as ready for review February 16, 2024 08:44
@erik-krogh erik-krogh requested a review from a team as a code owner February 16, 2024 08:44
Copy link
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!


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" };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Use the brand new C# 12 collection expression language feature

Suggested change
private List<string> VALID_REDIRECTS = new List<string>{ "http://cwe.mitre.org/data/definitions/601.html", "http://cwe.mitre.org/data/definitions/79.html" };
private List<string> VALID_REDIRECTS = ["http://cwe.mitre.org/data/definitions/601.html", "http://cwe.mitre.org/data/definitions/79.html"];

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer not to use language features that are that new.

@erik-krogh erik-krogh merged commit 037e64a into github:main Feb 16, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants