Prevent security issues from preload media option (#154)

* Prevent security concern from preload media option

packages/critters/package.json

@@ -52,6 +52,7 @@
     "domhandler": "^5.0.2",
     "htmlparser2": "^8.0.2",
     "postcss": "^8.4.23",
+    "postcss-media-query-parser": "^0.2.3",
     "pretty-bytes": "^5.3.0"
   },
   "devDependencies": {

packages/critters/src/css.js

@@ -15,6 +15,7 @@
  */
 
 import { parse, stringify } from 'postcss';
+import mediaParser from 'postcss-media-query-parser';
 
 /**
  * Parse a textual CSS Stylesheet into a Stylesheet instance.
@@ -191,3 +192,65 @@ function filterSelectors(predicate) {
     this.selectors = this.selectors.filter(predicate);
   }
 }
+
+const MEDIA_TYPES = new Set(['all', 'print', 'screen', 'speech']);
+const MEDIA_KEYWORDS = new Set(['and', 'not', ',']);
+const MEDIA_FEATURES = [
+  'width',
+  'aspect-ratio',
+  'color',
+  'color-index',
+  'grid',
+  'height',
+  'monochrome',
+  'orientation',
+  'resolution',
+  'scan'
+];
+function validateMediaType(node) {
+  const { type: nodeType, value: nodeValue } = node;
+
+  if (nodeType === 'media-type') {
+    return MEDIA_TYPES.has(nodeValue);
+  } else if (nodeType === 'keyword') {
+    return MEDIA_KEYWORDS.has(nodeValue);
+  } else if (nodeType === 'media-feature') {
+    return MEDIA_FEATURES.some((feature) => {
+      return (
+        nodeValue === feature ||
+        nodeValue === `min-${feature}` ||
+        nodeValue === `max-${feature}`
+      );
+    });
+  }
+}
+
+/**
+ *
+ * @param {string} Media query to validate
+ * @returns {boolean}
+ *
+ * This function performs a basic media query validation
+ * to ensure the values passed as part of the 'media' config
+ * is HTML safe and does not cause any injection issue
+ */
+export function validateMediaQuery(query) {
+  const mediaTree = mediaParser(query);
+  const nodeTypes = new Set(['media-type', 'keyword', 'media-feature']);
+
+  const stack = [mediaTree];
+
+  while (stack.length > 0) {
+    const node = stack.pop();
+
+    if (nodeTypes.has(node.type) && !validateMediaType(node)) {
+      return false;
+    }
+
+    if (node.nodes) {
+      stack.push(...node.nodes);
+    }
+  }
+
+  return true;
+}

packages/critters/src/index.js

@@ -24,7 +24,8 @@ import {
   walkStyleRules,
   walkStyleRulesWithReverseMirror,
   markOnly,
-  applyMarkedSelectors
+  applyMarkedSelectors,
+  validateMediaQuery
 } from './css';
 import { createLogger } from './util';
 
@@ -311,7 +312,11 @@ export default class Critters {
    */
   async embedLinkedStylesheet(link, document) {
     const href = link.getAttribute('href');
-    const media = link.getAttribute('media');
+    let media = link.getAttribute('media');
+
+    if (media && !validateMediaQuery(media)) {
+      media = undefined;
+    }
 
     const preloadMode = this.options.preload;
 
@@ -361,9 +366,9 @@ export default class Critters {
       link.setAttribute('as', 'style');
       if (preloadMode === 'js' || preloadMode === 'js-lazy') {
         const script = document.createElement('script');
-        const js = `${cssLoaderPreamble}$loadcss(${JSON.stringify(href)}${
-          lazy ? ',' + JSON.stringify(media || 'all') : ''
-        })`;
+        script.setAttribute('data-href', href);
+        script.setAttribute('data-media', media || 'all');
+        const js = `${cssLoaderPreamble}$loadcss(document.currentScript.dataset.href,document.currentScript.dataset.media)`;
         // script.appendChild(document.createTextNode(js));
         script.textContent = js;
         link.parentNode.insertBefore(script, link.nextSibling);
@@ -548,7 +553,7 @@ export default class Critters {
             sel = sel
               .replace(/(?<!\\)::?[a-z-]+(?![a-z-(])/gi, '')
               .replace(/::?not\(\s*\)/g, '')
-               // Remove tailing or leading commas from cleaned sub selector `is(.active, :hover)` -> `is(.active)`.
+              // Remove tailing or leading commas from cleaned sub selector `is(.active, :hover)` -> `is(.active)`.
               .replace(/\(\s*,/g, '(')
               .replace(/,\s*\)/g, ')')
               .trim();

packages/critters/src/text.css

@@ -0,0 +1,17 @@
+@media screen and (min-width: 480px) {
+  body {
+      background-color: lightgreen;
+  }
+}
+
+#main {
+  border: 1px solid black;
+}
+
+ul li {
+padding: 5px;
+}
+
+@media print and alert('hellp') {
+  padding: 5px;
+}

packages/critters/test/__snapshots__/critters.test.js.snap

@@ -12,6 +12,33 @@ exports[`Critters Basic Usage 1`] = `
 </html>"
 `;
 
+exports[`Critters Prevent injection via media attr 1`] = `
+"<html>
+  <link>
+    <title>Testing</title>
+    <style>h1{color:blue}p{color:purple}.contents{padding:50px;text-align:center}.input-field{padding:10px}.custom-element::part(tab){color:#0c0dcc;border-bottom:transparent solid 2px}.custom-element::part(tab):hover:active{background-color:#0c0d33;color:#ffffff}.custom-element::part(tab):focus{box-shadow:0 0 0 1px #0a84ff inset, 0 0 0 1px #0a84ff,
+    0 0 0 4px rgba(10, 132, 255, 0.3)}div:is(:hover, .active){color:#000}div:is(.selected, :hover){color:#fff}body{height:100%}</style><link rel=\\"stylesheet\\" href=\\"styles.css\\" media=\\"print\\" onload=\\"this.media='all'\\"><noscript><link rel=\\"stylesheet\\" href=\\"styles.css\\"></noscript>
+    <link rel=\\"stylesheet\\" href=\\"styles2.css\\" media=\\"print\\" onload=\\"this.media='screen and (min-width: 480px)'\\"><noscript><link rel=\\"stylesheet\\" href=\\"styles2.css\\" media=\\"screen and (min-width: 480px)\\"></noscript>
+  
+  <body>
+    <div class=\\"container\\">
+      <header>
+        <div class=\\"banner\\">Lorem ipsum dolor sit amet</div>
+      </header>
+    </div>
+    <div class data-critters-container>
+      <h1>Hello World!</h1>
+      <p>This is a paragraph</p>
+      <input class=\\"input-field\\">
+      <div class=\\"contents\\"></div>
+      <div class=\\"selected active\\"></div>
+    </div>
+    <footer class></footer>
+  </body>
+</html>
+"
+`;
+
 exports[`Critters Run on HTML file 1`] = `
 "<html>
   <head>

packages/critters/test/critters.test.js

@@ -110,4 +110,20 @@ describe('Critters', () => {
     `);
     expect(result).toMatch('<h1>Hello World!</h1>');
   });
+
+  test('Prevent injection via media attr', async () => {
+    const critters = new Critters({
+      reduceInlineStyles: false,
+      path: path.join(__dirname, 'src'),
+      preload: 'media'
+    });
+
+    const html = fs.readFileSync(
+      path.join(__dirname, 'src/media-validation.html'),
+      'utf8'
+    );
+
+    const result = await critters.process(html);
+    expect(result).toMatchSnapshot();
+  });
 });

packages/critters/test/src/media-validation.html

@@ -0,0 +1,22 @@
+<html>
+  <link>
+    <title>Testing</title>
+    <link rel="stylesheet" href="styles.css" media="alert(1)"></link>
+    <link rel="stylesheet" href="styles2.css" media="screen and (min-width: 480px)"></link>
+  </head>
+  <body>
+    <div class="container">
+      <header>
+        <div class="banner">Lorem ipsum dolor sit amet</div>
+      </header>
+    </div>
+    <div class="" data-critters-container>
+      <h1>Hello World!</h1>
+      <p>This is a paragraph</p>
+      <input class="input-field" />
+      <div class="contents"></div>
+      <div class="selected active"></div>
+    </div>
+    <footer class=""></footer>
+  </body>
+</html>

packages/critters/test/src/styles2.css

@@ -0,0 +1,3 @@
+body {
+  height: 100%;
+}