Potential catastrophic backtracking

[davisjam]

One of the regexes in eslint is vulnerable to catastrophic backtracking.

@nzakas says this isn't a problem as used in eslint:

Oh yeah, that’s basically just used to insert data into placeholders for our error messages. So we can have a message like this:

“Expected {{count}} spaces.”

And be able to insert a value for “count” without doing string concatenation.

We did this in order to avoid pulling in a whole library like Mustache. Given that there are whole templating libraries on the server, I can’t imagine why someone would want to use this.

Perhaps a comment to warn future copy/pasters might be in order, however.

Here's a description of the problematic pattern and an attack string.

{
"pattern" : "\{\{\s*([^{}]+?)\s*\}\}",
"nPumpsFor10Sec" : "48325",
"filesIn" : [
[
"lib/util/interpolate.js"
]
],
"blowupCurve" : {
"r2" : 0.996141596724248,
"parms" : [
3.10391182858704e-08,
1.82075786106333
],
"type" : "POWER"
},
"attackFormat" : {
"suffix" : "{\t{",
"pumpPairs" : [
{
"pump" : "a",
"prefix" : "{{\t"
},
{
"pump" : "\t",
"prefix" : "a"
}
]
},
"stringLenFor10Sec" : 96657
}

[ljharb]

Since this would have to be caused by a malicious rule, and the "victim" of the attack would have to enable the rule (or the shared config providing it) themselves, this doesn't seem worrying at all.

[davisjam]

Agreed, not a problem as currently used in eslint. But since people like to copy/paste regexes, thought I would let you know about it.

[platinumazure]

Can the regex be improved to prevent catastrophic backtracking without changing the function at all? If so, I would love to see a pull request.

If not, though, I agree that this isn't worth worrying about.

[davisjam]

Sure. See #10019.