JavaScript: Improve query help for js/server-side-unvalidated-url-redirection.

[max-schaefer]

I'm not really sure about this, so opening as a draft pull request first to get some input from the JavaScript team.

Basically, our query help currently suggests an allowlist approach to preventing redirections. That's great advice in general, but in practice may be difficult and invasive to implement, so I was wondering whether we also want to suggest a more local fix.

The one I added to the query help is to check whether the URL is "local" in the sense that it doesn't redirect to a different host. Working back from the query, one way of doing that is to ensure it starts with a single, but not a double, slash. However, I've seen the slides from this BlackHat talk, and while they are a bit hard to understand I think the upshot is that this sort of thing is extremely tricky to check reliably, so perhaps this is actually dangerous advice?

[github-actions]

QHelp previews:

javascript/ql/src/Security/CWE-601/ServerSideUrlRedirect.qhelp

Server-side URL redirect

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:

const app = require("express")();

app.get('/redirect', function(req, res) {
  // BAD: a request parameter is incorporated without validation into a URL redirect
  res.redirect(req.params["target"]);
});

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

const app = require("express")();

const VALID_REDIRECT = "http://cwe.mitre.org/data/definitions/601.html";

app.get('/redirect', function(req, res) {
  // GOOD: the request parameter is validated against a known fixed string
  let target = req.params["target"]
  if (VALID_REDIRECT === target) {
    res.redirect(target);
  } else {
    res.redirect("/");
  }
});

Alternatively, we can check that the target URL does not redirect to a different host:

const app = require("express")();

function isLocalUrl(url) {
  return url.startsWith("/") && !url.startsWith("//") && !url.startsWith("/\\");
}

app.get('/redirect', function(req, res) {
  // GOOD: check that we don't redirect to a different host
  let target = req.params["target"];
  if (isLocalUrl(target)) {
    res.redirect(target);
  } else {
    res.redirect("/");
  }
});

References

• OWASP: XSS Unvalidated Redirects and Forwards Cheat Sheet.
• Common Weakness Enumeration: CWE-601.