Swift: New query for Missing Regular Expression Anchor

[geoffw0]

New Swift query for missing regular expression anchors. This is a port of the existing query from JS (it also exists for Ruby and Go), with most of the query logic residing in the shared and Swift regular expressions libraries. I suggest that reviewers focus on results (e.g. in the tests, which I have also adapted from JS), or compare to other language versions of the query.

The PR also contains qhelp and examples, adapted from JS as well.

TODO:

• DCA run
• team review
• docs review

[github-actions]

QHelp previews:

swift/ql/src/queries/Security/CWE-020/MissingRegexAnchor.qhelp

Missing regular expression anchor

Sanitizing untrusted input with regular expressions is a common technique, but malicious actors may be able to embed one of the allowed patterns in an unexpected location. To prevent this, you should use anchors in your regular expressions, such as ^ or $.

Even if the matching is not done in a security-critical context, it may still cause undesirable behavior when the regular expression accidentally matches.

Recommendation

Use anchors to ensure that regular expressions match at the expected locations.

Example

The following example code attempts to check that a URL redirection will reach the example.com domain, and not a malicious site:

func handleUrl(_ urlString: String) {
    // get the 'url=' parameter from the URL
    let components = URLComponents(string: urlString)
    let redirectParam = components?.queryItems?.first(where: { $0.name == "url" })

    // check we trust the host
    let regex = try Regex(#"https?://www\.example\.com"#) // BAD: the host of `url` may be controlled by an attacker
    if let match = redirectParam?.value?.firstMatch(of: regex) {
        // ... trust the URL ...
    }
}

However, this regular expression check can be easily bypassed, and a malicious actor could embed http://www.example.com/ in the query string component of a malicious site. For example, http://evil-example.net/?x=http://www.example.com/. Instead, you should use anchors in the regular expression check:

func handleUrl(_ urlString: String) {
    // get the 'url=' parameter from the URL
    let components = URLComponents(string: urlString)
    let redirectParam = components?.queryItems?.first(where: { $0.name == "url" })

    // check we trust the host
    let regex = try Regex(#"^https?://www\.example\.com"#) // GOOD: the host of `url` can not be controlled by an attacker
    if let match = redirectParam?.value?.firstMatch(of: regex) {
        // ... trust the URL ...
    }
}

If you need to write a regular expression to match multiple hosts, you should include an anchor for all of the alternatives. For example, the regular expression /^www\.example\.com|beta\.example\.com/ will match the host evil.beta.example.com, because the regular expression is parsed as /(^www\.example\.com)|(beta\.example\.com)/.

References

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

[MathiasVP]

The code LGTM!

[geoffw0]

Thanks for the quick review. I've added the docs review tag.

[subatoi]

Hey @geoffw0 👋 I've just left some minor suggestions that I think may help the reader—do please let me know what you think and whether you have any questions. My apologies if the suggestions mess up the formatting and you apply them/need to adjust the indentation.

Many thanks!

[geoffw0]

Hi @subatoi, thank you for your review and suggestions. I've accepted three, the other two I think I'm going to adapt a little...

[subatoi]

Thanks @geoffw0, no problem! Just let me know when you want me to approve again 👍

[geoffw0]

Hi, sorry for the delay on this. I've accepted the other two suggestions and only made one edit in the end - deleting the word "only" where it was incorrect to say that.

Thanks again for the review and changes! Ready for approval.

[subatoi]

Thanks again!

[geoffw0]

DCA run LGTM.

I'll merge this when green.