From 2e8cbe845fe6d0deff74f8802402213fcc437414 Mon Sep 17 00:00:00 2001 From: Janicklas Ralph Date: Fri, 23 Feb 2024 14:41:15 -0800 Subject: [PATCH] Validates CSS path before loading the file (#155) * Prevent security concern from preload media option * Update yarn.lock * Validates CSS path before loading the file * Modify isSubpath logic --- packages/critters/src/index.js | 6 ++- packages/critters/src/util.js | 5 +++ .../test/__snapshots__/critters.test.js.snap | 27 +++++++++++++ packages/critters/test/critters.test.js | 38 +++++++++++++++++++ .../critters/test/src/subpath-validation.html | 22 +++++++++++ 5 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 packages/critters/test/src/subpath-validation.html diff --git a/packages/critters/src/index.js b/packages/critters/src/index.js index 938a91e..2b1386e 100644 --- a/packages/critters/src/index.js +++ b/packages/critters/src/index.js @@ -27,7 +27,7 @@ import { applyMarkedSelectors, validateMediaQuery } from './css'; -import { createLogger } from './util'; +import { createLogger, isSubpath } from './util'; /** * The mechanism to use for lazy-loading stylesheets. @@ -253,6 +253,10 @@ export default class Critters { } const filename = path.resolve(outputPath, normalizedPath); + // Check if the resolved path is valid + if (!isSubpath(outputPath, filename)) { + return undefined; + } let sheet; diff --git a/packages/critters/src/util.js b/packages/critters/src/util.js index fdf0e17..7312c36 100644 --- a/packages/critters/src/util.js +++ b/packages/critters/src/util.js @@ -1,4 +1,5 @@ import chalk from 'chalk'; +import path from 'path'; const LOG_LEVELS = ['trace', 'debug', 'info', 'warn', 'error', 'silent']; @@ -38,3 +39,7 @@ export function createLogger(logLevel) { return logger; }, {}); } + +export function isSubpath(basePath, currentPath) { + return !path.relative(basePath, currentPath).startsWith('..'); +} diff --git a/packages/critters/test/__snapshots__/critters.test.js.snap b/packages/critters/test/__snapshots__/critters.test.js.snap index 3856a4d..58a1539 100644 --- a/packages/critters/test/__snapshots__/critters.test.js.snap +++ b/packages/critters/test/__snapshots__/critters.test.js.snap @@ -64,3 +64,30 @@ exports[`Critters Run on HTML file 1`] = ` " `; + +exports[`Critters Skip invalid path 1`] = ` +" + + Testing + + + + +
+
+
Lorem ipsum dolor sit amet
+
+
+
+

Hello World!

+

This is a paragraph

+ +
+
+
+ + + +" +`; diff --git a/packages/critters/test/critters.test.js b/packages/critters/test/critters.test.js index 12efeb8..1478c9d 100644 --- a/packages/critters/test/critters.test.js +++ b/packages/critters/test/critters.test.js @@ -126,4 +126,42 @@ describe('Critters', () => { const result = await critters.process(html); expect(result).toMatchSnapshot(); }); + + test('Skip invalid path', async () => { + const consoleSpy = jest.spyOn(console, 'warn'); + + const critters = new Critters({ + reduceInlineStyles: false, + path: path.join(__dirname, 'src') + }); + + const html = fs.readFileSync( + path.join(__dirname, 'src/subpath-validation.html'), + 'utf8' + ); + + const result = await critters.process(html); + expect(consoleSpy).not.toHaveBeenCalledWith( + expect.stringContaining('Unable to locate stylesheet') + ); + expect(result).toMatchSnapshot(); + }); + + it('should not load stylesheets outside of the base path', async () => { + const critters = new Critters({ path: '/var/www' }); + jest.spyOn(critters, 'readFile'); + await critters.process(` + + + + + + + + `); + expect(critters.readFile).toHaveBeenCalledWith('/var/www/file.css'); + expect(critters.readFile).not.toHaveBeenCalledWith( + '/company-secrets/secret.css' + ); + }); }); diff --git a/packages/critters/test/src/subpath-validation.html b/packages/critters/test/src/subpath-validation.html new file mode 100644 index 0000000..6d5c087 --- /dev/null +++ b/packages/critters/test/src/subpath-validation.html @@ -0,0 +1,22 @@ + + + Testing + + + + +
+
+ +
+
+
+

Hello World!

+

This is a paragraph

+ +
+
+
+ + +