Skip to content
Permalink
Browse files Browse the repository at this point in the history
fix potential regexp DDOS
  • Loading branch information
fabiosantoscode committed Jul 13, 2022
1 parent 839b81b commit a4da734
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 7 deletions.
5 changes: 3 additions & 2 deletions lib/compress/evaluate.js
Expand Up @@ -46,7 +46,8 @@ import {
makePredicate,
return_this,
string_template,
regexp_source_fix
regexp_source_fix,
regexp_is_safe,
} from "../utils/index.js";
import {
AST_Array,
Expand Down Expand Up @@ -129,7 +130,7 @@ def_eval(AST_BigInt, return_this);

def_eval(AST_RegExp, function (compressor) {
let evaluated = compressor.evaluated_regexps.get(this.value);
if (evaluated === undefined) {
if (evaluated === undefined && regexp_is_safe(this.value.source)) {
try {
const { source, flags } = this.value;
evaluated = new RegExp(source, flags);
Expand Down
2 changes: 2 additions & 0 deletions lib/compress/index.js
Expand Up @@ -158,6 +158,7 @@ import {
return_true,
regexp_source_fix,
has_annotation,
regexp_is_safe,
} from "../utils/index.js";
import { first_in_statement } from "../utils/first_in_statement.js";
import { equivalent_to } from "../equivalent-to.js";
Expand Down Expand Up @@ -2140,6 +2141,7 @@ def_optimize(AST_Call, function(self, compressor) {
params.push(value);
return arg !== value;
})
&& regexp_is_safe(params[0])
) {
let [ source, flags ] = params;
source = regexp_source_fix(new RegExp(source).source);
Expand Down
10 changes: 9 additions & 1 deletion lib/utils/index.js
Expand Up @@ -249,7 +249,15 @@ function regexp_source_fix(source) {
return (escaped ? "" : "\\") + lineTerminatorEscape[match];
});
}
const all_flags = "gimuy";

// Subset of regexps that is not going to cause regexp based DDOS
// https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
const re_safe_regexp = /^[\\/|\0\s\w^$.[\]()]*$/;

/** Check if the regexp is safe for Terser to create without risking a RegExp DOS */
export const regexp_is_safe = (source) => re_safe_regexp.test(source);

const all_flags = "dgimsuy";
function sort_regexp_flags(flags) {
const existing_flags = new Set(flags.split(""));
let out = "";
Expand Down
21 changes: 17 additions & 4 deletions test/compress/regexp.js
Expand Up @@ -30,10 +30,10 @@ regexp_2: {
unsafe: true,
}
input: {
console.log(JSON.stringify("COMPASS? Overpass.".match(new RegExp("([Sap]+)", "ig"))));
console.log(JSON.stringify("COMPASS? Overpass.".match(new RegExp("(pass)", "ig"))));
}
expect: {
console.log(JSON.stringify("COMPASS? Overpass.".match(/([Sap]+)/gi)));
console.log(JSON.stringify("COMPASS? Overpass.".match(/(pass)/gi)));
}
expect_stdout: '["PASS","pass"]'
}
Expand All @@ -44,10 +44,10 @@ unsafe_slashes: {
unsafe: true
}
input: {
console.log(new RegExp("^https://"))
console.log(new RegExp("^https//"))
}
expect: {
console.log(/^https:\/\//)
console.log(/^https\/\//)
}
}
unsafe_nul_byte: {
Expand Down Expand Up @@ -75,3 +75,16 @@ inline_script: {
}
expect_exact: '/* <\\/script> */\n/[<\\/script>]/;'
}

regexp_no_ddos: {
options = { unsafe: true, evaluate: true }
input: {
console.log(/(b+)b+/.test("bbb"))
console.log(RegExp("(b+)b+").test("bbb"))
}
expect: {
console.log(/(b+)b+/.test("bbb"))
console.log(RegExp("(b+)b+").test("bbb"))
}
expect_stdout: ["true", "true"]

This comment has been minimized.

Copy link
@Victorcorcos

Victorcorcos Aug 18, 2022

1. regexp_is_safe(/(b+)b+/)
false
2. regexp_is_safe(/b+/)
false
3. regexp_is_safe(/b/)
true

Hi @fabiosantoscode wouldn't the case 2. be true?

This comment has been minimized.

This comment has been minimized.

Copy link
@fabiosantoscode

fabiosantoscode Aug 18, 2022

Author Collaborator

I prefered to err on the safe side here.

This case should be rare enough that nobody will care about the few instances Terser won't be compressing.

}

0 comments on commit a4da734

Please sign in to comment.