-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Description
Problem Statement
We've had incidents in the past with dynamically constructing regular expressions based on user input.
fictional example:
new RegExp(`\/${paramValue}\/`)Solution Brainstorm
Since this is highly problematic in certain situations, we should add a lint rule that flags usage of dynamically created regular expressions. In many situations, it's totally fine to opt-out/ignore this rule if the RegExp usage is safe but it's better to have a mechanism in place that explicitly and automatically reminds us of being extra careful.
Out of scope/already covered:
- Analysis of "static" regular expressions like
/some(regex)?/. We have CodeQL in CI which already checks for ReDoS susceptible expressions in each PR
Alternative: There's a CodeQL rule that we currently haven't enabled that more aggressively checks for RegExp usage. In this case we could also enable this rule instead of adding a lint rule.
This is an action item from INC-585. @anonrig will investigate applicable rules we'll discuss/evaluate them afterwards.