Skip to content

Commit

Permalink
Secure regular expression fix for hostnames
Browse files Browse the repository at this point in the history
We received a code scanning alert with the following content: [1]

> This regular expression has an unescaped '.' before 'jsdelivr.net',
> so it might match more hosts than expected.

We used escaping "." to get remove the problem.

[1]: https://github.com/fractalsoft/blog.fractalsoft.org/security/code-scanning/1

TL;DR

> Tool: CodeQL
> Rule ID: js/incomplete-hostname-regexp
> Query: [View source](https://github.com/github/codeql/blob/2501a701ad93601b7f892d9f510edb65b7e4a2da/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql)
>
> ------------------------------------------------------------------
>
> Sanitizing untrusted URLs is an important technique
> for preventing attacks such as request forgeries
> and malicious redirections.
> Often, this is done by checking that the host of a URL is in a set
> of allowed hosts.
>
> If a regular expression implements such a check,
> it is easy to accidentally make the check too permissive
> by not escaping the `.` meta-characters appropriately.
> Even if the check is not used in a security-critical context,
> the incomplete check may still cause undesirable behaviors
> when it accidentally succeeds.
>
> Recommendation
> ==============
>
> Escape all meta-characters appropriately when constructing regular
> expressions for security checks, and pay special attention
> to the `.` meta-character.
>
> Example
> -------
>
> The following example code checks that a URL redirection will
> reach the `example.com` domain, or one of its subdomains.
>
> ```
> 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
>     let regex = /^((www|beta).)?example.com/;
>     if (host.match(regex)) {
>         res.redirect(url);
>     }
> });
> ```
>
> The check is however easy to bypass because the unescaped `.` allows
> for any character before `example.com`,
> effectively allowing the redirect to go to
>  an attacker-controlled domain such as `wwwXexample.com`.
>
> Address this vulnerability by escaping `.` appropriately:
> `let regex = /^((www|beta)\.)?example\.com/.`
>
> References
> - MDN: [Regular Expressions](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions)
> - OWASP: [SSRF](https://www.owasp.org/index.php/Server_Side_Request_Forgery)
> - OWASP: [XSS Unvalidated Redirects and Forwards Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html)
> - Common Weakness Enumeration: [CWE-20](https://cwe.mitre.org/data/definitions/20.html)
  • Loading branch information
torrocus committed Oct 12, 2023
1 parent c436106 commit d670bd7
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion sw-register.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ workbox.routing.registerRoute(

// third party files
workbox.routing.registerRoute(
/^https?:\/\/cdn.jsdelivr.net/,
/^https?:\/\/cdn\.jsdelivr\.net/,
new workbox.strategies.StaleWhileRevalidate()
);

0 comments on commit d670bd7

Please sign in to comment.