Skip to content

Conversation

@Git-Hub-Chris
Copy link
Owner

Potential fix for https://github.com/Git-Hub-Chris/MicrosoftVsCode/security/code-scanning/59

The fix involves rewriting the ambiguous and potentially catastrophic part of the regex to ensure each iteration of the repetition "owns" its whitespace and arrow parts in an unambiguous way. Specifically:

  • Replace the inner (?:\s+->\s+(?<file2>.*))* so the whitespace around the -> cannot be confused with the trailing whitespace in its own file capture (i.e., .*? will try to consume whitespace, which can be then grabbed in the following repetition).
  • Instead of .*?, use something that will not overlap with the separator: for example, capture everything up to the -> or the end of line. This can be accomplished with a negated lookahead or by using a character class to exclude the separator sequence.
  • Use [^\s] or ([^\r\n]|(?!\s+->)) type approach to avoid ambiguity, but here we can use a character class that matches anything except the start of a separator, or a "greedy but up-to" match.

For Git diff files, filenames in commit messages do not contain literal tabs or ->, so matching up to the separator is safe.

Thus, for each "file" in the arrow-separated chain, we match any text up to (but not including) the next separator, replacing .*? with something like [^\r\n]+? (stopping at separator), or a lazy match up to lookahead for \s+-> or end of string.

So, rewrite as:

  • For file1: [^\r\n]+?
  • For file2: in the repeated group, say (?<file2>[^\r\n]+?)
  • For the repeated group, use a lookahead so the repeated whitespace and arrows can't be split up in exponentially many ways.

Concrete edit:
Replace this (line 70):

private readonly _regex = /^#\s+(modified|new file|deleted|renamed|copied|type change):\s+(?<file1>.*?)(?:\s+->\s+(?<file2>.*))*$/gm;

with:

private readonly _regex = /^#\s+(modified|new file|deleted|renamed|copied|type change):\s+(?<file1>[^\r\n]+?)(?:\s+->\s+(?<file2>[^\r\n]+?))*$/gm;

Now, each file is matched up to the next separator or end of line, never consuming separator whitespace in ambiguous ways.

No new imports or utility methods are needed.


Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…ression

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Signed-off-by: Christopher Birnie-Browne <153604499+Git-Hub-Chris@users.noreply.github.com>
@Git-Hub-Chris Git-Hub-Chris marked this pull request as ready for review October 2, 2025 06:24
@Git-Hub-Chris Git-Hub-Chris merged commit 8d65630 into Main Oct 2, 2025
11 checks passed
@Git-Hub-Chris Git-Hub-Chris deleted the alert-autofix-59 branch October 2, 2025 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants