You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We've had incidents in the past with dynamically constructing regular expressions based on user input.
fictional example:
newRegExp(`\/${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.
Ok, so from biome, we already use the noControlCharactersInRegex rule and we can easily add useRegexLiterals (although the automatic fix is still super buggy with "/" path characters.
I'll open a PR soon which adds the useRegexLiterals rule.
the noMisleadingCharacterClassrule doesn't seem to be stable at all yet (if I understand vnext correctly).
While I think all of these rules are worthwhile to have, they wouldn't capture the problematic RegExp usage from the vulnerability, as they don't flag the general usage of new RegExp or when a variable is used within the constructor.
Activity
anonrig commentedon Dec 21, 2023
Biome rules relevant to Regexp:
mydea commentedon Dec 22, 2023
Sounds like a good idea to me!
Lms24 commentedon Jan 2, 2024
Ok, so from biome, we already use the
noControlCharactersInRegexrule and we can easily adduseRegexLiterals(although the automatic fix is still super buggy with"/"path characters.I'll open a PR soon which adds the
useRegexLiteralsrule.the
noMisleadingCharacterClassrule doesn't seem to be stable at all yet (if I understandvnextcorrectly).While I think all of these rules are worthwhile to have, they wouldn't capture the problematic
RegExpusage from the vulnerability, as they don't flag the general usage ofnew RegExpor when a variable is used within the constructor.