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 1 commit
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
Clear filters
Jump to
Jump to file
Failed to load files.

Always

Just for now

Prev Next
Loading status checks…

Remove regex functionality and update unit tests

  • Loading branch information
@benstrumeyer
benstrumeyer committed Feb 27, 2020
commit 5b14de20a522a93396aabcc0650d3f4cd380521c
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, 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"
38 app/panel/components/Settings/TrustAndRestrict.jsx
View file
Open in desktop
@@ -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,44 +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!

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) {
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-Z1-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, '\\$&');
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;
}

/**
114 app/panel/components/Settings/__tests__/TrustAndRestrict.jsx
View file
Open in desktop
@@ -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(<TrustAndRestrict />);
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(<TrustAndRestrict />);

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(<TrustAndRestrict />);

let input = '<script>*</script>';
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(<TrustAndRestrict />);

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(<TrustAndRestrict />);

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(<TrustAndRestrict />);

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);
});
});
1 package.json
View file
Open in desktop
@@ -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",
27 src/classes/Policy.js
View file
Open in desktop
@@ -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;
}
}
ProTip! Use n and p to navigate between commits in a pull request.