Skip to content

/ ghostery-extension

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

Jump to bottom

GH-1477 Wildcard/Regex Whitelisting #501

Merged
merged 8 commits into from Mar 3, 2020
+588 −15
Merged

GH-1477 Wildcard/Regex Whitelisting #501

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
17381a6
Add regex/wildcard listing with unit tests. Add safe-regex and jest-w…
benstrumeyer Feb 21, 2020
4831bf5
Add more tests. Check if a pageHost has valid url or regex characters
benstrumeyer Feb 25, 2020
359ed37
Update tests for checking if a regex is safe
benstrumeyer Feb 25, 2020
12031ac
Merge branch 'develop' into feature/regex-whitelisting
benstrumeyer Feb 25, 2020
5b14de2
Remove regex functionality and update unit tests
benstrumeyer Feb 27, 2020
7b5445f
Check for truthy instead of not undefined
benstrumeyer Feb 27, 2020
3b81390
Remove bad unit test
benstrumeyer Feb 27, 2020
5e3998b
Fix localhost:3000
benstrumeyer Mar 3, 2020
File filter
Filter file types
Jump to
Jump to file
Failed to load files.

Always

Just for now

4 _locales/en/messages.json
View file
Open in desktop
@@ -802,7 +802,7 @@
"message": "unblocked"
},
"white_black_list_error_invalid_url": {
"message": "Please enter a valid URL."
"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"
"message": "example.com (wildcards supported)"
},
"settings_restricted_sites": {
"message": "Restricted Sites"
35 app/panel/components/Settings/TrustAndRestrict.jsx
View file
Open in desktop
@@ -89,8 +89,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 +119,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.isValidUrlorWildcard(pageHost)) {
this.showWarning(t('white_black_list_error_invalid_url'));
return;
}

// Check for Duplicates
if (list.includes(pageHost)) {
this.showWarning(duplicateWarning);
@@ -137,6 +137,35 @@ 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: '' });
}
}
This conversation was marked as resolved by christophertino

This comment has been minimized.

Sign in to view
@christophertino

christophertino Feb 26, 2020
Member

Once the site is successfully added to the list, clear the input field

This comment has been minimized.

Sign in to view
@benstrumeyer

benstrumeyer Feb 27, 2020
Author Contributor

Input field cleared!

isValidUrlorWildcard(pageHost) {
This conversation was marked as resolved by christophertino

This comment has been minimized.

Sign in to view
@christophertino

christophertino Mar 2, 2020
Member

This fails when trying to add localhost:3000 to the list.

This comment has been minimized.

Sign in to view
@benstrumeyer

benstrumeyer Mar 3, 2020
Author Contributor

Good catch! I fixed the validation regex and added a unit test
// Only allow valid host name characters, ':' for port numbers and '*' for wildcards
const isSafePageHost = /^[a-zA-Z0-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
let isValidWildcard = false;
if (pageHost.includes('*')) {
const wildcardPattern = pageHost.replace(/\*/g, '.*');
try {
// eslint-disable-next-line
new RegExp(wildcardPattern);
isValidWildcard = true;
} catch {
return false;
}
}
return isValidWildcard;
}

/**
153 app/panel/components/Settings/__tests__/TrustAndRestrict.jsx
View file
Open in desktop
@@ -0,0 +1,153 @@
/**
* 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(
<TrustAndRestrict />
).toJSON();
expect(wrapper).toMatchSnapshot();
});
});
});

describe('app/panel/components/Settings/', () => {
test('isValidUrlorWildcard should return true with url entered', () => {
const wrapper = shallow(<TrustAndRestrict />);
let input = 'ghostery.com';

let fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard');
when(fn).calledWith(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);
});

test('isValidUrlorWildcard should return true with wildcard URL entered', () => {
const wrapper = shallow(<TrustAndRestrict />);

let input = 'developer.*.org';
let fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard');
when(fn).calledWith(input);
let returnValue = wrapper.instance().isValidUrlorWildcard(input);
expect(returnValue).toBe(true);

input = '*.com';
fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard');
when(fn).calledWith(input);
returnValue = wrapper.instance().isValidUrlorWildcard(input);
expect(returnValue).toBe(true);

input = '*';
fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard');
when(fn).calledWith(input);
returnValue = wrapper.instance().isValidUrlorWildcard(input);
expect(returnValue).toBe(true);

input = 'developer.*';
fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard');
when(fn).calledWith(input);
returnValue = wrapper.instance().isValidUrlorWildcard(input);
expect(returnValue).toBe(true);

input = '****';
fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard');
when(fn).calledWith(input);
returnValue = wrapper.instance().isValidUrlorWildcard(input);
expect(returnValue).toBe(true);
});

test('isValidUrlorWildcard should return false with wildcard URL entered', () => {
const wrapper = shallow(<TrustAndRestrict />);

let input = '<script>*</script>';
let fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard');
when(fn).calledWith(input);
let 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);

input = 'αράδειγμα.δοκιμ.*';
fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard');
when(fn).calledWith(input);
returnValue = wrapper.instance().isValidUrlorWildcard(input);
expect(returnValue).toBe(false);

input = 'SELECT * FROM USERS';
fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard');
when(fn).calledWith(input);
returnValue = wrapper.instance().isValidUrlorWildcard(input);
expect(returnValue).toBe(false);
});

test('isValidUrlorWildcard should return false with regex entered', () => {
const wrapper = shallow(<TrustAndRestrict />);

let input = ')';
let fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard');
when(fn).calledWith(input);
let 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);

input = '/foo(?)/';
fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard');
when(fn).calledWith(input);
returnValue = wrapper.instance().isValidUrlorWildcard(input);
expect(returnValue).toBe(false);
});

test('isValidUrlorWildcard should return false with unsafe test entered', () => {
const wrapper = shallow(<TrustAndRestrict />);

let input = '/^(\w+\s?)*$/';
let fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard');
when(fn).calledWith(input);
let returnValue = wrapper.instance().isValidUrlorWildcard(input);
expect(returnValue).toBe(false);

input = '/^([0-9]+)*$/';
fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard');
when(fn).calledWith(input);
returnValue = wrapper.instance().isValidUrlorWildcard(input);
expect(returnValue).toBe(false);

input = '(x\w{1,10})+y';
fn = jest.spyOn(wrapper.instance(), 'isValidUrlorWildcard');
when(fn).calledWith(input);
returnValue = wrapper.instance().isValidUrlorWildcard(input);
expect(returnValue).toBe(false);
});
});
119 app/panel/components/Settings/__tests__/__snapshots__/TrustAndRestrict.jsx.snap
View file
Open in desktop
@@ -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`] = `
<div
className="s-trust-restrict-panel s-tabs-panel"
>
<div
className="row"
>
<div
className="columns"
>
<h3>
settings_trusted_restricted_sites
</h3>
</div>
</div>
<div
className="s-trust-restrict-menu"
>
<div
className="s-active-pane s-pane-title"
id="showTrustedSites"
onClick={[Function]}
>
<span>
settings_trusted_sites
</span>
</div>
<div
className="s-pane-title-next"
id="showRestrictedSites"
onClick={[Function]}
>
<span>
settings_restricted_sites
</span>
</div>
</div>
<div
className="s-sites-pane"
>
<div
className="row"
>
<div
className="columns"
>
<div
className="s-sites-input-box"
>
<input
onChange={[Function]}
onKeyDown={[Function]}
placeholder="settings_sites_placeholder"
type="text"
value=""
/>
<div
className="s-sites-input-icon"
onClick={[Function]}
/>
</div>
<div
className="s-site-description"
>
<span>
settings_trusted_sites_description
</span>
</div>
<div
className="s-invisible s-callout"
>

</div>
</div>
</div>
</div>
<div
className="s-hide s-sites-pane"
>
<div
className="row"
>
<div
className="columns"
>
<div
className="s-sites-input-box"
>
<input
onChange={[Function]}
onKeyDown={[Function]}
placeholder="settings_sites_placeholder"
type="text"
value=""
/>
<div
className="s-sites-input-icon"
onClick={[Function]}
/>
</div>
<div
className="s-site-description"
>
<span>
settings_restricted_sites_description
</span>
</div>
<div
className="s-invisible s-callout"
>

</div>
</div>
</div>
</div>
</div>
`;
1 package.json
View file
Open in desktop
@@ -91,6 +91,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",
ProTip! Use n and p to navigate between commits in a pull request.