Convert Bash escaping to support linear regexp engine#2358
Merged
ericcornelissen merged 6 commits intomainfrom Feb 8, 2026
Merged
Convert Bash escaping to support linear regexp engine#2358ericcornelissen merged 6 commits intomainfrom
ericcornelissen merged 6 commits intomainfrom
Conversation
Update `bash.js` to use the lregexp package to transparently create regular expressions that use the linear engine in order to avoid any ReDoS vulnerabilities. This commit target the logic behind Shescape's `escape(All)` API only. The refactoring is mostly straightforward with the exception of the logic for `~` (home directory expansion). Please note that this logic is related to CVE-2022-24725, so extra care has been taken to avoid re-introducing a vulnerability. In particular, before this change there were two regular expressions that escaped `~`, namely: - `/(?<=^|\s)([#~])/gu` - `/(?<=[:=])(~)(?=[\s+\-/0:=]|$)/gu` The former is for the straightforward case where `~` (or `#`, comments) is preceded by whitespace (or is at the start of the string, in which case it is implicitly preceded by whitespace) in which case Bash expand it to the user's home directory. The former is a more complicated case in which It is the second expression that has been changed. In particular, we now always escape `~` when it is preceded by either `=` or `:` instead of conditionally based on what succeeds it. The reason for this change is that the linear regular expression engine does not support lookaheads, but the suffix of the expression that used to be on line 19 may be part of the prefix of the same expression. With lookaheads the succeeding characters are not consumed so can be matched again for the prefix, but without lookaheads this is not the case. For this reason we now escape unconditionally (on the suffix), which also allows us to fold the two separate expressions into one. The impact of this change is that we may now unnecessarily escape `~` in certain cases, as evidenced by the fixture changes, which 1) is not a problem because it does not affect the meaning of the command, and 2) is extremely rare in practice except for malicious commands. As demonstrated by the explanation of the changes as well as the changes to the test fixtures, this change should be safe (w.r.t. the concerns of CVE-2022-24725) as the new implementation only escapes `~` in more scenarios.
ericcornelissen
added
the
refactor
|
Here's the code health analysis summary for commits Analysis Summary
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
ericcornelissen
changed the title
bash using lregexpbash escaping to support linear regexp engine
ericcornelissen
changed the title
bash escaping to support linear regexp engine
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relates to #2122