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

cli(run): prevent file:// from running, return error #5936

Merged
merged 5 commits into from
Sep 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions lighthouse-core/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ const log = require('lighthouse-logger');
const ChromeProtocol = require('./gather/connections/cri.js');
const Config = require('./config/config');

const URL = require('./lib/url-shim.js');
const LHError = require('./lib/lh-error.js');

/** @typedef {import('./gather/connections/connection.js')} Connection */

/*
Expand All @@ -36,6 +39,11 @@ const Config = require('./config/config');
* @return {Promise<LH.RunnerResult|undefined>}
*/
async function lighthouse(url, flags = {}, configJSON, connection) {
// verify the url is valid and that protocol is allowed
if (url && (!URL.isValid(url) || !URL.isProtocolAllowed(url))) {
throw new LHError(LHError.errors.INVALID_URL);
}

// set logging preferences, assume quiet
flags.logLevel = flags.logLevel || 'error';
log.setLevel(flags.logLevel);
Expand Down
6 changes: 6 additions & 0 deletions lighthouse-core/lib/lh-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,12 @@ const ERRORS = {
code: 'REQUEST_CONTENT_TIMEOUT',
message: strings.requestContentTimeout,
},

// URL parsing failures
INVALID_URL: {
code: 'INVALID_URL',
message: strings.urlInvalid,
},
};

/** @type {Record<keyof typeof ERRORS, LighthouseErrorDefinition>} */
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/lib/strings.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ module.exports = {
pageLoadFailed: `Your page failed to load. Verify that the URL is valid and re-run Lighthouse.`,
internalChromeError: `An internal Chrome error occurred. Please restart Chrome and try re-running Lighthouse.`,
requestContentTimeout: 'Fetching resource content has exceeded the allotted time',
urlInvalid: `The URL you have provided appears to be invalid.`,
};
19 changes: 19 additions & 0 deletions lighthouse-core/lib/url-shim.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ const listOfTlds = [
'com', 'co', 'gov', 'edu', 'ac', 'org', 'go', 'gob', 'or', 'net', 'in', 'ne', 'nic', 'gouv',
'web', 'spb', 'blog', 'jus', 'kiev', 'mil', 'wi', 'qc', 'ca', 'bel', 'on',
];

const allowedProtocols = [
'https:', 'http:', 'chrome:',
];

/**
* There is fancy URL rewriting logic for the chrome://settings page that we need to work around.
* Why? Special handling was added by Chrome team to allow a pushState transition between chrome:// pages.
Expand Down Expand Up @@ -184,6 +189,20 @@ class URLShim extends URL {
return false;
}
}

/**
* Determine if the url has a protocol that we're able to test
* @param {string} url
* @return {boolean}
*/
static isProtocolAllowed(url) {
try {
const parsed = new URL(url);
return allowedProtocols.includes(parsed.protocol);
} catch (e) {
return false;
}
}
}

URLShim.URL = URL;
Expand Down
24 changes: 21 additions & 3 deletions lighthouse-core/test/index-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ describe('Module Tests', function() {
});

it('should throw an error when the second parameter is not an object', function() {
return lighthouse('SOME_URL', 'flags')
return lighthouse('chrome://version', 'flags')
.then(() => {
throw new Error('Should not have resolved when second arg is not an object');
}, err => {
Expand All @@ -61,7 +61,7 @@ describe('Module Tests', function() {
});

it('should throw an error when the config is invalid', function() {
return lighthouse('SOME_URL', {}, {})
return lighthouse('chrome://version', {}, {})
.then(() => {
throw new Error('Should not have resolved when second arg is not an object');
}, err => {
Expand All @@ -70,7 +70,7 @@ describe('Module Tests', function() {
});

it('should throw an error when the config contains incorrect audits', function() {
return lighthouse('SOME_URL', {}, {
return lighthouse('chrome://version', {}, {
passes: [{
gatherers: [
'viewport',
Expand All @@ -87,6 +87,24 @@ describe('Module Tests', function() {
});
});

it('should throw an error when the url is invalid', function() {
return lighthouse('https:/i-am-not-valid', {}, {})
.then(() => {
throw new Error('Should not have resolved when url is invalid');
}, err => {
assert.ok(err);
});
});

it('should throw an error when the url is invalid protocol (file:///)', function() {
return lighthouse('file:///a/fake/index.html', {}, {})
.then(() => {
throw new Error('Should not have resolved when url is file:///');
}, err => {
assert.ok(err);
});
});

it('should return formatted LHR when given no categories', function() {
const exampleUrl = 'https://www.reddit.com/r/nba';
return lighthouse(exampleUrl, {
Expand Down
12 changes: 12 additions & 0 deletions lighthouse-core/test/lib/url-shim-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,18 @@ describe('URL Shim', () => {
assert.equal(URL.isValid('eval(<context>):45:16'), false);
});

it('safely identifies allowed URL protocols', () => {
assert.ok(URL.isProtocolAllowed('http://google.com/'));
assert.ok(URL.isProtocolAllowed('https://google.com/'));
assert.ok(URL.isProtocolAllowed('chrome://version'));
});

it('safely identifies disallowed URL protocols', () => {
assert.equal(URL.isProtocolAllowed('file:///i/am/a/fake/file.html'), false);
assert.equal(URL.isProtocolAllowed('ftp://user:password@private.ftp.example.com/index.html'), false);
assert.equal(URL.isProtocolAllowed('gopher://underground:9090/path'), false);
});

it('safely identifies same hosts', () => {
const urlA = 'https://5321212.fls.net/page?query=string#hash';
const urlB = 'http://5321212.fls.net/deeply/nested/page';
Expand Down