Skip to content
This repository has been archived by the owner on Oct 25, 2024. It is now read-only.

Validates CSS path before loading the file #155

Merged
merged 5 commits into from
Feb 23, 2024
Merged

Validates CSS path before loading the file #155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
317c1a0
Prevent security concern from preload media option
janicklas-ralph Feb 4, 2024
d8360e4
Update yarn.lock
janicklas-ralph Feb 4, 2024
3475cb2
Validates CSS path before loading the file
janicklas-ralph Feb 6, 2024
6b2e130
Merge branch 'main' of github.com:GoogleChromeLabs/critters into secu…
janicklas-ralph Feb 23, 2024
aedd05c
Modify isSubpath logic
janicklas-ralph Feb 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/critters/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;

Expand Down
5 changes: 5 additions & 0 deletions packages/critters/src/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import chalk from 'chalk';
import path from 'path';

const LOG_LEVELS = ['trace', 'debug', 'info', 'warn', 'error', 'silent'];

Expand Down Expand Up @@ -38,3 +39,7 @@ export function createLogger(logLevel) {
return logger;
}, {});
}

export function isSubpath(basePath, currentPath) {
return !path.relative(basePath, currentPath).startsWith('..');
}
27 changes: 27 additions & 0 deletions packages/critters/test/__snapshots__/critters.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,30 @@ exports[`Critters Run on HTML file 1`] = `
</html>
"
`;

exports[`Critters Skip invalid path 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}</style><link rel=\\"preload\\" href=\\"styles.css\\" as=\\"style\\">
<link rel=\\"stylesheet\\" href=\\"../../../../../style3.css\\">

<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>
<link rel=\\"stylesheet\\" href=\\"styles.css\\"></body>
</html>
"
`;
38 changes: 38 additions & 0 deletions packages/critters/test/critters.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(`
<html>
<head>
<link rel=stylesheet href=/file.css>
<link rel=stylesheet href=/../../../company-secrets/secret.css>
</head>
<body></body>
</html>
`);
expect(critters.readFile).toHaveBeenCalledWith('/var/www/file.css');
expect(critters.readFile).not.toHaveBeenCalledWith(
'/company-secrets/secret.css'
);
});
});
22 changes: 22 additions & 0 deletions packages/critters/test/src/subpath-validation.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<html>
<link>
<title>Testing</title>
<link rel="stylesheet" href="styles.css"></link>
<link rel="stylesheet" href="../../../../../style3.css"></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>
Loading