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…

Add more tests. Check if a pageHost has valid url or regex characters

  • Loading branch information
@benstrumeyer
benstrumeyer committed Feb 25, 2020
commit 4831bf5c30e415020a6e1970a9984846ffb9fa26
28 app/panel/components/Settings/TrustAndRestrict.jsx
View file
Open in desktop
@@ -141,32 +141,36 @@ class TrustAndRestrict extends React.Component {
}
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) {
// 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;

// Check for valid regex
let isValidRegex = true;
try {
// eslint-disable-next-line
new RegExp(pageHost);
new RegExp(escapedPattern);
} catch {
isValidRegex = false;
}
162 app/panel/components/Settings/__tests__/TrustAndRestrict.jsx
View file
Open in desktop
@@ -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(<TrustAndRestrict />);
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(<TrustAndRestrict />);
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 = '<script>*</script>';
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(<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 unsafe regex entered', () => {
test('isValidUrlWildcardOrRegex should return false with regex entered', () => {
const wrapper = shallow(<TrustAndRestrict />);
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(<TrustAndRestrict />);
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);
});
});
30 src/classes/Policy.js
View file
Open in desktop
@@ -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('*')) {
This conversation was marked as resolved by christophertino

This comment has been minimized.

Sign in to view
@christophertino

christophertino Feb 26, 2020
Member

On fresh install, this is throwing a background error Cannot read property 'includes' of null

This comment has been minimized.

Sign in to view
@christophertino

christophertino Feb 26, 2020
Member

Looks to be coming from BrowserButton ln 154 this._setIcon(!globals.SESSION.paused_blocking && !this.policy.checkSiteWhitelist(tab.url), tabId, trackerCount, alert);

This comment has been minimized.

Sign in to view
@benstrumeyer

benstrumeyer Feb 27, 2020
Author Contributor

Nice catch, I've added a check to ensure pattern is truthy
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;
}
}
97 test/src/policy.test.js
View file
Open in desktop
@@ -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 = '<script>*</script>';
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 = '<script></script>';
expect(policy.matchesWildcardOrRegex(url, input)).toBeFalsy();
});
})
});
ProTip! Use n and p to navigate between commits in a pull request.