From 17381a6cb519d1e20293b70db00fa8f475a5c08e Mon Sep 17 00:00:00 2001 From: Ben Date: Fri, 21 Feb 2020 16:48:14 -0500 Subject: [PATCH 1/7] Add regex/wildcard listing with unit tests. Add safe-regex and jest-when dependency --- _locales/en/messages.json | 4 +- .../components/Settings/TrustAndRestrict.jsx | 41 +++- .../Settings/__tests__/TrustAndRestrict.jsx | 91 ++++++++ .../__snapshots__/TrustAndRestrict.jsx.snap | 119 +++++++++++ package.json | 2 + src/classes/Policy.js | 33 ++- test/src/policy.test.js | 34 +++ yarn.lock | 202 +++++++++++++++++- 8 files changed, 511 insertions(+), 15 deletions(-) create mode 100644 app/panel/components/Settings/__tests__/TrustAndRestrict.jsx create mode 100644 app/panel/components/Settings/__tests__/__snapshots__/TrustAndRestrict.jsx.snap create mode 100644 test/src/policy.test.js diff --git a/_locales/en/messages.json b/_locales/en/messages.json index 0e5300f9b..6c3bada36 100644 --- a/_locales/en/messages.json +++ b/_locales/en/messages.json @@ -802,7 +802,7 @@ "message": "unblocked" }, "white_black_list_error_invalid_url": { - "message": "Please enter a valid URL." + "message": "Please enter a valid URL, regex, or wildcard." }, "whitelist_error_blacklist_url": { "message": "This site has been removed from your Restricted Sites list and added to your Trusted Sites list." @@ -1058,7 +1058,7 @@ "message": "Trusted Sites" }, "settings_sites_placeholder": { - "message": "example.com" + "message": "example.com (wildcards/regex supported)" }, "settings_restricted_sites": { "message": "Restricted Sites" diff --git a/app/panel/components/Settings/TrustAndRestrict.jsx b/app/panel/components/Settings/TrustAndRestrict.jsx index 53141842a..e91d150c1 100644 --- a/app/panel/components/Settings/TrustAndRestrict.jsx +++ b/app/panel/components/Settings/TrustAndRestrict.jsx @@ -12,6 +12,7 @@ */ import React from 'react'; +import safe from 'safe-regex'; import Sites from './Sites'; /** * @class Implement Trust and Restrict subview presenting the lists @@ -89,8 +90,6 @@ class TrustAndRestrict extends React.Component { * if it has been alreday added to the opposite list. Displays appropriate warnings. */ addSite() { - // from node-validator - const isValidUrlRegex = /^(?!mailto:)(?:(?:https?|ftp):\/\/)?(?:\S+(?::\S*)?@)?(?:(?:(?:[1-9]\d?|1\d\d|2[01]\d|22[0-3])(?:\.(?:1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.(?:[1-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(?:(?:[a-z\u00a1-\uffff0-9]+-?)*[a-z\u00a1-\uffff0-9]+)(?:\.(?:[a-z\u00a1-\uffff0-9]+-?)*[a-z\u00a1-\uffff0-9]+)*(?:\.(?:[a-z\u00a1-\uffff]{2,})))|localhost)(?::\d{2,5})?(?:\/[^\s]*)?$/i; let pageHost; let list; let listType; @@ -121,10 +120,12 @@ class TrustAndRestrict extends React.Component { pageHost = pageHost.toLowerCase().replace(/^(http[s]?:\/\/)?(www\.)?/, ''); // Check for Validity - if (pageHost.length >= 2083 || !isValidUrlRegex.test(pageHost)) { + if (pageHost.length >= 2083 + || !this.isValidUrlWildcardOrRegex(pageHost)) { this.showWarning(t('white_black_list_error_invalid_url')); return; } + // Check for Duplicates if (list.includes(pageHost)) { this.showWarning(duplicateWarning); @@ -139,6 +140,40 @@ class TrustAndRestrict extends React.Component { this.props.actions.updateSitePolicy({ type: listType, pageHost }); } + isValidUrlWildcardOrRegex(pageHost) { + // Check for valid URL + // from node-validator + const isValidUrlRegex = /^(?!mailto:)(?:(?:https?|ftp):\/\/)?(?:\S+(?::\S*)?@)?(?:(?:(?:[1-9]\d?|1\d\d|2[01]\d|22[0-3])(?:\.(?:1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.(?:[1-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(?:(?:[a-z\u00a1-\uffff0-9]+-?)*[a-z\u00a1-\uffff0-9]+)(?:\.(?:[a-z\u00a1-\uffff0-9]+-?)*[a-z\u00a1-\uffff0-9]+)*(?:\.(?:[a-z\u00a1-\uffff]{2,})))|localhost)(?::\d{2,5})?(?:\/[^\s]*)?$/i; + if (isValidUrlRegex.test(pageHost)) return true; + + // Check for valid wildcard + const escapedPattern = pageHost.replace(/[|\\{}()[\]^$+*?.-]/g, '\\$&'); + const wildcardPattern = escapedPattern.replace(/\*/g, '.*'); + let isValidWildcard = true; + try { + // eslint-disable-next-line + new RegExp(wildcardPattern); + } catch { + isValidWildcard = false; + } + + if (isValidWildcard) return true; + + // Prevent ReDoS attack + if (!safe(pageHost)) return false; + + // Check for valid regex + let isValidRegex = true; + try { + // eslint-disable-next-line + new RegExp(pageHost); + } catch { + isValidRegex = false; + } + + return isValidRegex; + } + /** * Save current warning in state. * @param {string} warning warning to save diff --git a/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx b/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx new file mode 100644 index 000000000..b939c294a --- /dev/null +++ b/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx @@ -0,0 +1,91 @@ +/** + * Rewards Test Component + * + * Ghostery Browser Extension + * https://www.ghostery.com/ + * + * Copyright 2019 Ghostery, Inc. All rights reserved. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0 + */ + +import React from 'react'; +import renderer from 'react-test-renderer'; +import { shallow } from 'enzyme'; +import { when } from 'jest-when'; +import TrustAndRestrict from '../TrustAndRestrict'; + +describe('app/panel/components/Settings/TrustAndRestrict', () => { + describe('Snapshot test with react-test-renderer', () => { + test('Testing TrustAndRestrict is rendering', () => { + const wrapper = renderer.create( + + ).toJSON(); + expect(wrapper).toMatchSnapshot(); + }); + }); +}); + +describe('app/panel/components/Settings/', () => { + test('isValidUrlWildcardOrRegex should return true with url entered', () => { + const wrapper = shallow(); + const input = 'ghostery.com'; + + const fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn) + .calledWith(input) + .mockReturnValue(true); + const returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + expect(returnValue).toBe(true); + }); + + test('isValidUrlWildcardOrRegex should return true with wildcard URL entered', () => { + const wrapper = shallow(); + const input = 'developer.*.org'; + + const fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn) + .calledWith(input) + .mockReturnValue(true); + const returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + expect(returnValue).toBe(true); + }); + + test('isValidUrlWildcardOrRegex should return true with regex URL entered', () => { + const wrapper = shallow(); + const input = '[ds]eveloper.mozilla.org'; + + const fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn) + .calledWith(input) + .mockReturnValue(true); + const returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + expect(returnValue).toBe(true); + }); + + test('isValidUrlWildcardOrRegex should return false with unsafe regex entered', () => { + const wrapper = shallow(); + const input = '/^(\w+\s?)*$/'; + + const fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn) + .calledWith(input) + .mockReturnValue(false); + const returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + expect(returnValue).toBe(false); + }); + + test('isValidUrlWildcardOrRegex should return false with incorrect regex format entered', () => { + const wrapper = shallow(); + const input = '[.ghostery.com'; + + const fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn) + .calledWith(input) + .mockReturnValue(false); + const returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + expect(returnValue).toBe(false); + }); +}); diff --git a/app/panel/components/Settings/__tests__/__snapshots__/TrustAndRestrict.jsx.snap b/app/panel/components/Settings/__tests__/__snapshots__/TrustAndRestrict.jsx.snap new file mode 100644 index 000000000..0403041cf --- /dev/null +++ b/app/panel/components/Settings/__tests__/__snapshots__/TrustAndRestrict.jsx.snap @@ -0,0 +1,119 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`app/panel/components/Settings/TrustAndRestrict Snapshot test with react-test-renderer Testing TrustAndRestrict is rendering 1`] = ` +
+
+
+

+ settings_trusted_restricted_sites +

+
+
+
+
+ + settings_trusted_sites + +
+
+ + settings_restricted_sites + +
+
+
+
+
+
+ +
+
+
+ + settings_trusted_sites_description + +
+
+ +
+
+
+
+
+
+
+
+ +
+
+
+ + settings_restricted_sites_description + +
+
+ +
+
+
+
+
+`; diff --git a/package.json b/package.json index b8f162cdb..cb0ea7b71 100644 --- a/package.json +++ b/package.json @@ -64,6 +64,7 @@ "redux-object": "^0.5.10", "redux-thunk": "^2.2.0", "rsvp": "^4.8.5", + "safe-regex": "^2.1.1", "spanan": "^2.0.0", "ua-parser-js": "^0.7.21", "underscore": "^1.9.2", @@ -91,6 +92,7 @@ "eslint-plugin-react": "^7.18.3", "fs-extra": "^8.1.0", "jest": "^25.1.0", + "jest-when": "^2.7.0", "jsdoc": "^3.6.3", "jsonfile": "^5.0.0", "license-checker": "^25.0.1", diff --git a/src/classes/Policy.js b/src/classes/Policy.js index 547784d42..c6af879a5 100644 --- a/src/classes/Policy.js +++ b/src/classes/Policy.js @@ -14,7 +14,6 @@ */ /* eslint no-param-reassign: 0 */ - import c2pDb from './Click2PlayDb'; import conf from './Conf'; import globals from './Globals'; @@ -71,7 +70,8 @@ class Policy { // TODO: speed up for (let i = 0; i < num_sites; i++) { // TODO match from the beginning of the string to avoid false matches (somewhere in the querystring for instance) - if (replacedUrl === sites[i]) { + if (replacedUrl === sites[i] + || this.matchesWildcardOrRegex(replacedUrl, sites[i])) { return sites[i]; } } @@ -119,7 +119,8 @@ class Policy { // TODO: speed up for (let i = 0; i < num_sites; i++) { // TODO match from the beginning of the string to avoid false matches (somewhere in the querystring for instance) - if (replacedUrl === sites[i]) { + if (replacedUrl === sites[i] + || this.matchesWildcardOrRegex(replacedUrl, sites[i])) { return sites[i]; } } @@ -174,6 +175,32 @@ class Policy { } return { block: false, reason: allowedOnce ? BLOCK_REASON_C2P_ALLOWED_ONCE : BLOCK_REASON_GLOBAL_UNBLOCKED }; } + + /** + * Check given url against pattern which might be a wildcard, or a regex + * @param {string} url site url + * @param {string} pattern regex pattern + * @return {boolean} + */ + matchesWildcardOrRegex(url, pattern) { + // Input string might be a wildcard + const escapedPattern = pattern.replace(/[|\\{}()[\]^$+*?.-]/g, '\\$&'); + const wildcardPattern = escapedPattern.replace(/\*/g, '.*'); + const wildcardRegex = RegExp(wildcardPattern); + console.log('url: ', url); + console.log('wildcardPattern: ', wildcardPattern); + + if (wildcardRegex.test(url)) { return true; } + + console.log('pattern: ', pattern); + // or a regex + const regex = RegExp(pattern); + if (regex.test(url)) { return true; } + + console.log('returning false'); + + return false; + } } export default Policy; diff --git a/test/src/policy.test.js b/test/src/policy.test.js new file mode 100644 index 000000000..02e7d7169 --- /dev/null +++ b/test/src/policy.test.js @@ -0,0 +1,34 @@ +import sinon from 'sinon'; +import PolicySmartBlock from '../../src/classes/PolicySmartBlock'; + +let policySmartBlock = new PolicySmartBlock(); +let policy = policySmartBlock.policy; + +// Mock imports for dependencies +jest.mock('../../src/classes/TabInfo', () => {}); + +describe('src/classes/Policy.js', () => { + describe('test functions', () => { + + test('matchesWildcardOrRegex should return true with wildcard entered ', () => { + const input = 'mozilla.*.com'; + const stub = sinon.stub(policy, 'matchesWildcardOrRegex'); + expect(stub.withArgs(input).returns(true)); + stub.restore(); + }); + + test('matchesWildcardOrRegex should return true with regex entered', () => { + const input = '[de]eveloper.mozilla.org'; + const stub = sinon.stub(policy, 'matchesWildcardOrRegex'); + expect(stub.withArgs(input).returns(true)); + stub.restore(); + }); + + test('matchesWildcardOrRegex should return false with ', () => { + const input = '[google.com'; + const stub = sinon.stub(policy, 'matchesWildcardOrRegex'); + expect(stub.withArgs(input).returns(false)); + stub.restore(); + }); + }) +}); diff --git a/yarn.lock b/yarn.lock index 322e594e1..e1f320711 100644 --- a/yarn.lock +++ b/yarn.lock @@ -396,6 +396,15 @@ resolved "https://registry.yarnpkg.com/@istanbuljs/schema/-/schema-0.1.2.tgz#26520bf09abe4a5644cd5414e37125a8954241dd" integrity sha512-tsAQNx32a8CoFhjhijUIhI4kccIAgmGhy8LZMZgGfmXcpMbPRUqn5LWmgRttILi6yeGmBJd2xsPkFMs0PzgPCw== +"@jest/console@^24.9.0": + version "24.9.0" + resolved "https://registry.yarnpkg.com/@jest/console/-/console-24.9.0.tgz#79b1bc06fb74a8cfb01cbdedf945584b1b9707f0" + integrity sha512-Zuj6b8TnKXi3q4ymac8EQfc3ea/uhLeCGThFqXeC8H9/raaH8ARPUTdId+XyGd03Z4In0/VjD2OYFcBF09fNLQ== + dependencies: + "@jest/source-map" "^24.9.0" + chalk "^2.0.1" + slash "^2.0.0" + "@jest/console@^25.1.0": version "25.1.0" resolved "https://registry.yarnpkg.com/@jest/console/-/console-25.1.0.tgz#1fc765d44a1e11aec5029c08e798246bd37075ab" @@ -493,6 +502,15 @@ optionalDependencies: node-notifier "^6.0.0" +"@jest/source-map@^24.9.0": + version "24.9.0" + resolved "https://registry.yarnpkg.com/@jest/source-map/-/source-map-24.9.0.tgz#0e263a94430be4b41da683ccc1e6bffe2a191714" + integrity sha512-/Xw7xGlsZb4MJzNDgB7PW5crou5JqWiBQaz6xyPd3ArOg2nfn/PunV8+olXbbEZzNl591o5rWKE9BRDaFAuIBg== + dependencies: + callsites "^3.0.0" + graceful-fs "^4.1.15" + source-map "^0.6.0" + "@jest/source-map@^25.1.0": version "25.1.0" resolved "https://registry.yarnpkg.com/@jest/source-map/-/source-map-25.1.0.tgz#b012e6c469ccdbc379413f5c1b1ffb7ba7034fb0" @@ -502,6 +520,15 @@ graceful-fs "^4.2.3" source-map "^0.6.0" +"@jest/test-result@^24.9.0": + version "24.9.0" + resolved "https://registry.yarnpkg.com/@jest/test-result/-/test-result-24.9.0.tgz#11796e8aa9dbf88ea025757b3152595ad06ba0ca" + integrity sha512-XEFrHbBonBJ8dGp2JmF8kP/nQI/ImPpygKHwQ/SY+es59Z3L5PI4Qb9TQQMAEeYsThG1xF0k6tmG0tIKATNiiA== + dependencies: + "@jest/console" "^24.9.0" + "@jest/types" "^24.9.0" + "@types/istanbul-lib-coverage" "^2.0.0" + "@jest/test-result@^25.1.0": version "25.1.0" resolved "https://registry.yarnpkg.com/@jest/test-result/-/test-result-25.1.0.tgz#847af2972c1df9822a8200457e64be4ff62821f7" @@ -545,6 +572,15 @@ source-map "^0.6.1" write-file-atomic "^3.0.0" +"@jest/types@^24.9.0": + version "24.9.0" + resolved "https://registry.yarnpkg.com/@jest/types/-/types-24.9.0.tgz#63cb26cb7500d069e5a389441a7c6ab5e909fc59" + integrity sha512-XKK7ze1apu5JWQ5eZjHITP66AX+QsLlbaJRBGYr8pNzwcAE2JVkwnf0yqjHTsDRcjR0mujy/NmZMXw5kl+kGBw== + dependencies: + "@types/istanbul-lib-coverage" "^2.0.0" + "@types/istanbul-reports" "^1.1.1" + "@types/yargs" "^13.0.0" + "@jest/types@^25.1.0": version "25.1.0" resolved "https://registry.yarnpkg.com/@jest/types/-/types-25.1.0.tgz#b26831916f0d7c381e11dbb5e103a72aed1b4395" @@ -743,6 +779,13 @@ resolved "https://registry.yarnpkg.com/@types/yargs-parser/-/yargs-parser-15.0.0.tgz#cb3f9f741869e20cce330ffbeb9271590483882d" integrity sha512-FA/BWv8t8ZWJ+gEOnLLd8ygxH/2UFbAvgEonyfN6yWGLKc7zVjbpl2Y4CTjid9h2RfgPP6SEt6uHwEOply00yw== +"@types/yargs@^13.0.0": + version "13.0.8" + resolved "https://registry.yarnpkg.com/@types/yargs/-/yargs-13.0.8.tgz#a38c22def2f1c2068f8971acb3ea734eb3c64a99" + integrity sha512-XAvHLwG7UQ+8M4caKIH0ZozIOYay5fQkAgyIXegXT9jPtdIGdhga+sUEdAr1CiG46aB+c64xQEYyEzlwWVTNzA== + dependencies: + "@types/yargs-parser" "*" + "@types/yargs@^15.0.0": version "15.0.2" resolved "https://registry.yarnpkg.com/@types/yargs/-/yargs-15.0.2.tgz#0bf292a0369493cee030e2e4f4ff84f5982b028d" @@ -1013,7 +1056,7 @@ ansi-regex@^3.0.0: resolved "https://registry.yarnpkg.com/ansi-regex/-/ansi-regex-3.0.0.tgz#ed0317c322064f79466c02966bddb605ab37d998" integrity sha1-7QMXwyIGT3lGbAKWa922Bas32Zg= -ansi-regex@^4.1.0: +ansi-regex@^4.0.0, ansi-regex@^4.1.0: version "4.1.0" resolved "https://registry.yarnpkg.com/ansi-regex/-/ansi-regex-4.1.0.tgz#8b9f8f08cf1acb843756a839ca8c7e3168c51997" integrity sha512-1apePfXM1UOSqw0o9IiFAovVz9M5S1Dg+4TrDwfMewQ6p/rmMueb7tWZjQ1rx4Loy1ArBggoqGpfqqdI4rondg== @@ -1593,6 +1636,16 @@ builtin-status-codes@^3.0.0: resolved "https://registry.yarnpkg.com/builtin-status-codes/-/builtin-status-codes-3.0.0.tgz#85982878e21b98e1c66425e03d0174788f569ee8" integrity sha1-hZgoeOIbmOHGZCXgPQF0eI9Wnug= +bunyan@^1.8.12: + version "1.8.12" + resolved "https://registry.yarnpkg.com/bunyan/-/bunyan-1.8.12.tgz#f150f0f6748abdd72aeae84f04403be2ef113797" + integrity sha1-8VDw9nSKvdcq6uhPBEA74u8RN5c= + optionalDependencies: + dtrace-provider "~0.8" + moment "^2.10.6" + mv "~2" + safe-json-stringify "~1" + cacache@^12.0.2: version "12.0.3" resolved "https://registry.yarnpkg.com/cacache/-/cacache-12.0.3.tgz#be99abba4e1bf5df461cd5a2c1071fc432573390" @@ -1676,7 +1729,7 @@ catharsis@^0.8.11: dependencies: lodash "^4.17.14" -chalk@2.4.2, chalk@^2.0.0, chalk@^2.1.0, chalk@^2.4.1, chalk@^2.4.2: +chalk@2.4.2, chalk@^2.0.0, chalk@^2.0.1, chalk@^2.1.0, chalk@^2.4.1, chalk@^2.4.2: version "2.4.2" resolved "https://registry.yarnpkg.com/chalk/-/chalk-2.4.2.tgz#cd42541677a54333cf541a49108c1432b44c9424" integrity sha512-Mti+f9lpJNcwF4tWV8/OrTTtF1gZi+f8FqlyAdouralcFWFQWF2+NgCHShjkCb+IFBLq9buZwE1xckQU4peSuQ== @@ -2590,6 +2643,11 @@ dezalgo@^1.0.0: asap "^2.0.0" wrappy "1" +diff-sequences@^24.9.0: + version "24.9.0" + resolved "https://registry.yarnpkg.com/diff-sequences/-/diff-sequences-24.9.0.tgz#5715d6244e2aa65f48bba0bc972db0b0b11e95b5" + integrity sha512-Dj6Wk3tWyTE+Fo1rW8v0Xhwk80um6yFYKbuAxc9c3EZxIHFDYwbi34Uk42u1CdnIiVorvt4RmlSDjIPyzGC2ew== + diff-sequences@^25.1.0: version "25.1.0" resolved "https://registry.yarnpkg.com/diff-sequences/-/diff-sequences-25.1.0.tgz#fd29a46f1c913fd66c22645dc75bffbe43051f32" @@ -2718,6 +2776,13 @@ domutils@^2.0.0: domelementtype "^2.0.1" domhandler "^3.0.0" +dtrace-provider@~0.8: + version "0.8.8" + resolved "https://registry.yarnpkg.com/dtrace-provider/-/dtrace-provider-0.8.8.tgz#2996d5490c37e1347be263b423ed7b297fb0d97e" + integrity sha512-b7Z7cNtHPhH9EJhNNbbeqTcXB8LGFFZhq1PGgEvpeHlzd36bhbdTWoE/Ba/YguqpBSlAPKnARWhVlhunCMwfxg== + dependencies: + nan "^2.14.0" + duplexify@^3.4.2, duplexify@^3.6.0: version "3.7.1" resolved "https://registry.yarnpkg.com/duplexify/-/duplexify-3.7.1.tgz#2a4df5317f6ccfd91f86d6fd25d8d8a103b88309" @@ -3211,6 +3276,18 @@ expand-tilde@^2.0.0, expand-tilde@^2.0.2: dependencies: homedir-polyfill "^1.0.1" +expect@^24.8.0: + version "24.9.0" + resolved "https://registry.yarnpkg.com/expect/-/expect-24.9.0.tgz#b75165b4817074fa4a157794f46fe9f1ba15b6ca" + integrity sha512-wvVAx8XIol3Z5m9zvZXiyZOQ+sRJqNTIm6sGjdWlaZIeupQGO3WbYI+15D/AmEwZywL6wtJkbAbJtzkOfBuR0Q== + dependencies: + "@jest/types" "^24.9.0" + ansi-styles "^3.2.0" + jest-get-type "^24.9.0" + jest-matcher-utils "^24.9.0" + jest-message-util "^24.9.0" + jest-regex-util "^24.9.0" + expect@^25.1.0: version "25.1.0" resolved "https://registry.yarnpkg.com/expect/-/expect-25.1.0.tgz#7e8d7b06a53f7d66ec927278db3304254ee683ee" @@ -3637,6 +3714,17 @@ glob-parent@^5.0.0: dependencies: is-glob "^4.0.1" +glob@^6.0.1: + version "6.0.4" + resolved "https://registry.yarnpkg.com/glob/-/glob-6.0.4.tgz#0f08860f6a155127b2fadd4f9ce24b1aab6e4d22" + integrity sha1-DwiGD2oVUSey+t1PnOJLGqtuTSI= + dependencies: + inflight "^1.0.4" + inherits "2" + minimatch "2 || 3" + once "^1.3.0" + path-is-absolute "^1.0.0" + glob@^7.0.0, glob@^7.0.3, glob@^7.1.1, glob@^7.1.2, glob@^7.1.3, glob@^7.1.4, glob@~7.1.1: version "7.1.6" resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.6.tgz#141f33b81a7c2492e125594307480c46679278a6" @@ -4595,6 +4683,16 @@ jest-config@^25.1.0: pretty-format "^25.1.0" realpath-native "^1.1.0" +jest-diff@^24.9.0: + version "24.9.0" + resolved "https://registry.yarnpkg.com/jest-diff/-/jest-diff-24.9.0.tgz#931b7d0d5778a1baf7452cb816e325e3724055da" + integrity sha512-qMfrTs8AdJE2iqrTp0hzh7kTd2PQWrsFyj9tORoKmu32xjPjeE4NyjVRDz8ybYwqS2ik8N4hsIpiVTyFeo2lBQ== + dependencies: + chalk "^2.0.1" + diff-sequences "^24.9.0" + jest-get-type "^24.9.0" + pretty-format "^24.9.0" + jest-diff@^25.1.0: version "25.1.0" resolved "https://registry.yarnpkg.com/jest-diff/-/jest-diff-25.1.0.tgz#58b827e63edea1bc80c1de952b80cec9ac50e1ad" @@ -4646,6 +4744,11 @@ jest-environment-node@^25.1.0: jest-mock "^25.1.0" jest-util "^25.1.0" +jest-get-type@^24.9.0: + version "24.9.0" + resolved "https://registry.yarnpkg.com/jest-get-type/-/jest-get-type-24.9.0.tgz#1684a0c8a50f2e4901b6644ae861f579eed2ef0e" + integrity sha512-lUseMzAley4LhIcpSP9Jf+fTrQ4a1yHQwLNeeVa2cEmbCGeoZAtYPOIv8JaxLD/sUpKxetKGP+gsHl8f8TSj8Q== + jest-get-type@^25.1.0: version "25.1.0" resolved "https://registry.yarnpkg.com/jest-get-type/-/jest-get-type-25.1.0.tgz#1cfe5fc34f148dc3a8a3b7275f6b9ce9e2e8a876" @@ -4700,6 +4803,16 @@ jest-leak-detector@^25.1.0: jest-get-type "^25.1.0" pretty-format "^25.1.0" +jest-matcher-utils@^24.9.0: + version "24.9.0" + resolved "https://registry.yarnpkg.com/jest-matcher-utils/-/jest-matcher-utils-24.9.0.tgz#f5b3661d5e628dffe6dd65251dfdae0e87c3a073" + integrity sha512-OZz2IXsu6eaiMAwe67c1T+5tUAtQyQx27/EMEkbFAGiw52tB9em+uGbzpcgYVpA8wl0hlxKPZxrly4CXU/GjHA== + dependencies: + chalk "^2.0.1" + jest-diff "^24.9.0" + jest-get-type "^24.9.0" + pretty-format "^24.9.0" + jest-matcher-utils@^25.1.0: version "25.1.0" resolved "https://registry.yarnpkg.com/jest-matcher-utils/-/jest-matcher-utils-25.1.0.tgz#fa5996c45c7193a3c24e73066fc14acdee020220" @@ -4710,6 +4823,20 @@ jest-matcher-utils@^25.1.0: jest-get-type "^25.1.0" pretty-format "^25.1.0" +jest-message-util@^24.9.0: + version "24.9.0" + resolved "https://registry.yarnpkg.com/jest-message-util/-/jest-message-util-24.9.0.tgz#527f54a1e380f5e202a8d1149b0ec872f43119e3" + integrity sha512-oCj8FiZ3U0hTP4aSui87P4L4jC37BtQwUMqk+zk/b11FR19BJDeZsZAvIHutWnmtw7r85UmR3CEWZ0HWU2mAlw== + dependencies: + "@babel/code-frame" "^7.0.0" + "@jest/test-result" "^24.9.0" + "@jest/types" "^24.9.0" + "@types/stack-utils" "^1.0.1" + chalk "^2.0.1" + micromatch "^3.1.10" + slash "^2.0.0" + stack-utils "^1.0.1" + jest-message-util@^25.1.0: version "25.1.0" resolved "https://registry.yarnpkg.com/jest-message-util/-/jest-message-util-25.1.0.tgz#702a9a5cb05c144b9aa73f06e17faa219389845e" @@ -4736,6 +4863,11 @@ jest-pnp-resolver@^1.2.1: resolved "https://registry.yarnpkg.com/jest-pnp-resolver/-/jest-pnp-resolver-1.2.1.tgz#ecdae604c077a7fbc70defb6d517c3c1c898923a" integrity sha512-pgFw2tm54fzgYvc/OHrnysABEObZCUNFnhjoRjaVOCN8NYc032/gVjPaHD4Aq6ApkSieWtfKAFQtmDKAmhupnQ== +jest-regex-util@^24.9.0: + version "24.9.0" + resolved "https://registry.yarnpkg.com/jest-regex-util/-/jest-regex-util-24.9.0.tgz#c13fb3380bde22bf6575432c493ea8fe37965636" + integrity sha512-05Cmb6CuxaA+Ys6fjr3PhvV3bGQmO+2p2La4hFbU+W5uOc479f7FdLXUWXw4pYMAhhSZIuKHwSXSu6CsSBAXQA== + jest-regex-util@^25.1.0: version "25.1.0" resolved "https://registry.yarnpkg.com/jest-regex-util/-/jest-regex-util-25.1.0.tgz#efaf75914267741838e01de24da07b2192d16d87" @@ -4875,6 +5007,14 @@ jest-watcher@^25.1.0: jest-util "^25.1.0" string-length "^3.1.0" +jest-when@^2.7.0: + version "2.7.0" + resolved "https://registry.yarnpkg.com/jest-when/-/jest-when-2.7.0.tgz#9549185ae8847b47d5d40262f1c59a5143e89a0c" + integrity sha512-psU0pXdomBORY9TGuSut/k8vViVki9l92WggL0m5/Lk8zTrDYtcCpPIFdZQDKqXvmW5Jzoh7SCsLKITvBJ0jyQ== + dependencies: + bunyan "^1.8.12" + expect "^24.8.0" + jest-worker@^25.1.0: version "25.1.0" resolved "https://registry.yarnpkg.com/jest-worker/-/jest-worker-25.1.0.tgz#75d038bad6fdf58eba0d2ec1835856c497e3907a" @@ -5576,7 +5716,7 @@ minimalistic-crypto-utils@^1.0.0, minimalistic-crypto-utils@^1.0.1: resolved "https://registry.yarnpkg.com/minimalistic-crypto-utils/-/minimalistic-crypto-utils-1.0.1.tgz#f6c00c1c0b082246e5c4d99dfb8c7c083b2b582a" integrity sha1-9sAMHAsIIkblxNmd+4x8CDsrWCo= -minimatch@^3.0.4, minimatch@~3.0.2: +"minimatch@2 || 3", minimatch@^3.0.4, minimatch@~3.0.2: version "3.0.4" resolved "https://registry.yarnpkg.com/minimatch/-/minimatch-3.0.4.tgz#5166e286457f03306064be5497e8dbb0c3d32083" integrity sha512-yJHVQEhyqPLUTgt9B83PXu6W3rx4MvvHvSUvToogpwoGDOUQ+yDrR0HRot+yOCdCO7u4hX3pWft6kWBBcqh0UA== @@ -5629,7 +5769,7 @@ mkdirp@0.5.1, "mkdirp@>=0.5 0", mkdirp@^0.5.0, mkdirp@^0.5.1, mkdirp@~0.5.1: dependencies: minimist "0.0.8" -moment@^2.19.1, moment@^2.22.2: +moment@^2.10.6, moment@^2.19.1, moment@^2.22.2: version "2.24.0" resolved "https://registry.yarnpkg.com/moment/-/moment-2.24.0.tgz#0d055d53f5052aa653c9f6eb68bb5d12bf5c2b5b" integrity sha512-bV7f+6l2QigeBBZSM/6yTNq4P2fNpSWj/0e7jQcy87A8e7o2nAfP/34/2ky5Vw4B9S446EtIhodAzkFCcR4dQg== @@ -5666,7 +5806,16 @@ mute-stream@0.0.8: resolved "https://registry.yarnpkg.com/mute-stream/-/mute-stream-0.0.8.tgz#1630c42b2251ff81e2a283de96a5497ea92e5e0d" integrity sha512-nnbWWOkoWyUsTjKrhgD0dcz22mdkSnpYqbEjIm2nhwhuxlSkpywJmBo8h0ZqJdkp73mb90SssHkN4rsRaBAfAA== -nan@^2.12.1, nan@^2.13.2: +mv@~2: + version "2.1.1" + resolved "https://registry.yarnpkg.com/mv/-/mv-2.1.1.tgz#ae6ce0d6f6d5e0a4f7d893798d03c1ea9559b6a2" + integrity sha1-rmzg1vbV4KT32JN5jQPB6pVZtqI= + dependencies: + mkdirp "~0.5.1" + ncp "~2.0.0" + rimraf "~2.4.0" + +nan@^2.12.1, nan@^2.13.2, nan@^2.14.0: version "2.14.0" resolved "https://registry.yarnpkg.com/nan/-/nan-2.14.0.tgz#7818f722027b2459a86f0295d434d1fc2336c52c" integrity sha512-INOFj37C7k3AfaNTtX8RhsTw7qRy7eLET14cROi9+5HAVbbHuIWUHEauBv5qT4Av2tWasiTY1Jw6puUNqRJXQg== @@ -5693,7 +5842,7 @@ natural-compare@^1.4.0: resolved "https://registry.yarnpkg.com/natural-compare/-/natural-compare-1.4.0.tgz#4abebfeed7541f2c27acfb29bdbbd15c8d5ba4f7" integrity sha1-Sr6/7tdUHywnrPspvbvRXI1bpPc= -ncp@2.0.0: +ncp@2.0.0, ncp@~2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/ncp/-/ncp-2.0.0.tgz#195a21d6c46e361d2fb1281ba38b91e9df7bdbb3" integrity sha1-GVoh1sRuNh0vsSgbo4uR6d9727M= @@ -6524,6 +6673,16 @@ prepend-http@^1.0.0: resolved "https://registry.yarnpkg.com/prepend-http/-/prepend-http-1.0.4.tgz#d4f4562b0ce3696e41ac52d0e002e57a635dc6dc" integrity sha1-1PRWKwzjaW5BrFLQ4ALlemNdxtw= +pretty-format@^24.9.0: + version "24.9.0" + resolved "https://registry.yarnpkg.com/pretty-format/-/pretty-format-24.9.0.tgz#12fac31b37019a4eea3c11aa9a959eb7628aa7c9" + integrity sha512-00ZMZUiHaJrNfk33guavqgvfJS30sLYf0f8+Srklv0AMPodGGHcoHgksZ3OThYnIvOd+8yMCn0YiEOogjlgsnA== + dependencies: + "@jest/types" "^24.9.0" + ansi-regex "^4.0.0" + ansi-styles "^3.2.0" + react-is "^16.8.4" + pretty-format@^25.1.0: version "25.1.0" resolved "https://registry.yarnpkg.com/pretty-format/-/pretty-format-25.1.0.tgz#ed869bdaec1356fc5ae45de045e2c8ec7b07b0c8" @@ -6756,7 +6915,7 @@ react-dom@^16.12.0: prop-types "^15.6.2" scheduler "^0.18.0" -react-is@^16.12.0, react-is@^16.6.0, react-is@^16.7.0, react-is@^16.8.1, react-is@^16.8.6, react-is@^16.9.0: +react-is@^16.12.0, react-is@^16.6.0, react-is@^16.7.0, react-is@^16.8.1, react-is@^16.8.4, react-is@^16.8.6, react-is@^16.9.0: version "16.12.0" resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.12.0.tgz#2cc0fe0fba742d97fd527c42a13bec4eeb06241c" integrity sha512-rPCkf/mWBtKc97aLL9/txD8DZdemK0vkA3JMLShjlJB3Pj3s+lpf1KaBzMfQrAmhMQB0n1cU/SUGgKKBCe837Q== @@ -7086,6 +7245,11 @@ regexp-quote@0.0.0: resolved "https://registry.yarnpkg.com/regexp-quote/-/regexp-quote-0.0.0.tgz#1e0f4650c862dcbfed54fd42b148e9bb1721fcf2" integrity sha1-Hg9GUMhi3L/tVP1CsUjpuxch/PI= +regexp-tree@~0.1.1: + version "0.1.19" + resolved "https://registry.yarnpkg.com/regexp-tree/-/regexp-tree-0.1.19.tgz#9326e91d8d1d23298dd33a78cf5e788f57cdc359" + integrity sha512-mVeVLF/qg5qFbZSW0f7pXbuVX73dKIjuhOyC2JLKxzmpya75O4qLcvI9j0jp31Iz7IAkEVHa1UErDCAqaLKo5A== + regexp.prototype.flags@^1.2.0, regexp.prototype.flags@^1.3.0: version "1.3.0" resolved "https://registry.yarnpkg.com/regexp.prototype.flags/-/regexp.prototype.flags-1.3.0.tgz#7aba89b3c13a64509dabcf3ca8d9fbb9bdf5cb75" @@ -7304,6 +7468,13 @@ rimraf@^3.0.0: dependencies: glob "^7.1.3" +rimraf@~2.4.0: + version "2.4.5" + resolved "https://registry.yarnpkg.com/rimraf/-/rimraf-2.4.5.tgz#ee710ce5d93a8fdb856fb5ea8ff0e2d75934b2da" + integrity sha1-7nEM5dk6j9uFb7Xqj/Di11k0sto= + dependencies: + glob "^6.0.1" + ripemd160@^2.0.0, ripemd160@^2.0.1: version "2.0.2" resolved "https://registry.yarnpkg.com/ripemd160/-/ripemd160-2.0.2.tgz#a1c1a6f624751577ba5d07914cbc92850585890c" @@ -7366,6 +7537,11 @@ safe-buffer@~5.1.0, safe-buffer@~5.1.1: resolved "https://registry.yarnpkg.com/safe-buffer/-/safe-buffer-5.1.2.tgz#991ec69d296e0313747d59bdfd2b745c35f8828d" integrity sha512-Gd2UZBJDkXlY7GbJxfsE8/nvKkUEU1G38c1siN6QP6a9PT9MmHB8GnpscSmMJSoF8LOIrt8ud/wPtojys4G6+g== +safe-json-stringify@~1: + version "1.2.0" + resolved "https://registry.yarnpkg.com/safe-json-stringify/-/safe-json-stringify-1.2.0.tgz#356e44bc98f1f93ce45df14bcd7c01cda86e0afd" + integrity sha512-gH8eh2nZudPQO6TytOvbxnuhYBOvDBBLW52tz5q6X58lJcd/tkmqFR+5Z9adS8aJtURSXWThWy/xJtJwixErvg== + safe-regex@^1.1.0: version "1.1.0" resolved "https://registry.yarnpkg.com/safe-regex/-/safe-regex-1.1.0.tgz#40a3669f3b077d1e943d44629e157dd48023bf2e" @@ -7373,6 +7549,13 @@ safe-regex@^1.1.0: dependencies: ret "~0.1.10" +safe-regex@^2.1.1: + version "2.1.1" + resolved "https://registry.yarnpkg.com/safe-regex/-/safe-regex-2.1.1.tgz#f7128f00d056e2fe5c11e81a1324dd974aadced2" + integrity sha512-rx+x8AMzKb5Q5lQ95Zoi6ZbJqwCLkqi3XuJXp5P3rT8OEc6sZCJG5AE5dU3lsgRr/F4Bs31jSlVN+j5KrsGu9A== + dependencies: + regexp-tree "~0.1.1" + "safer-buffer@>= 2.1.2 < 3", safer-buffer@^2.0.2, safer-buffer@^2.1.0, safer-buffer@~2.1.0: version "2.1.2" resolved "https://registry.yarnpkg.com/safer-buffer/-/safer-buffer-2.1.2.tgz#44fa161b0187b9549dd84bb91802f9bd8385cd6a" @@ -7630,6 +7813,11 @@ sisteransi@^1.0.3: resolved "https://registry.yarnpkg.com/sisteransi/-/sisteransi-1.0.4.tgz#386713f1ef688c7c0304dc4c0632898941cad2e3" integrity sha512-/ekMoM4NJ59ivGSfKapeG+FWtrmWvA1p6FBZwXrqojw90vJu8lBmrTxCMuBCydKtkaUe2zt4PlxeTKpjwMbyig== +slash@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/slash/-/slash-2.0.0.tgz#de552851a1759df3a8f206535442f5ec4ddeab44" + integrity sha512-ZYKh3Wh2z1PpEXWr0MpSBZ0V6mZHAQfYevttO11c51CaWjGTaadiKZ+wVt1PbMlDV5qhMFslpZCemhwOK7C89A== + slash@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/slash/-/slash-3.0.0.tgz#6539be870c165adbd5240220dbe361f1bc4d4634" From 4831bf5c30e415020a6e1970a9984846ffb9fa26 Mon Sep 17 00:00:00 2001 From: Ben Date: Tue, 25 Feb 2020 15:12:57 -0500 Subject: [PATCH 2/7] Add more tests. Check if a pageHost has valid url or regex characters --- .../components/Settings/TrustAndRestrict.jsx | 28 +-- .../Settings/__tests__/TrustAndRestrict.jsx | 162 ++++++++++++++---- src/classes/Policy.js | 30 ++-- test/src/policy.test.js | 97 +++++++++-- 4 files changed, 243 insertions(+), 74 deletions(-) diff --git a/app/panel/components/Settings/TrustAndRestrict.jsx b/app/panel/components/Settings/TrustAndRestrict.jsx index e91d150c1..6c9ccb974 100644 --- a/app/panel/components/Settings/TrustAndRestrict.jsx +++ b/app/panel/components/Settings/TrustAndRestrict.jsx @@ -141,24 +141,28 @@ class TrustAndRestrict extends React.Component { } isValidUrlWildcardOrRegex(pageHost) { - // Check for valid URL - // from node-validator + // Only allow valid host name, and regex characters as well as : for port numbers + const isSafePageHost = /^[a-zA-Z-1-9-:.,^$*+?()[{}|\]]*$/; + if (!isSafePageHost.test(pageHost)) { return false; } + + // Check for valid URL from node-validator const isValidUrlRegex = /^(?!mailto:)(?:(?:https?|ftp):\/\/)?(?:\S+(?::\S*)?@)?(?:(?:(?:[1-9]\d?|1\d\d|2[01]\d|22[0-3])(?:\.(?:1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.(?:[1-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(?:(?:[a-z\u00a1-\uffff0-9]+-?)*[a-z\u00a1-\uffff0-9]+)(?:\.(?:[a-z\u00a1-\uffff0-9]+-?)*[a-z\u00a1-\uffff0-9]+)*(?:\.(?:[a-z\u00a1-\uffff]{2,})))|localhost)(?::\d{2,5})?(?:\/[^\s]*)?$/i; if (isValidUrlRegex.test(pageHost)) return true; // Check for valid wildcard const escapedPattern = pageHost.replace(/[|\\{}()[\]^$+*?.-]/g, '\\$&'); - const wildcardPattern = escapedPattern.replace(/\*/g, '.*'); - let isValidWildcard = true; - try { - // eslint-disable-next-line - new RegExp(wildcardPattern); - } catch { - isValidWildcard = false; + if (pageHost.includes('*')) { + const wildcardPattern = escapedPattern.replace(/\*/g, '.*'); + let isValidWildcard = true; + try { + // eslint-disable-next-line + new RegExp(wildcardPattern); + } catch { + isValidWildcard = false; + } + if (isValidWildcard) return true; } - if (isValidWildcard) return true; - // Prevent ReDoS attack if (!safe(pageHost)) return false; @@ -166,7 +170,7 @@ class TrustAndRestrict extends React.Component { let isValidRegex = true; try { // eslint-disable-next-line - new RegExp(pageHost); + new RegExp(escapedPattern); } catch { isValidRegex = false; } diff --git a/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx b/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx index b939c294a..d2c8d1389 100644 --- a/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx +++ b/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx @@ -34,58 +34,160 @@ describe('app/panel/components/Settings/', () => { const input = 'ghostery.com'; const fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); - when(fn) - .calledWith(input) - .mockReturnValue(true); + when(fn).calledWith(input); const returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); expect(returnValue).toBe(true); }); test('isValidUrlWildcardOrRegex should return true with wildcard URL entered', () => { const wrapper = shallow(); - const input = 'developer.*.org'; - const fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); - when(fn) - .calledWith(input) - .mockReturnValue(true); - const returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + let input = 'developer.*.org'; + let fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn).calledWith(input); + let returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + expect(returnValue).toBe(true); + + input = '*.com'; + fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn).calledWith(input); + returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + expect(returnValue).toBe(true); + + input = '*'; + fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn).calledWith(input); + returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + expect(returnValue).toBe(true); + + input = 'developer.*'; + fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn).calledWith(input); + returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + expect(returnValue).toBe(true); + + input = '****'; + fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn).calledWith(input); + returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); expect(returnValue).toBe(true); }); - test('isValidUrlWildcardOrRegex should return true with regex URL entered', () => { + test('isValidUrlWildcardOrRegex should return false with wildcard URL entered', () => { const wrapper = shallow(); - const input = '[ds]eveloper.mozilla.org'; - const fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); - when(fn) - .calledWith(input) - .mockReturnValue(true); - const returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + let input = ''; + let fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn).calledWith(input); + let returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + expect(returnValue).toBe(false); + + input = '+$@@#$*'; + fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn).calledWith(input); + returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + expect(returnValue).toBe(false); + + input = 'αράδειγμα.δοκιμ.*'; + fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn).calledWith(input); + returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + expect(returnValue).toBe(false); + + input = 'SELECT * FROM USERS'; + fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn).calledWith(input); + returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + expect(returnValue).toBe(false); + }); + + test('isValidUrlWildcardOrRegex should return true with regex entered', () => { + const wrapper = shallow(); + + let input = '[de]eveloper.mozilla.org'; + let fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn).calledWith(input); + let returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + expect(returnValue).toBe(true); + + input = '\d{3}'; + fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn).calledWith(input); + returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + expect(returnValue).toBe(true); + + input = 'mi.....ft'; + fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn).calledWith(input); + returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + expect(returnValue).toBe(true); + + input = '^pet'; + fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn).calledWith(input); + returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + expect(returnValue).toBe(true); + + input = '[lu]z{2,6}'; + fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn).calledWith(input); + returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); expect(returnValue).toBe(true); }); - test('isValidUrlWildcardOrRegex should return false with unsafe regex entered', () => { + test('isValidUrlWildcardOrRegex should return false with regex entered', () => { const wrapper = shallow(); - const input = '/^(\w+\s?)*$/'; - const fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); - when(fn) - .calledWith(input) - .mockReturnValue(false); - const returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + let input = ')'; + let fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn).calledWith(input); + let returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + expect(returnValue).toBe(false); + + input = '++'; + fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn).calledWith(input); + returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + expect(returnValue).toBe(false); + + input = '/foo(?)/'; + fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + when(fn).calledWith(input); + returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); expect(returnValue).toBe(false); }); - test('isValidUrlWildcardOrRegex should return false with incorrect regex format entered', () => { + test('isValidUrlWildcardOrRegex should return false with unsafe test entered', () => { const wrapper = shallow(); - const input = '[.ghostery.com'; - const fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); - when(fn) - .calledWith(input) - .mockReturnValue(false); - const returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + let input = '/^(\w+\s?)*$/'; + let fn = jest.spyOn(wrapper.instance(), 'isSafe'); + when(fn).calledWith(input); + let returnValue = wrapper.instance().isSafe(input); + expect(returnValue).toBe(false); + + input = '/^([0-9]+)*$/'; + fn = jest.spyOn(wrapper.instance(), 'isSafe'); + when(fn).calledWith(input); + returnValue = wrapper.instance().isSafe(input); + expect(returnValue).toBe(false); + + input = '(?:.\s*)*?'; + fn = jest.spyOn(wrapper.instance(), 'isSafe'); + when(fn).calledWith(input); + returnValue = wrapper.instance().isSafe(input); + expect(returnValue).toBe(false); + + input = '(x\w{1,10})+y'; + fn = jest.spyOn(wrapper.instance(), 'isSafe'); + when(fn).calledWith(input); + returnValue = wrapper.instance().isSafe(input); + expect(returnValue).toBe(false); + + input = '^((ab)*)+$'; + fn = jest.spyOn(wrapper.instance(), 'isSafe'); + when(fn).calledWith(input); + returnValue = wrapper.instance().isSafe(input); expect(returnValue).toBe(false); }); }); diff --git a/src/classes/Policy.js b/src/classes/Policy.js index c6af879a5..3e3c3db95 100644 --- a/src/classes/Policy.js +++ b/src/classes/Policy.js @@ -183,22 +183,24 @@ class Policy { * @return {boolean} */ matchesWildcardOrRegex(url, pattern) { - // Input string might be a wildcard - const escapedPattern = pattern.replace(/[|\\{}()[\]^$+*?.-]/g, '\\$&'); - const wildcardPattern = escapedPattern.replace(/\*/g, '.*'); - const wildcardRegex = RegExp(wildcardPattern); - console.log('url: ', url); - console.log('wildcardPattern: ', wildcardPattern); - - if (wildcardRegex.test(url)) { return true; } + // Pattern might be a wildcard + if (pattern.includes('*')) { + const wildcardPattern = pattern.replace(/\*/g, '.*'); + try { + const wildcardRegex = new RegExp(wildcardPattern); + if (wildcardRegex.test(url)) { return true; } + } catch { + return false; + } + } - console.log('pattern: ', pattern); // or a regex - const regex = RegExp(pattern); - if (regex.test(url)) { return true; } - - console.log('returning false'); - + try { + const regex = new RegExp(pattern); + if (regex.test(url)) { return true; } + } catch { + return false; + } return false; } } diff --git a/test/src/policy.test.js b/test/src/policy.test.js index 02e7d7169..e9571160a 100644 --- a/test/src/policy.test.js +++ b/test/src/policy.test.js @@ -1,34 +1,95 @@ import sinon from 'sinon'; -import PolicySmartBlock from '../../src/classes/PolicySmartBlock'; -let policySmartBlock = new PolicySmartBlock(); -let policy = policySmartBlock.policy; +import Policy from '../../src/classes/Policy'; +let policy = new Policy(); // Mock imports for dependencies jest.mock('../../src/classes/TabInfo', () => {}); describe('src/classes/Policy.js', () => { - describe('test functions', () => { - + describe('test matchesWildcardOrRegex', () => { test('matchesWildcardOrRegex should return true with wildcard entered ', () => { - const input = 'mozilla.*.com'; - const stub = sinon.stub(policy, 'matchesWildcardOrRegex'); - expect(stub.withArgs(input).returns(true)); - stub.restore(); + let url = 'developer.mozilla.org'; + let input = 'developer.*.org'; + expect(policy.matchesWildcardOrRegex(url, input)).toBeTruthy(); + + url = 'ghostery.com'; + input = '*.com'; + expect(policy.matchesWildcardOrRegex(url, input)).toBeTruthy(); + + url = 'ghostery.com' + input = '*'; + expect(policy.matchesWildcardOrRegex(url, input)).toBeTruthy(); + + url = 'developer.mozilla.org'; + input = 'developer.*'; + expect(policy.matchesWildcardOrRegex(url , input)).toBeTruthy(); + + url = 'developer.mozilla.org'; + input = '****'; + expect(policy.matchesWildcardOrRegex(url, input)).toBeTruthy(); + }); + + test('matchesWildcardOrRegex should return false with wildcard entered ', () => { + let url = 'developer.mozilla.org'; + let input = ''; + expect(policy.matchesWildcardOrRegex(url, input)).toBeFalsy(); + + url = 'ghostery.com'; + input = '+$@@#$*'; + expect(policy.matchesWildcardOrRegex(url, input)).toBeFalsy(); + + url = 'ghostery.com' + input = 'αράδειγμα.δοκιμ.*'; + expect(policy.matchesWildcardOrRegex(url, input)).toBeFalsy(); + + url = 'SELECT * FROM USERS'; + input = 'developer.*'; + expect(policy.matchesWildcardOrRegex(url , input)).toBeFalsy(); }); test('matchesWildcardOrRegex should return true with regex entered', () => { - const input = '[de]eveloper.mozilla.org'; - const stub = sinon.stub(policy, 'matchesWildcardOrRegex'); - expect(stub.withArgs(input).returns(true)); - stub.restore(); + let url = 'developer.mozilla.org'; + let input = '[de]eveloper.mozilla.org'; + expect(policy.matchesWildcardOrRegex(url, input)).toBeTruthy(); + + url = 'regex101.com'; + input = '\\d{3}'; + expect(policy.matchesWildcardOrRegex(url, input)).toBeTruthy(); + + url = 'microsoft.com'; + input = 'mi.....ft'; + expect(policy.matchesWildcardOrRegex(url, input)).toBeTruthy(); + + url = 'petfinder.com'; + input = '^pet'; + expect(policy.matchesWildcardOrRegex(url, input)).toBeTruthy(); + + url = 'buzzfeed.com'; + input = '[lu]z{2,6}'; + expect(policy.matchesWildcardOrRegex(url, input)).toBeTruthy(); }); - test('matchesWildcardOrRegex should return false with ', () => { - const input = '[google.com'; - const stub = sinon.stub(policy, 'matchesWildcardOrRegex'); - expect(stub.withArgs(input).returns(false)); - stub.restore(); + test('matchesWildcardOrRegex should return false with regex entered', () => { + let url = 'foo.com'; + let input = '/foo)]/'; + expect(policy.matchesWildcardOrRegex(url, input)).toBeFalsy(); + + url = 'foo.com'; + input = 'test\\'; + expect(policy.matchesWildcardOrRegex(url, input)).toBeFalsy(); + + url = 'foo.com'; + input = '/(?<=x*)foo/'; + expect(policy.matchesWildcardOrRegex(url, input)).toBeFalsy(); + + url = 'foo.com'; + input = '/foo(?)/'; + expect(policy.matchesWildcardOrRegex(url, input)).toBeFalsy(); + + url = 'foo.com'; + input = ''; + expect(policy.matchesWildcardOrRegex(url, input)).toBeFalsy(); }); }) }); From 359ed37958174854d5d9f4d04b205355d392799a Mon Sep 17 00:00:00 2001 From: Ben Date: Tue, 25 Feb 2020 15:16:48 -0500 Subject: [PATCH 3/7] Update tests for checking if a regex is safe --- .../Settings/__tests__/TrustAndRestrict.jsx | 24 +++++-------------- 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx b/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx index d2c8d1389..2cc1b269a 100644 --- a/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx +++ b/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx @@ -161,33 +161,21 @@ describe('app/panel/components/Settings/', () => { const wrapper = shallow(); let input = '/^(\w+\s?)*$/'; - let fn = jest.spyOn(wrapper.instance(), 'isSafe'); + let fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); when(fn).calledWith(input); - let returnValue = wrapper.instance().isSafe(input); + let returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); expect(returnValue).toBe(false); input = '/^([0-9]+)*$/'; - fn = jest.spyOn(wrapper.instance(), 'isSafe'); - when(fn).calledWith(input); - returnValue = wrapper.instance().isSafe(input); - expect(returnValue).toBe(false); - - input = '(?:.\s*)*?'; - fn = jest.spyOn(wrapper.instance(), 'isSafe'); + fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); when(fn).calledWith(input); - returnValue = wrapper.instance().isSafe(input); + returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); expect(returnValue).toBe(false); input = '(x\w{1,10})+y'; - fn = jest.spyOn(wrapper.instance(), 'isSafe'); - when(fn).calledWith(input); - returnValue = wrapper.instance().isSafe(input); - expect(returnValue).toBe(false); - - input = '^((ab)*)+$'; - fn = jest.spyOn(wrapper.instance(), 'isSafe'); + fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); when(fn).calledWith(input); - returnValue = wrapper.instance().isSafe(input); + returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); expect(returnValue).toBe(false); }); }); From 5b14de20a522a93396aabcc0650d3f4cd380521c Mon Sep 17 00:00:00 2001 From: Ben Date: Thu, 27 Feb 2020 16:19:04 -0500 Subject: [PATCH 4/7] Remove regex functionality and update unit tests --- _locales/en/messages.json | 4 +- .../components/Settings/TrustAndRestrict.jsx | 38 +++--- .../Settings/__tests__/TrustAndRestrict.jsx | 114 +++++++----------- package.json | 1 - src/classes/Policy.js | 27 ++--- test/src/policy.test.js | 60 +++------ yarn.lock | 12 -- 7 files changed, 88 insertions(+), 168 deletions(-) diff --git a/_locales/en/messages.json b/_locales/en/messages.json index 6c3bada36..4a850e047 100644 --- a/_locales/en/messages.json +++ b/_locales/en/messages.json @@ -802,7 +802,7 @@ "message": "unblocked" }, "white_black_list_error_invalid_url": { - "message": "Please enter a valid URL, regex, or wildcard." + "message": "Please enter a valid URL or wildcard." }, "whitelist_error_blacklist_url": { "message": "This site has been removed from your Restricted Sites list and added to your Trusted Sites list." @@ -1058,7 +1058,7 @@ "message": "Trusted Sites" }, "settings_sites_placeholder": { - "message": "example.com (wildcards/regex supported)" + "message": "example.com (wildcards supported)" }, "settings_restricted_sites": { "message": "Restricted Sites" diff --git a/app/panel/components/Settings/TrustAndRestrict.jsx b/app/panel/components/Settings/TrustAndRestrict.jsx index 6c9ccb974..04e131585 100644 --- a/app/panel/components/Settings/TrustAndRestrict.jsx +++ b/app/panel/components/Settings/TrustAndRestrict.jsx @@ -12,7 +12,6 @@ */ import React from 'react'; -import safe from 'safe-regex'; import Sites from './Sites'; /** * @class Implement Trust and Restrict subview presenting the lists @@ -121,7 +120,7 @@ class TrustAndRestrict extends React.Component { // Check for Validity if (pageHost.length >= 2083 - || !this.isValidUrlWildcardOrRegex(pageHost)) { + || !this.isValidUrlorWildcard(pageHost)) { this.showWarning(t('white_black_list_error_invalid_url')); return; } @@ -138,11 +137,16 @@ class TrustAndRestrict extends React.Component { this.props.actions.updateSitePolicy({ type: otherListType, pageHost }); } this.props.actions.updateSitePolicy({ type: listType, pageHost }); + if (listType === 'whitelist') { + this.setState({ trustedValue: '' }); + } else { + this.setState({ restrictedValue: '' }); + } } - isValidUrlWildcardOrRegex(pageHost) { - // Only allow valid host name, and regex characters as well as : for port numbers - const isSafePageHost = /^[a-zA-Z-1-9-:.,^$*+?()[{}|\]]*$/; + isValidUrlorWildcard(pageHost) { + // Only allow valid host name characters, ':' for port numbers and '*' for wildcards + const isSafePageHost = /^[a-zA-Z1-9-.:*]*$/; if (!isSafePageHost.test(pageHost)) { return false; } // Check for valid URL from node-validator @@ -150,32 +154,18 @@ class TrustAndRestrict extends React.Component { if (isValidUrlRegex.test(pageHost)) return true; // Check for valid wildcard - const escapedPattern = pageHost.replace(/[|\\{}()[\]^$+*?.-]/g, '\\$&'); + let isValidWildcard = false; if (pageHost.includes('*')) { - const wildcardPattern = escapedPattern.replace(/\*/g, '.*'); - let isValidWildcard = true; + const wildcardPattern = pageHost.replace(/\*/g, '.*'); try { // eslint-disable-next-line new RegExp(wildcardPattern); + isValidWildcard = true; } catch { - isValidWildcard = false; + return false; } - if (isValidWildcard) return true; - } - - // Prevent ReDoS attack - if (!safe(pageHost)) return false; - - // Check for valid regex - let isValidRegex = true; - try { - // eslint-disable-next-line - new RegExp(escapedPattern); - } catch { - isValidRegex = false; } - - return isValidRegex; + return isValidWildcard; } /** diff --git a/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx b/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx index 2cc1b269a..3026d452f 100644 --- a/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx +++ b/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx @@ -29,153 +29,125 @@ describe('app/panel/components/Settings/TrustAndRestrict', () => { }); describe('app/panel/components/Settings/', () => { - test('isValidUrlWildcardOrRegex should return true with url entered', () => { + test('isValidUrlorWildcard should return true with url entered', () => { const wrapper = shallow(); const input = 'ghostery.com'; - const fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + const fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); when(fn).calledWith(input); - const returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + const returnValue = wrapper.instance().isValidUrlorWildcard(input); expect(returnValue).toBe(true); }); - test('isValidUrlWildcardOrRegex should return true with wildcard URL entered', () => { + test('isValidUrlorWildcard should return true with wildcard URL entered', () => { const wrapper = shallow(); let input = 'developer.*.org'; - let fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + let fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); when(fn).calledWith(input); - let returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + let returnValue = wrapper.instance().isValidUrlorWildcard(input); expect(returnValue).toBe(true); input = '*.com'; - fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); when(fn).calledWith(input); - returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + returnValue = wrapper.instance().isValidUrlorWildcard(input); expect(returnValue).toBe(true); input = '*'; - fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); when(fn).calledWith(input); - returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + returnValue = wrapper.instance().isValidUrlorWildcard(input); expect(returnValue).toBe(true); input = 'developer.*'; - fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); when(fn).calledWith(input); - returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + returnValue = wrapper.instance().isValidUrlorWildcard(input); expect(returnValue).toBe(true); input = '****'; - fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); when(fn).calledWith(input); - returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + returnValue = wrapper.instance().isValidUrlorWildcard(input); expect(returnValue).toBe(true); }); - test('isValidUrlWildcardOrRegex should return false with wildcard URL entered', () => { + test('isValidUrlorWildcard should return false with wildcard URL entered', () => { const wrapper = shallow(); let input = ''; - let fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + let fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); when(fn).calledWith(input); - let returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + let returnValue = wrapper.instance().isValidUrlorWildcard(input); expect(returnValue).toBe(false); input = '+$@@#$*'; - fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); when(fn).calledWith(input); - returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + returnValue = wrapper.instance().isValidUrlorWildcard(input); expect(returnValue).toBe(false); input = 'αράδειγμα.δοκιμ.*'; - fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); when(fn).calledWith(input); - returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + returnValue = wrapper.instance().isValidUrlorWildcard(input); expect(returnValue).toBe(false); input = 'SELECT * FROM USERS'; - fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); when(fn).calledWith(input); - returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + returnValue = wrapper.instance().isValidUrlorWildcard(input); expect(returnValue).toBe(false); }); - test('isValidUrlWildcardOrRegex should return true with regex entered', () => { - const wrapper = shallow(); - - let input = '[de]eveloper.mozilla.org'; - let fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); - when(fn).calledWith(input); - let returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); - expect(returnValue).toBe(true); - - input = '\d{3}'; - fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); - when(fn).calledWith(input); - returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); - expect(returnValue).toBe(true); - - input = 'mi.....ft'; - fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); - when(fn).calledWith(input); - returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); - expect(returnValue).toBe(true); - - input = '^pet'; - fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); - when(fn).calledWith(input); - returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); - expect(returnValue).toBe(true); - - input = '[lu]z{2,6}'; - fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); - when(fn).calledWith(input); - returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); - expect(returnValue).toBe(true); - }); - - test('isValidUrlWildcardOrRegex should return false with regex entered', () => { + test('isValidUrlorWildcard should return false with regex entered', () => { const wrapper = shallow(); let input = ')'; - let fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + let fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); when(fn).calledWith(input); - let returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + let returnValue = wrapper.instance().isValidUrlorWildcard(input); expect(returnValue).toBe(false); input = '++'; - fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); when(fn).calledWith(input); - returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + returnValue = wrapper.instance().isValidUrlorWildcard(input); expect(returnValue).toBe(false); input = '/foo(?)/'; - fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); when(fn).calledWith(input); - returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + returnValue = wrapper.instance().isValidUrlorWildcard(input); expect(returnValue).toBe(false); }); - test('isValidUrlWildcardOrRegex should return false with unsafe test entered', () => { + test('isValidUrlorWildcard should return false with unsafe test entered', () => { const wrapper = shallow(); let input = '/^(\w+\s?)*$/'; - let fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + let fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); when(fn).calledWith(input); - let returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + let returnValue = wrapper.instance().isValidUrlorWildcard(input); expect(returnValue).toBe(false); input = '/^([0-9]+)*$/'; - fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); when(fn).calledWith(input); - returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + returnValue = wrapper.instance().isValidUrlorWildcard(input); expect(returnValue).toBe(false); input = '(x\w{1,10})+y'; - fn = jest.spyOn(wrapper.instance(), 'isValidUrlWildcardOrRegex'); + fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); + when(fn).calledWith(input); + returnValue = wrapper.instance().isValidUrlorWildcard(input); + expect(returnValue).toBe(false); + + input = '.*.*.*.*.*.*.*.*.*.*.*'; + fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); when(fn).calledWith(input); - returnValue = wrapper.instance().isValidUrlWildcardOrRegex(input); + returnValue = wrapper.instance().isValidUrlorWildcard(input); expect(returnValue).toBe(false); }); }); diff --git a/package.json b/package.json index cf906f778..ff7fa0e60 100644 --- a/package.json +++ b/package.json @@ -64,7 +64,6 @@ "redux-object": "^0.5.10", "redux-thunk": "^2.2.0", "rsvp": "^4.8.5", - "safe-regex": "^2.1.1", "spanan": "^2.0.0", "ua-parser-js": "^0.7.21", "underscore": "^1.9.2", diff --git a/src/classes/Policy.js b/src/classes/Policy.js index 3e3c3db95..93f6fb99f 100644 --- a/src/classes/Policy.js +++ b/src/classes/Policy.js @@ -70,8 +70,10 @@ class Policy { // TODO: speed up for (let i = 0; i < num_sites; i++) { // TODO match from the beginning of the string to avoid false matches (somewhere in the querystring for instance) - if (replacedUrl === sites[i] - || this.matchesWildcardOrRegex(replacedUrl, sites[i])) { + if (!sites[i].includes('*') && replacedUrl === sites[i]) { + return sites[i]; + } + if (this.matchesWildcard(replacedUrl, sites[i])) { return sites[i]; } } @@ -119,8 +121,10 @@ class Policy { // TODO: speed up for (let i = 0; i < num_sites; i++) { // TODO match from the beginning of the string to avoid false matches (somewhere in the querystring for instance) - if (replacedUrl === sites[i] - || this.matchesWildcardOrRegex(replacedUrl, sites[i])) { + if (!sites[i].includes('*') && replacedUrl === sites[i]) { + return sites[i]; + } + if (this.matchesWildcard(replacedUrl, sites[i])) { return sites[i]; } } @@ -177,14 +181,13 @@ class Policy { } /** - * Check given url against pattern which might be a wildcard, or a regex + * Check given url against pattern which might be a wildcard * @param {string} url site url * @param {string} pattern regex pattern * @return {boolean} */ - matchesWildcardOrRegex(url, pattern) { - // Pattern might be a wildcard - if (pattern.includes('*')) { + matchesWildcard(url, pattern) { + if (typeof pattern !== 'undefined' && pattern.includes('*')) { const wildcardPattern = pattern.replace(/\*/g, '.*'); try { const wildcardRegex = new RegExp(wildcardPattern); @@ -193,14 +196,6 @@ class Policy { return false; } } - - // or a regex - try { - const regex = new RegExp(pattern); - if (regex.test(url)) { return true; } - } catch { - return false; - } return false; } } diff --git a/test/src/policy.test.js b/test/src/policy.test.js index e9571160a..d9ff77486 100644 --- a/test/src/policy.test.js +++ b/test/src/policy.test.js @@ -1,5 +1,3 @@ -import sinon from 'sinon'; - import Policy from '../../src/classes/Policy'; let policy = new Policy(); @@ -7,89 +5,67 @@ let policy = new Policy(); jest.mock('../../src/classes/TabInfo', () => {}); describe('src/classes/Policy.js', () => { - describe('test matchesWildcardOrRegex', () => { - test('matchesWildcardOrRegex should return true with wildcard entered ', () => { + describe('test matchesWildcard', () => { + test('matchesWildcard should return true with wildcard entered ', () => { let url = 'developer.mozilla.org'; let input = 'developer.*.org'; - expect(policy.matchesWildcardOrRegex(url, input)).toBeTruthy(); + expect(policy.matchesWildcard(url, input)).toBeTruthy(); url = 'ghostery.com'; input = '*.com'; - expect(policy.matchesWildcardOrRegex(url, input)).toBeTruthy(); + expect(policy.matchesWildcard(url, input)).toBeTruthy(); url = 'ghostery.com' input = '*'; - expect(policy.matchesWildcardOrRegex(url, input)).toBeTruthy(); + expect(policy.matchesWildcard(url, input)).toBeTruthy(); url = 'developer.mozilla.org'; input = 'developer.*'; - expect(policy.matchesWildcardOrRegex(url , input)).toBeTruthy(); + expect(policy.matchesWildcard(url , input)).toBeTruthy(); url = 'developer.mozilla.org'; input = '****'; - expect(policy.matchesWildcardOrRegex(url, input)).toBeTruthy(); + expect(policy.matchesWildcard(url, input)).toBeTruthy(); }); - test('matchesWildcardOrRegex should return false with wildcard entered ', () => { + test('matchesWildcard should return false with wildcard entered ', () => { let url = 'developer.mozilla.org'; let input = ''; - expect(policy.matchesWildcardOrRegex(url, input)).toBeFalsy(); + expect(policy.matchesWildcard(url, input)).toBeFalsy(); url = 'ghostery.com'; input = '+$@@#$*'; - expect(policy.matchesWildcardOrRegex(url, input)).toBeFalsy(); + expect(policy.matchesWildcard(url, input)).toBeFalsy(); url = 'ghostery.com' input = 'αράδειγμα.δοκιμ.*'; - expect(policy.matchesWildcardOrRegex(url, input)).toBeFalsy(); + expect(policy.matchesWildcard(url, input)).toBeFalsy(); url = 'SELECT * FROM USERS'; input = 'developer.*'; - expect(policy.matchesWildcardOrRegex(url , input)).toBeFalsy(); - }); - - test('matchesWildcardOrRegex should return true with regex entered', () => { - let url = 'developer.mozilla.org'; - let input = '[de]eveloper.mozilla.org'; - expect(policy.matchesWildcardOrRegex(url, input)).toBeTruthy(); - - url = 'regex101.com'; - input = '\\d{3}'; - expect(policy.matchesWildcardOrRegex(url, input)).toBeTruthy(); - - url = 'microsoft.com'; - input = 'mi.....ft'; - expect(policy.matchesWildcardOrRegex(url, input)).toBeTruthy(); - - url = 'petfinder.com'; - input = '^pet'; - expect(policy.matchesWildcardOrRegex(url, input)).toBeTruthy(); - - url = 'buzzfeed.com'; - input = '[lu]z{2,6}'; - expect(policy.matchesWildcardOrRegex(url, input)).toBeTruthy(); + expect(policy.matchesWildcard(url , input)).toBeFalsy(); }); - test('matchesWildcardOrRegex should return false with regex entered', () => { + test('matchesWildcard should return false with regex entered', () => { let url = 'foo.com'; let input = '/foo)]/'; - expect(policy.matchesWildcardOrRegex(url, input)).toBeFalsy(); + expect(policy.matchesWildcard(url, input)).toBeFalsy(); url = 'foo.com'; input = 'test\\'; - expect(policy.matchesWildcardOrRegex(url, input)).toBeFalsy(); + expect(policy.matchesWildcard(url, input)).toBeFalsy(); url = 'foo.com'; input = '/(?<=x*)foo/'; - expect(policy.matchesWildcardOrRegex(url, input)).toBeFalsy(); + expect(policy.matchesWildcard(url, input)).toBeFalsy(); url = 'foo.com'; input = '/foo(?)/'; - expect(policy.matchesWildcardOrRegex(url, input)).toBeFalsy(); + expect(policy.matchesWildcard(url, input)).toBeFalsy(); url = 'foo.com'; input = ''; - expect(policy.matchesWildcardOrRegex(url, input)).toBeFalsy(); + expect(policy.matchesWildcard(url, input)).toBeFalsy(); }); }) }); diff --git a/yarn.lock b/yarn.lock index 965095aa4..f03b3d3c7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7301,11 +7301,6 @@ regexp-quote@0.0.0: resolved "https://registry.yarnpkg.com/regexp-quote/-/regexp-quote-0.0.0.tgz#1e0f4650c862dcbfed54fd42b148e9bb1721fcf2" integrity sha1-Hg9GUMhi3L/tVP1CsUjpuxch/PI= -regexp-tree@~0.1.1: - version "0.1.19" - resolved "https://registry.yarnpkg.com/regexp-tree/-/regexp-tree-0.1.19.tgz#9326e91d8d1d23298dd33a78cf5e788f57cdc359" - integrity sha512-mVeVLF/qg5qFbZSW0f7pXbuVX73dKIjuhOyC2JLKxzmpya75O4qLcvI9j0jp31Iz7IAkEVHa1UErDCAqaLKo5A== - regexp.prototype.flags@^1.2.0, regexp.prototype.flags@^1.3.0: version "1.3.0" resolved "https://registry.yarnpkg.com/regexp.prototype.flags/-/regexp.prototype.flags-1.3.0.tgz#7aba89b3c13a64509dabcf3ca8d9fbb9bdf5cb75" @@ -7605,13 +7600,6 @@ safe-regex@^1.1.0: dependencies: ret "~0.1.10" -safe-regex@^2.1.1: - version "2.1.1" - resolved "https://registry.yarnpkg.com/safe-regex/-/safe-regex-2.1.1.tgz#f7128f00d056e2fe5c11e81a1324dd974aadced2" - integrity sha512-rx+x8AMzKb5Q5lQ95Zoi6ZbJqwCLkqi3XuJXp5P3rT8OEc6sZCJG5AE5dU3lsgRr/F4Bs31jSlVN+j5KrsGu9A== - dependencies: - regexp-tree "~0.1.1" - "safer-buffer@>= 2.1.2 < 3", safer-buffer@^2.0.2, safer-buffer@^2.1.0, safer-buffer@~2.1.0: version "2.1.2" resolved "https://registry.yarnpkg.com/safer-buffer/-/safer-buffer-2.1.2.tgz#44fa161b0187b9549dd84bb91802f9bd8385cd6a" From 7b5445f161871015956b09d2d8b4d404c95d12a4 Mon Sep 17 00:00:00 2001 From: Ben Date: Thu, 27 Feb 2020 16:27:44 -0500 Subject: [PATCH 5/7] Check for truthy instead of not undefined --- src/classes/Policy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/classes/Policy.js b/src/classes/Policy.js index 93f6fb99f..57a56fe97 100644 --- a/src/classes/Policy.js +++ b/src/classes/Policy.js @@ -187,7 +187,7 @@ class Policy { * @return {boolean} */ matchesWildcard(url, pattern) { - if (typeof pattern !== 'undefined' && pattern.includes('*')) { + if (pattern && pattern.includes('*')) { const wildcardPattern = pattern.replace(/\*/g, '.*'); try { const wildcardRegex = new RegExp(wildcardPattern); From 3b8139038c9621d0beba85515352e32855d5c108 Mon Sep 17 00:00:00 2001 From: Ben Date: Thu, 27 Feb 2020 16:33:30 -0500 Subject: [PATCH 6/7] Remove bad unit test --- .../components/Settings/__tests__/TrustAndRestrict.jsx | 6 ------ 1 file changed, 6 deletions(-) diff --git a/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx b/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx index 3026d452f..343bd9d49 100644 --- a/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx +++ b/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx @@ -143,11 +143,5 @@ describe('app/panel/components/Settings/', () => { when(fn).calledWith(input); returnValue = wrapper.instance().isValidUrlorWildcard(input); expect(returnValue).toBe(false); - - input = '.*.*.*.*.*.*.*.*.*.*.*'; - fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); - when(fn).calledWith(input); - returnValue = wrapper.instance().isValidUrlorWildcard(input); - expect(returnValue).toBe(false); }); }); From 5e3998b785f0e74e2fc5908e369610db7506d56f Mon Sep 17 00:00:00 2001 From: Ben Date: Tue, 3 Mar 2020 09:35:38 -0500 Subject: [PATCH 7/7] Fix localhost:3000 --- app/panel/components/Settings/TrustAndRestrict.jsx | 2 +- .../Settings/__tests__/TrustAndRestrict.jsx | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/app/panel/components/Settings/TrustAndRestrict.jsx b/app/panel/components/Settings/TrustAndRestrict.jsx index 04e131585..019dcec7d 100644 --- a/app/panel/components/Settings/TrustAndRestrict.jsx +++ b/app/panel/components/Settings/TrustAndRestrict.jsx @@ -146,7 +146,7 @@ class TrustAndRestrict extends React.Component { isValidUrlorWildcard(pageHost) { // Only allow valid host name characters, ':' for port numbers and '*' for wildcards - const isSafePageHost = /^[a-zA-Z1-9-.:*]*$/; + const isSafePageHost = /^[a-zA-Z0-9-.:*]*$/; if (!isSafePageHost.test(pageHost)) { return false; } // Check for valid URL from node-validator diff --git a/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx b/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx index 343bd9d49..bac125429 100644 --- a/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx +++ b/app/panel/components/Settings/__tests__/TrustAndRestrict.jsx @@ -31,11 +31,17 @@ describe('app/panel/components/Settings/TrustAndRestrict', () => { describe('app/panel/components/Settings/', () => { test('isValidUrlorWildcard should return true with url entered', () => { const wrapper = shallow(); - const input = 'ghostery.com'; + let input = 'ghostery.com'; - const fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); + let fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); when(fn).calledWith(input); - const returnValue = wrapper.instance().isValidUrlorWildcard(input); + let returnValue = wrapper.instance().isValidUrlorWildcard(input); + expect(returnValue).toBe(true); + + input = 'localhost:3000'; + fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard'); + when(fn).calledWith(input); + returnValue = wrapper.instance().isValidUrlorWildcard(input); expect(returnValue).toBe(true); });