Commit ef6d6a84 authored by Axel Chalon's avatar Axel Chalon Committed by Amaury Martiny
Browse files

Refactor URLs we allow navigation to (#556)

* Add note that the will-navigate checks aren't fired

* Use default TLS certificate verification handler

The linked document warns against explicitly opting out from
TLS validation. This doesn't require setting a custom handler.

* Refactor trusted navigation urls

* Fix regex typo
parent 6d67ec27
Pipeline #51652 passed with stages
in 15 minutes and 59 seconds
......@@ -51,8 +51,7 @@
"i18next-node-fs-backend": "^2.0.0",
"pino": "^4.16.1",
"pino-multi-stream": "^3.1.2",
"source-map-support": "^0.5.10",
"url-pattern": "^1.0.3"
"source-map-support": "^0.5.10"
},
"devDependencies": {
"copyfiles": "^2.1.0",
......
......@@ -5,71 +5,13 @@
import debounce from 'lodash/debounce';
import { SECURITY_OPTIONS } from '../options/config';
import Pino from '../utils/pino';
const { TRUSTED_HOSTS } = SECURITY_OPTIONS.fetherNetwork;
const trustedHostsAll = Object.values(TRUSTED_HOSTS).flat();
const pino = Pino();
function setupWinListeners (fetherApp) {
const { onWindowClose, processSaveWinPosition, win } = fetherApp;
/**
* Insecure TLS Validation - verify the application does not explicitly opt-out of TLS validation
*
* References:
* - https://doyensec.com/resources/us-17-Carettoni-Electronegativity-A-Study-Of-Electron-Security-wp.pdf
* - https://electronjs.org/docs/api/session#sessetcertificateverifyprocproc
*/
win.webContents.session.setCertificateVerifyProc((request, callback) => {
const { hostname, certificate, verificationResult, errorCode } = request; // eslint-disable-line
pino.debug(
'Processing server certificate verification request for the session in setCertificateVerifyProc with hostname: ',
hostname
);
if (errorCode) {
pino.error(
'Error processing server certificate verification request for the session in setCertificateVerifyProc: ',
errorCode
);
// Failure accepting certificate due to errorCode
callback(-2); // eslint-disable-line
} else if (!trustedHostsAll.includes(hostname)) {
pino.info(
'Failure accepting server certification due to its hostname being an untrusted host in setCertificateVerifyProc: ',
hostname
);
// Failure accepting server certificate due to its source hostname being untrusted
callback(-2); // eslint-disable-line
} else if (!verificationResult === 'net::OK') {
pino.info(
'Failure accepting server certificate due to it failing Chromium verification: ',
hostname,
verificationResult
);
// Failure accepting server certificate due to it failing Chromium verification
callback(-2); // eslint-disable-line
} else {
pino.info(
'Fallback to using the verification result from Chromium: ',
hostname,
verificationResult
);
// Fallback to using the verification result from Chromium
callback(-3); // eslint-disable-line
// // Success and accept the certifcate, disable Certificate Transparency verification
// callback(0); // eslint-disable-line
}
});
// Windows and Linux (unchecked on others)
win.on('move', () => {
/**
......
......@@ -21,27 +21,6 @@ pino.info(
`Running Fether in ${IS_PROD ? 'production' : 'development'} environment`
);
/**
* Note: We disallow users from using Fether
* with a remote node.
* WARNING: SSH tunnels from an attacker are still possible.
*/
const DEFAULT_HTTP_PORT = '3000';
const TRUSTED_HOSTS = {
github: ['api.github.com', 'github.com', 'raw.githubusercontent.com'],
blockscout: ['blockscout.com']
};
const TRUSTED_WS_PORTS = [DEFAULT_WS_PORT];
const DEFAULT_HTTP_TRUSTED_LOOPBACK = `http://${TRUSTED_LOOPBACK}:${DEFAULT_HTTP_PORT}`;
const TRUSTED_URLS = [
DEFAULT_HTTP_TRUSTED_LOOPBACK,
`ws://${TRUSTED_LOOPBACK}:${DEFAULT_WS_PORT}`,
'https://parity.io',
'https://wiki.parity.io/Fether-FAQ',
'https://github.com/paritytech/fether/issues/new',
'https://api.github.com/repos/paritytech/fether/releases/latest'
];
// https://electronjs.org/docs/tutorial/security#electron-security-warnings
process.env.ELECTRON_ENABLE_SECURITY_WARNINGS = true;
......@@ -124,10 +103,7 @@ const SECURITY_OPTIONS = {
fetherNetwork: {
DEFAULT_CHAIN,
DEFAULT_WS_PORT,
TRUSTED_HOSTS,
TRUSTED_LOOPBACK,
TRUSTED_URLS,
TRUSTED_WS_PORTS
TRUSTED_LOOPBACK
},
webPreferences: {
/**
......
......@@ -5,13 +5,7 @@
import { DEFAULT_OPTIONS, SECURITY_OPTIONS, TASKBAR_OPTIONS } from './config';
export default (withTaskbar, customOptions) =>
export default withTaskbar =>
withTaskbar
? Object.assign(
{},
DEFAULT_OPTIONS,
TASKBAR_OPTIONS,
customOptions || {},
SECURITY_OPTIONS
)
: Object.assign({}, DEFAULT_OPTIONS, customOptions || {}, SECURITY_OPTIONS);
? Object.assign({}, DEFAULT_OPTIONS, TASKBAR_OPTIONS, SECURITY_OPTIONS)
: Object.assign({}, DEFAULT_OPTIONS, SECURITY_OPTIONS);
// Copyright 2015-2019 Parity Technologies (UK) Ltd.
// This file is part of Parity.
//
// SPDX-License-Identifier: BSD-3-Clause
const BLOCKSCOUT_URL_REGEXP = new RegExp(
'^https://blockscout.com/(eth|etc|poa)/(mainnet|classic|ropsten|kovan|goerli|core|dai|sokol|rinkeby)/(tx|address)/0x[a-fA-F0-9]+/(internal_transactions|token_transfers)$'
);
const ALLOWED_URLS = [
'https://github.com/paritytech/fether/issues/new',
'mailto:press@parity.io',
'https://wiki.parity.io/Fether-FAQ#how-to-fix-a-webcam-error',
'https://opensource.org/licenses/BSD-3-Clause',
'https://paritytech.io/legal.html'
];
function isValidNavigationUrl (url) {
return BLOCKSCOUT_URL_REGEXP.test(url) || ALLOWED_URLS.includes(url);
}
export default isValidNavigationUrl;
// Copyright 2015-2019 Parity Technologies (UK) Ltd.
// This file is part of Parity.
//
// SPDX-License-Identifier: BSD-3-Clause
import UrlPattern from 'url-pattern';
import { SECURITY_OPTIONS } from '../options/config';
const { TRUSTED_HOSTS } = SECURITY_OPTIONS.fetherNetwork;
// Github
const GITHUB_TRUSTED_HOSTS = TRUSTED_HOSTS.github;
// Blockscout - Only includes those that are available on the Blockscout website
const BLOCKSCOUT_TRUSTED_HOSTS = TRUSTED_HOSTS.blockscout;
const BLOCKSCOUT_CHAINS_SUPPORTED = ['eth', 'etc', 'poa'];
const BLOCKSCOUT_NETWORKS_SUPPORTED = [
'mainnet',
'classic',
'ropsten',
'kovan',
'goerli',
'core',
'dai',
'sokol',
'rinkeby'
];
const BLOCKSCOUT_HASH_KINDS = ['tx', 'address'];
const BLOCKSCOUT_HASH_TRAILERS = [
'coin_balances',
'internal_transactions',
'tokens',
'transactions'
];
const HASH_ADDRESS_LENGTH = 40;
const HASH_TX_LENGTH = 64;
// Url Patterns
const GENERAL_PATTERN = new UrlPattern(
'(https\\://)(:subdomain.):domain.:tld(\\::port)(/*)'
);
const GITHUB_PATTERN_1 = new UrlPattern(
'(https\\://)(:subdomain.):domain.:tld(/)(*)(.png|.jpg)'
);
const GITHUB_PATTERN_2 = new UrlPattern(
'(https\\://)(:subdomain.):domain.:tld(/)atomiclabs/cryptocurrency-icons/(*)(.png|.jpg)'
);
const GITHUB_PATTERN_3 = new UrlPattern(
'(https\\://)(:subdomain.):domain.:tld(/)ethcore/(*)(.png|.jpg)'
);
const BLOCKSCOUT_PATTERN = new UrlPattern(
'(https\\://)(:subdomain.):domain.:tld(/):chain(/):network(/):hashKind(/0x):hash(/:hashTrailer)'
);
function isValidGithubUrl (url) {
const match =
GITHUB_PATTERN_1.match(url) ||
GITHUB_PATTERN_2.match(url) ||
GITHUB_PATTERN_3.match(url);
if (!match) {
return false;
}
return true;
}
function isValidBlockscoutUrl (url) {
const match = BLOCKSCOUT_PATTERN.match(url);
if (!match) {
return false;
}
if (
BLOCKSCOUT_CHAINS_SUPPORTED.includes(match.chain) &&
BLOCKSCOUT_NETWORKS_SUPPORTED.includes(match.network) &&
BLOCKSCOUT_HASH_KINDS.includes(match.hashKind) &&
[HASH_ADDRESS_LENGTH, HASH_TX_LENGTH].includes(match.hash.length) &&
BLOCKSCOUT_HASH_TRAILERS.includes(match.hashTrailer)
) {
return true;
}
return false;
}
/**
* List of trusted subdomains, domain extensions, and ports, as relevant.
* Detect trusted prefix and then call method to validate it.
*/
function isValidUrl (url) {
const match = GENERAL_PATTERN.match(url);
if (!match) {
return false;
}
if (!url.startsWith('https')) {
return false;
}
// Blockscout URL
if (
BLOCKSCOUT_TRUSTED_HOSTS.includes(
(match.subdomain && match.subdomain) +
(match.subdomain && '.') +
(match.domain && match.domain) +
(match.domain && '.') +
(match.tld && match.tld)
) ||
BLOCKSCOUT_TRUSTED_HOSTS.includes(
(match.domain && match.domain) +
(match.domain && '.') +
(match.tld && match.tld)
)
) {
return isValidBlockscoutUrl(url);
}
// Github URL
if (
GITHUB_TRUSTED_HOSTS.includes(
(match.subdomain && match.subdomain) +
(match.subdomain && '.') +
(match.domain && match.domain) +
(match.domain && '.') +
(match.tld && match.tld)
) ||
GITHUB_TRUSTED_HOSTS.includes(
(match.domain && match.domain) +
(match.domain && '.') +
(match.tld && match.tld)
)
) {
return isValidGithubUrl(url);
}
}
export default isValidUrl;
// Copyright 2015-2019 Parity Technologies (UK) Ltd.
// This file is part of Parity.
//
// SPDX-License-Identifier: BSD-3-Clause
/* eslint-env jest */
import isTrustedUrlPattern from './isTrustedUrlPattern';
jest.mock('./pino', () => () => ({
info: () => {}
}));
let url;
describe('trust url pattern', () => {
describe('blockscout', () => {
test('should not trust insecure (non-https) links', () => {
url = 'http://blockscout.com';
expect(isTrustedUrlPattern(url)).toBe(false);
});
test('should trust link to internal transaction on ethereum mainnet', () => {
url =
'https://blockscout.com/eth/mainnet/tx/0xfe7e97d1de24b47d92e815024757e388809425f1681920bc1923368ec1f0fcf1/internal_transactions';
expect(isTrustedUrlPattern(url)).toBe(true);
});
});
describe('github token icons', () => {
test('should trust token .png icons from Github ethcore', () => {
url =
'https://raw.githubusercontent.com/ethcore/dapp-assets/9e135f76fe9ba61e2d8ccbd72ed144c26c450780/tokens/gavcoin-64x64.png';
expect(isTrustedUrlPattern(url)).toBe(true);
});
test('should trust token .svg icons from Github ethcore', () => {
url =
'https://raw.githubusercontent.com/ethcore/dapp-assets/9e135f76fe9ba61e2d8ccbd72ed144c26c450780/tokens/gavcoin-64x64.svg';
expect(isTrustedUrlPattern(url)).toBe(true);
});
test('should trust token icons from Github atomiclabs', () => {
url =
'https://raw.githubusercontent.com/atomiclabs/cryptocurrency-icons/9867bdb19da14e63ffbe63805298fa60bf255cdd/32/black/arg.png';
expect(isTrustedUrlPattern(url)).toBe(true);
});
});
describe('chrome developer tools', () => {
url =
'chrome-devtools://devtools/bundled/toolbox.html?remoteBase=https://chrome-devtools-frontend.appspot.com/serve_file/@123/&can_dock=true&toolbarColor=rgba(223,223,223,1)&textColor=rgba(0,0,0,1)&experiments=true';
expect(isTrustedUrlPattern(url)).toBe(false);
});
});
......@@ -8,14 +8,12 @@ import { URL } from 'url';
import { killParity } from '@parity/electron';
import Pino from './app/utils/pino';
import isTrustedUrlPattern from './app/utils/isTrustedUrlPattern';
import isTrustedNavigationUrl from './app/utils/isTrustedNavigationUrl';
import FetherApp from './app';
import { SECURITY_OPTIONS } from './app/options/config';
import fetherAppOptions from './app/options';
const pino = Pino();
const { app, shell } = electron;
const { TRUSTED_URLS } = SECURITY_OPTIONS.fetherNetwork;
let withTaskbar = process.env.TASKBAR !== 'false';
......@@ -32,7 +30,7 @@ if (!['darwin', 'win32'].includes(process.platform)) {
}
let fetherApp;
const options = fetherAppOptions(withTaskbar, {});
const options = fetherAppOptions(withTaskbar);
const gotTheLock = app.requestSingleInstanceLock();
pino.info(
......@@ -104,6 +102,8 @@ app.on('quit', () => {
// Security
app.on('web-contents-created', (eventOuter, win) => {
// TODO: will-navigate is not fired for sandboxed BrowserWindow;
// waiting on a fix: https://github.com/electron/electron/issues/8841
win.on('will-navigate', (event, url) => {
// FIXME - check that parser is memory-safe
//
......@@ -116,14 +116,13 @@ app.on('web-contents-created', (eventOuter, win) => {
parsedUrl.href
);
if (
!TRUSTED_URLS.includes(parsedUrl.href) &&
!isTrustedUrlPattern(parsedUrl.href)
) {
if (!isTrustedNavigationUrl(parsedUrl.href)) {
pino.info(
'Unable to navigate to untrusted content url due to will-navigate listener: ',
parsedUrl.href
);
event.preventDefault();
}
});
......@@ -150,7 +149,7 @@ app.on('web-contents-created', (eventOuter, win) => {
parsedUrl.href
);
if (!TRUSTED_URLS.includes(parsedUrl.href) && !isTrustedUrlPattern(url)) {
if (!isTrustedNavigationUrl(parsedUrl.href)) {
pino.info(
'Unable to open new window with untrusted content url due to new-window listener: ',
parsedUrl.href
......@@ -159,10 +158,7 @@ app.on('web-contents-created', (eventOuter, win) => {
return;
}
// FIXME - Note that we need to check for a valid certificate in 'certificate-error' event handler
// so we only allow trusted content.
// See https://electronjs.org/docs/tutorial/security#14-do-not-use-openexternal-with-untrusted-content
shell.openExternal(url);
shell.openExternal(parsedUrl.href);
}
);
......
......@@ -15348,11 +15348,6 @@ url-parse@^1.4.3:
querystringify "^2.0.0"
requires-port "^1.0.0"
 
url-pattern@^1.0.3:
version "1.0.3"
resolved "https://registry.yarnpkg.com/url-pattern/-/url-pattern-1.0.3.tgz#0409292471b24f23c50d65a47931793d2b5acfc1"
integrity sha1-BAkpJHGyTyPFDWWkeTF5PStaz8E=
url-template@^2.0.8:
version "2.0.8"
resolved "https://registry.yarnpkg.com/url-template/-/url-template-2.0.8.tgz#fc565a3cccbff7730c775f5641f9555791439f21"
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment