Skip to content
This repository has been archived by the owner on Nov 6, 2023. It is now read-only.

Detect static securecookies to trivialize more rules #16029

Merged
merged 10 commits into from
Aug 22, 2018
+93 −3
Merged

Detect static securecookies to trivialize more rules #16029

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
77f08cf
Detect static securecookies to trivialize more rules
Jul 11, 2018
ca2844f
Add comments on static cookie terminology
Jul 12, 2018
4bbd731
Remove securecookie tags if they are not effective
Jul 12, 2018
ca52ba2
Add comments on the safety removing dangling securecookies
Jul 13, 2018
5c149aa
Fix a bug for handling the dangling securecookie rules
Aug 7, 2018
a37df21
semistandard --fix explode-regexp.js trivialize-rules.js
Aug 7, 2018
db2b8b6
Revert "semistandard --fix explode-regexp.js trivialize-rules.js"
Aug 7, 2018
e8f27ee
Remove unused variables and fix syntax manually
Aug 9, 2018
6e82204
Update comments and remove unnecessary checks
Aug 13, 2018
e5ad3c5
Update comments on partial dangling securecookie
Aug 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions utils/trivialize-rules/explode-regexp.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ const { parse } = require('regulex');

class UnsupportedRegExp extends Error {}

function explodeRegExp(re, callback) {
(function buildUrls(str, items) {
function explodeRegExp (re, callback) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this file was previously formatted with prettier. Did it add these whitespaces or did you manually? Looks quite odd.

Copy link
Collaborator Author

@cschanaj cschanaj Aug 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I know which prettier options did you use? I can revert the changes in a37df21

P.S. a37df21 was mostly done automatically using semistandard from standardjs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think just single-quotes plus semicolons.

Copy link
Collaborator Author

@cschanaj cschanaj Aug 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that prettier will create some strange changes to trivialize-rules.js, e.g. spliting the back-tick quoted string to the following

+            let scReSrc = `\n([\t ]*)<securecookie\\s*host=\\s*"${escapeStringRegexp(
+              securecookie.host
+            )}"(\\s*)name=\\s*"${escapeStringRegexp(
+              securecookie.name
+            )}"\\s*?/>[\t ]*\n`;

So I guess it is better not to run prettier on it. Would you mind if I leave the file as-is in e8f27ee?

(function buildUrls (str, items) {
if (items.length === 0) {
callback(str + '*');
return;
Expand Down Expand Up @@ -79,7 +79,7 @@ function explodeRegExp(re, callback) {

throw new UnsupportedRegExp(first.raw);
})('*', parse(re).tree);
};
}

module.exports = {
UnsupportedRegExp,
Expand Down
23 changes: 10 additions & 13 deletions utils/trivialize-rules/trivialize-rules.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const rulesDir = `${__dirname}/../../src/chrome/content/rules`;

const tagsRegExps = new Map();

function createTagsRegexp(tag) {
function createTagsRegexp (tag) {
let re = tagsRegExps.get(tag);
if (!re) {
const tagRe = `<${tag}(?:\\s+\\w+=".*?")*\\s*\\/>`;
Expand All @@ -26,7 +26,7 @@ function createTagsRegexp(tag) {
return re;
}

function replaceXML(source, tag, newXML) {
function replaceXML (source, tag, newXML) {
let pos, indent;
let re = createTagsRegexp(tag);

Expand Down Expand Up @@ -75,12 +75,12 @@ const rules =
}
});

function isTrivial(rule) {
function isTrivial (rule) {
return rule.from === '^http:' && rule.to === 'https:';
}

files.fork().zipAll([ sources.fork(), rules ]).map(([name, source, ruleset]) => {
function createTag(tagName, colour, print) {
function createTag (tagName, colour, print) {
return (strings, ...values) => {
let result = `[${tagName}] ${chalk.bold(name)}: ${strings[0]}`;
for (let i = 1; i < strings.length; i++) {
Expand All @@ -103,16 +103,14 @@ files.fork().zipAll([ sources.fork(), rules ]).map(([name, source, ruleset]) =>
let securecookies = ruleset.securecookie ? ruleset.securecookie.map(sc => sc.$) : new Array();
let rules = ruleset.rule.map(rule => rule.$);

let shouldRemoveSecurecookies = false;

if (rules.length === 1 && isTrivial(rules[0])) {
return;
}

let targetRe = new RegExp(`^(?:${targets.map(target => target.replace(/\./g, '\\.').replace(/\*/g, '.*')).join('|')})$`);
let domains = new Set();

function isStatic(rule) {
function isStatic (rule) {
if (isTrivial(rule)) {
for (let target of targets) {
domains.add(target);
Expand Down Expand Up @@ -201,7 +199,7 @@ files.fork().zipAll([ sources.fork(), rules ]).map(([name, source, ruleset]) =>
// 3. Each exploded securecookie.host should be included in ruleset.target/
// exploded target. Otherwise, this ruleset is likely problematic itself. It
// is dangerous for a rewrite.
function isStaticCookie(securecookie) {
function isStaticCookie (securecookie) {
if (securecookie.host === '.+' && securecookie.name === '.+') {
return [true, false];
}
Expand All @@ -227,7 +225,7 @@ files.fork().zipAll([ sources.fork(), rules ]).map(([name, source, ruleset]) =>
warn`Unsupported regexp part ${e.message} while traversing securecookie : ${JSON.stringify(securecookie)}`;
return [false, false];
}

for (const domain of localDomains) {
if (domains.indexOf(domain) === -1) {
warn`Ruleset does not cover target ${domain} for securecookie : ${JSON.stringify(securecookie)}`;
Expand Down Expand Up @@ -271,17 +269,17 @@ files.fork().zipAll([ sources.fork(), rules ]).map(([name, source, ruleset]) =>
if (shouldRemove) {
let scReSrc = `\n([\t ]*)<securecookie\\s*host=\\s*"${escapeStringRegexp(securecookie.host)}"(\\s*)name=\\s*"${escapeStringRegexp(securecookie.name)}"\\s*?/>[\t ]*\n`;
let scRe = new RegExp(scReSrc);

if (scRe && scRe.test(source)) {
source = source.replace(scRe, '');
} else {
fail`Failed to construct regexp which matches securecookie: ${JSON.stringify(securecookie)}`;
return ;
return;
}
}
} else {
// Skip this ruleset as it contain non-static securecookies
return ;
return;
}
}

Expand All @@ -293,7 +291,6 @@ files.fork().zipAll([ sources.fork(), rules ]).map(([name, source, ruleset]) =>
info`trivialized`;

return writeFile(`${rulesDir}/${name}`, source);

})
.filter(Boolean)
.parallel(10)
Expand Down