Prevent creating script tags from embedding stylesheets (#156)

* Prevent security concern from preload media option * Update yarn.lock * Validates CSS path before loading the file * Prevent creating script tags from embedding stylesheets * Remove launch.json * Add test case * Fix review comments
GoogleChromeLabs · Feb 23, 2024 · 6a75baf · 6a75baf
1 parent 2e8cbe8
commit 6a75baf
Show file tree

Hide file tree

Showing 5 changed files with 509 additions and 260 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -53,8 +53,7 @@
       "node_modules",
       "dist"
     ],
-    "transformIgnorePatterns": [
-    ]
+    "transformIgnorePatterns": []
   },
   "prettier": {
     "singleQuote": true,
@@ -82,6 +81,7 @@
     "@types/jest": "^26.0.23",
     "babel-core": "^6.26.0",
     "babel-jest": "^26.3.0",
+    "cheerio": "^1.0.0-rc.12",
     "eslint": "^7.6.0",
     "eslint-config-prettier": "^6.11.0",
     "eslint-config-standard": "^14.1.1",

diff --git a/packages/critters/src/css.js b/packages/critters/src/css.js
@@ -40,6 +40,10 @@ export function serializeStylesheet(ast, options) {
   let cssStr = '';
 
   stringify(ast, (result, node, type) => {
+    if (node?.type === 'decl' && node.value.includes('</style>')) {
+      return;
+    }
+
     if (!options.compress) {
       cssStr += result;
       return;

diff --git a/packages/critters/test/security.test.js b/packages/critters/test/security.test.js
@@ -0,0 +1,69 @@
+import Critters from '../src/index';
+import * as cheerio from 'cheerio';
+
+function hasEvilOnload(html) {
+  const $ = cheerio.load(html, { scriptingEnabled: true });
+  return $('[onload]').attr('onload').includes(`''-alert(1)-''`);
+}
+
+function hasEvilScript(html) {
+  const $ = cheerio.load(html, { scriptingEnabled: true });
+  const scripts = Array.from($('script'));
+  return scripts.some((s) => s.textContent.trim() === 'alert(1)');
+}
+
+describe('Critters', () => {
+  it('should not decode entities', async () => {
+    const critters = new Critters({});
+    const html = await critters.process(`
+            <html>
+                <body>
+                    &lt;script&gt;alert(1)&lt;/script&gt;
+        `);
+    expect(hasEvilScript(html)).toBeFalsy();
+  });
+  it('should not create a new script tag from embedding linked stylesheets', async () => {
+    const critters = new Critters({});
+    critters.readFile = () =>
+      `* { background: url('</style><script>alert(1)</script>') }`;
+    const html = await critters.process(`
+            <html>
+                <head>
+                    <link rel=stylesheet href=/file.css>
+                </head>
+                <body>
+                </body>
+        `);
+    expect(hasEvilScript(html)).toBeFalsy();
+  });
+  it('should not create a new script tag from embedding additional stylesheets', async () => {
+    const critters = new Critters({
+      additionalStylesheets: ['/style.css']
+    });
+    critters.readFile = () =>
+      `* { background: url('</style><script>alert(1)</script>') }`;
+    const html = await critters.process(`
+            <html>
+                <head>
+
+                </head>
+                <body>
+                </body>
+        `);
+    expect(hasEvilScript(html)).toBeFalsy();
+  });
+
+  it('should not create a new script tag by ending </script> from href', async () => {
+    const critters = new Critters({ preload: 'js' });
+    critters.readFile = () => `* { background: red }`;
+    const html = await critters.process(`
+        <html>
+            <head>
+                <link rel=stylesheet href="/abc/</script><script>alert(1)</script>/style.css">
+            </head>
+            <body>
+            </body>
+    `);
+    expect(hasEvilScript(html)).toBeFalsy();
+  });
+});