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

Added detection for specific Polyfill.io CDN compromise - edited existing library and added new query and tests #16886

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

aegilops
Copy link
Contributor

@aegilops aegilops commented Jul 1, 2024

Polyfill.io was a popular JavaScript polyflll Content Delivery Network (CDN), used to support new web browser standards on older browsers.

In February 2024 the domain was sold, and in June 2024 it was widely publicised that the domain had been used to serve malicious scripts. It was taken down later in that month, leaving a window where sites that used the service could have been compromised.

Subresource Integrity (SRI) would have prevented the undetected replacement of the Polyfill script code.

This CodeQL query builds on a library, semmle/javascript/security/FunctionalityFromUntrustedSource.qll, which was abstracted from an existing query, javascript/ql/src/Security/CWE-830/FunctionalityFromUntrustedSource.ql, to detect specific cases where a script from polyfill.io or cdn.polyfill.io was used without Subresource Integrity, and so the service using it was exposed to compromise.

I've modified that existing library to add polyfill.io and cdn.polyfill.io as domains that require SRI, which makes the existing query detect such uses, and I have added a getUrl() predicate to classes in the library so that I can filter down only to results from polyfill.io in a new query.

I added a specific test for polyfill.io use in its most common context, in an HTML script tag.

This query will work for cases in HTML or in JavaScript code, but will not find cases in any other languages such as PHP or Python where server-side scripting is used to generate client-side HTML or JavaScript.

A code search across github.com hosted sites will find more results manually.

@aegilops aegilops requested a review from a team as a code owner July 1, 2024 15:33
Copy link
Contributor

github-actions bot commented Jul 1, 2024

QHelp previews:

javascript/ql/src/Security/CWE-830/PolyfillIOCompromisedScript.qhelp

errors/warnings:

/home/runner/work/codeql/codeql/javascript/ql/src/Security/CWE-830/PolyfillIOCompromisedScript.qhelp:9:6: element "p" not allowed here; expected the element end-tag, text or element "a", "b", "code", "em", "i", "img", "strong", "sub", "sup" or "tt"
/home/runner/work/codeql/codeql/javascript/ql/src/Security/CWE-830/PolyfillIOCompromisedScript.qhelp:11:6: element "p" not allowed here; expected the element end-tag, text or element "a", "b", "code", "em", "i", "img", "strong", "sub", "sup" or "tt"
/home/runner/work/codeql/codeql/javascript/ql/src/Security/CWE-830/PolyfillIOCompromisedScript.qhelp:15:6: element "p" not allowed here; expected the element end-tag, text or element "a", "b", "code", "em", "i", "img", "strong", "sub", "sup" or "tt"
/home/runner/work/codeql/codeql/javascript/ql/src/Security/CWE-830/PolyfillIOCompromisedScript.qhelp:17:6: element "p" not allowed here; expected the element end-tag, text or element "a", "b", "code", "em", "i", "img", "strong", "sub", "sup" or "tt"
/home/runner/work/codeql/codeql/javascript/ql/src/Security/CWE-830/PolyfillIOCompromisedScript.qhelp:25:6: element "p" not allowed here; expected the element end-tag, text or element "a", "b", "code", "em", "i", "img", "strong", "sub", "sup" or "tt"
/home/runner/work/codeql/codeql/javascript/ql/src/Security/CWE-830/PolyfillIOCompromisedScript.qhelp:34:6: element "p" not allowed here; expected the element end-tag, text or element "a", "b", "code", "em", "i", "img", "strong", "sub", "sup" or "tt"
/home/runner/work/codeql/codeql/javascript/ql/src/Security/CWE-830/PolyfillIOCompromisedScript.qhelp:40:4: The element type "p" must be terminated by the matching end-tag "</p>".
A fatal error occurred: 1 qhelp files could not be processed.

Copy link
Contributor

github-actions bot commented Jul 1, 2024

QHelp previews:

javascript/ql/src/Security/CWE-830/PolyfillIOCompromisedScript.qhelp

errors/warnings:

/home/runner/work/codeql/codeql/javascript/ql/src/Security/CWE-830/PolyfillIOCompromisedScript.qhelp: Could not find sample polyfill-cloudflare-check.html
A fatal error occurred: 1 qhelp files could not be processed.

Copy link
Contributor

github-actions bot commented Jul 1, 2024

QHelp previews:

javascript/ql/src/Security/CWE-830/PolyfillIOCompromisedScript.qhelp

Polyfill.io script use

Polyfill.io was a popular JavaScript polyflll Content Delivery Network (CDN), used to support new web browser standards on older browsers.

In February 2024 the domain was sold, and in June 2024 it was widely publicised that the domain had been used to serve malicious scripts. It was taken down later in that month, leaving a window where sites that used the service could have been compromised.

Including a resource from an untrusted source or using an untrusted channel may allow an attacker to include arbitrary code in the response. When including an external resource (for example, a script element or an iframe element) on a page, it is important to ensure that the received data is not malicious.

Even when https is used, an attacker might still compromise the server. When you use a script element, you should check for subresource integrity - that is, you can check the contents of the data received by supplying a cryptographic digest of the expected sources to the script element. The script will only load sources that match the digest and an attacker will be unable to modify the script even when the server is compromised.

Subresource integrity checking is commonly recommended when importing a fixed version of a library - for example, from a CDN (content-delivery network). Then, the fixed digest of that version of the library can easily be added to the script element's integrity attribute.

Recommendation

To help mitigate the risk of including a compromised script, consider whether you need to use a polyfill at all, can use a different polyfill CDN service, or could host an uncompromised version of the polyfill yourself.

When using a script element to load a script, it is important to use an https URL and to check subresource integrity.

Example

The following example loads the Polyfill.io library from the polyfill.io CDN without checking subresource integrity. This use is open to malicious scripts being served by the CDN.

<html>
    <head>
        <title>Polyfill.io demo</title>
        <script src="https://cdn.polyfill.io/v2/polyfill.min.js" crossorigin="anonymous"></script>
    </head>
    <body>
        ...
    </body>
</html>

Instead, loading the Polyfill library from a trusted CDN, and checking subresource integrity is recommended, as in the next example.

<html>
    <head>
        <title>Polyfill demo - Cloudflare hosted with pinned version and integrity checking</title>
        <script src="https://cdnjs.cloudflare.com/polyfill/v3/polyfill.min.js?version=4.8.0" integrity="sha384-3d4jRKquKl90C9aFG+eH4lPJmtbPHgACWHrp+VomFOxF8lzx2jxqeYkhpRg18UWC" crossorigin="anonymous"></script>
    </head>
    <body>
        ...
    </body>
</html>

References

@aegilops aegilops mentioned this pull request Jul 1, 2024
@aegilops
Copy link
Contributor Author

aegilops commented Jul 1, 2024

Because Polyfill.io dynamically generates its content, SRI is apparently impossible.

I'm going to redraft this tomorrow to account for this.

I will refactor the library I made to separate known-bad domains from those requiring SRI.

I also need to remove any references to SRI being a mitigation, and edit the Cloudflare example somehow to make it work - my current example assumes SRI will function correctly.

@erik-krogh
Copy link
Contributor

Hmmm. From reading the updates in the article: https://sansec.io/research/polyfill-supply-chain-attack it sounds like the problem has been fixed.
The domain no longer exists (see status field in https://who.is/whois/polyfill.io), so https://polyfill.io no longer exists (you can safely click the link, it doesn't work).

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.

None yet

2 participants