fix Incomplete URL substring sanitization on revision()

[odaysec]

treeherder/ui/helpers/revision.js

Line 16 in 07236d2

return /^[a-f\d]{12,40}$/.test(str) || str.includes('hg.mozilla.org');

fix the issue the code should parse the URL and explicitly check the host component to ensure it matches the expected value (hg.mozilla.org). This can be achieved using the URL class, which provides a reliable way to parse and extract the host from a URL string. The fix involves replacing the substring check with a host comparison using the URL class.

Sanitizing untrusted URLs is an important technique for preventing attacks such as request forgeries and malicious redirections. Usually, this is done by checking that the host of a URL is in a set of allowed hosts. However, treating the URL as a string and checking if one of the allowed hosts is a substring of the URL is very prone to errors. Malicious URLs can bypass such security checks by embedding one of the allowed hosts in an unexpected location. Even if the substring check is not used in a security-critical context, the incomplete check may still cause undesirable behaviors when the check succeeds accidentally.

POC

The following code checks that a URL redirection will reach the hg.mozilla.org domain, or one of its subdomains, and not some malicious site.

app.get('/some/path', function(req, res) {
    let url = req.param("url");
    // BAD: the host of `url` may be controlled by an attacker
    if (url.includes("redacted.com")) {
        res.redirect(url);
    }
});

The substring check is, however, easy to bypass. by embedding redacted.com in the path component: http://evil-hg.mozilla.net/hg.mozilla.org, or in the query string component: http://evil-hg.mozilla.net/?x=hg.mozilla.org. Address these shortcomings by checking the host of the parsed URL instead:

app.get('/some/path', function(req, res) {
    let url = req.param("url"),
        host = urlLib.parse(url).host;
    // BAD: the host of `url` may be controlled by an attacker
    if (host.includes("redacted.com")) {
        res.redirect(url);
    }
});

This is still not a sufficient check as the following URLs bypass it: http://evil-hg.mozilla.com http://hg.mozilla.org.evil-redacted.net. Instead, use an explicit whitelist of allowed hosts to make the redirect secure:

app.get('/some/path', function(req, res) {
    let url = req.param('url'),
        host = urlLib.parse(url).host;
    // GOOD: the host of `url` can not be controlled by an attacker
    let allowedHosts = [
        'mozilla.org',
        'hg.mozilla.org',
        'hg.mozilla.org.net'
    ];
    if (allowedHosts.includes(host)) {
        res.redirect(url);
    }
});

References

SSRF
XSS Unvalidated Redirects and Forwards Cheat Sheet
CWE-20