Skip to content

Commit

Permalink
core: don't allow analysis of file:// urls (#5936)
Browse files Browse the repository at this point in the history
  • Loading branch information
justinribeiro authored and brendankenny committed Sep 14, 2018
1 parent 20d6fef commit 2d4e112
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 3 deletions.
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

0 comments on commit 2d4e112

Please sign in to comment.