Skip to content
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

GH-1477 Wildcard/Regex Whitelisting #497

Closed
wants to merge 24 commits into from
Closed
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
7db8cdd
Add regex and wildcard functionality to whitelist and blacklist
benstrumeyer Feb 8, 2020
7fdc2d8
Escape inputted regex and add error handling
benstrumeyer Feb 10, 2020
f07e151
Add escape-string-regexp dependency
benstrumeyer Feb 10, 2020
3cff5eb
Refactor matchesWildcardOrRegex and remove escape-strings-regex depen…
benstrumeyer Feb 11, 2020
a03896f
Make regex variables const
benstrumeyer Feb 11, 2020
b025685
Merge branch 'develop' into regex-whitelisting
christophertino Feb 14, 2020
3930487
Prevent ReDoS attack. Validate url, wildcard or regex. Update error m…
benstrumeyer Feb 18, 2020
48e8e60
Merge branch 'regex-whitelisting' of github.com:ghostery/ghostery-ext…
benstrumeyer Feb 18, 2020
63ec3b6
Remove newline
benstrumeyer Feb 18, 2020
fc1f621
Add period to error text
benstrumeyer Feb 18, 2020
fcd5e9a
Merge branch 'develop' into regex-whitelisting
benstrumeyer Feb 18, 2020
b9186bc
GH-1947 Plus checkout UTM params (#499)
benstrumeyer Feb 21, 2020
a726e87
update translations
christophertino Feb 21, 2020
be06d00
Add regex and wildcard functionality to whitelist and blacklist
benstrumeyer Feb 8, 2020
863f225
Escape inputted regex and add error handling
benstrumeyer Feb 10, 2020
032cbc6
Add escape-string-regexp dependency
benstrumeyer Feb 10, 2020
8a6533c
Refactor matchesWildcardOrRegex and remove escape-strings-regex depen…
benstrumeyer Feb 11, 2020
5e6e3c2
Make regex variables const
benstrumeyer Feb 11, 2020
16b89a7
Prevent ReDoS attack. Validate url, wildcard or regex. Update error m…
benstrumeyer Feb 18, 2020
1c63fcb
Remove newline
benstrumeyer Feb 18, 2020
d35ef22
Add period to error text
benstrumeyer Feb 18, 2020
6fdf07b
Create unit and snapshot test for isValidUrlWildcard function
benstrumeyer Feb 19, 2020
ab3ce25
Add unit tests for background portion
benstrumeyer Feb 21, 2020
abe1de5
Fix merge conflicts
benstrumeyer Feb 21, 2020
File filter
Filter file types
Jump to
Jump to file
Failed to load files.

Always

Just for now

Add regex and wildcard functionality to whitelist and blacklist
  • Loading branch information
benstrumeyer committed Feb 21, 2020
commit be06d00b16dbfa22a880a91daa5bc776577ab184
@@ -86,9 +86,10 @@ class TrustAndRestrict extends React.Component {
/**
* Implement adding site to the list of whitelisted or blacklisted sites
* This routine valides entered url and checks if the entered url is a duplicate, or
* if it has been alreday added to the opposite list. Displays appropriate warnings.
* if it has been already added to the opposite list. Displays appropriate warnings.
*/
addSite() {
// TODO: Implement paygating, use this regex for validating free users who don't have regex/wildcard ability
This conversation was marked as resolved by Eden12345

This comment has been minimized.

@Eden12345

Eden12345 Feb 10, 2020
Contributor

Just curious as to why we aren't implementing the paygating in this PR?

This comment has been minimized.

@benstrumeyer

benstrumeyer Feb 11, 2020
Author Contributor

Chris said don't worry about it for now. I think he wanted me to focus on implementation first.

This comment has been minimized.

@Eden12345

Eden12345 Feb 11, 2020
Contributor

Ah okay, then let's update the ticket to reflect that — unless @christophertino you would like me to pair up with @benstrumeyer and knock out the paygating real quick? It should be pretty straightforward since we have the account/subscription info in the AccountReducer.

This comment has been minimized.

@Eden12345

Eden12345 Feb 11, 2020
Contributor

@benstrumeyer Looks like @christophertino doesn't want this to be a Plus feature anyway, and he said he'd update the ticket to refelect that.

// 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;
This conversation was marked as resolved by Eden12345

This comment has been minimized.

@Eden12345

Eden12345 Feb 11, 2020
Contributor

Let's remove this variable if we aren't going to be using it.

This comment has been minimized.

@christophertino

christophertino Feb 14, 2020
Member

We still need front-end validation here so that we can prevent users from adding urls in unsupported formats. You should check that the url entered is either a valid url or a valid regex, and show an error if not.

This comment has been minimized.

@benstrumeyer

benstrumeyer Feb 18, 2020
Author Contributor

I checked if it's a valid URL, wildcard, or regex and updated the error messages. Let me know if you'd like to change anything else

let pageHost;
@@ -121,7 +122,7 @@ 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.showWarning(t('white_black_list_error_invalid_url'));
return;
}
@@ -71,7 +71,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 +120,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 +176,25 @@ 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 regex, or a wildcard
* @param {string} url site url
* @param {string} pattern regex pattern
* @return {boolean}
*/
matchesWildcardOrRegex(url, pattern) {
let regex;
try {
regex = RegExp(pattern);
if (regex.test(url)) { return true; }
} catch {
const wildcardPattern = pattern.replace(/\*/g, '.*');
regex = RegExp(wildcardPattern);
if (regex.test(url)) { return true; }
}
return false;
}
}

export default Policy;
ProTip! Use n and p to navigate between commits in a pull request.