From 15fb73551b95b116310bd9d30f1d102b24a8e6aa Mon Sep 17 00:00:00 2001 From: Neil Henderson Date: Mon, 28 Jun 2021 15:02:03 +1000 Subject: [PATCH 1/4] Fixes #485. Added a timeout to NegatableTypeOrMemberReferenceRegex and MemberReferenceRegex to prevent CPU hang when given input that can cause expensive regex backtracking. --- .../CommonInterest.cs | 30 +++++++++++++++---- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs b/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs index eaa836aff..d70298300 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs @@ -63,9 +63,11 @@ internal static class CommonInterest private const RegexOptions FileNamePatternRegexOptions = RegexOptions.IgnoreCase | RegexOptions.Singleline; - private static readonly Regex NegatableTypeOrMemberReferenceRegex = new Regex(@"^(?!)?\[(?[^\[\]\:]+)+\](?:\:\:(?\S+))?\s*$", RegexOptions.Singleline | RegexOptions.CultureInvariant); + private static readonly TimeSpan RegexMatchTimeout = TimeSpan.FromMilliseconds(100); // Prevent expensive CPU hang in Regex.Match if backtracking occurs due to pathological input (see #485). - private static readonly Regex MemberReferenceRegex = new Regex(@"^\[(?[^\[\]\:]+)+\]::(?\S+)\s*$", RegexOptions.Singleline | RegexOptions.CultureInvariant); + private static readonly Regex NegatableTypeOrMemberReferenceRegex = new Regex(@"^(?!)?\[(?[^\[\]\:]+)+\](?:\:\:(?\S+))?\s*$", RegexOptions.Singleline | RegexOptions.CultureInvariant, RegexMatchTimeout); + + private static readonly Regex MemberReferenceRegex = new Regex(@"^\[(?[^\[\]\:]+)+\]::(?\S+)\s*$", RegexOptions.Singleline | RegexOptions.CultureInvariant, RegexMatchTimeout); /// /// An array with '.' as its only element. @@ -84,8 +86,16 @@ internal static IEnumerable ReadTypesAndMembers(AnalyzerOptions a { foreach (string line in ReadAdditionalFiles(analyzerOptions, fileNamePattern, cancellationToken)) { - Match match = NegatableTypeOrMemberReferenceRegex.Match(line); - if (!match.Success) + Match? match = null; + try + { + match = NegatableTypeOrMemberReferenceRegex.Match(line); + } + catch (RegexMatchTimeoutException) + { + } + + if (match == null || !match.Success) { throw new InvalidOperationException($"Parsing error on line: {line}"); } @@ -175,8 +185,16 @@ internal static IEnumerable ReadLinesFromAdditionalFile(SourceText text) internal static QualifiedMember ParseAdditionalFileMethodLine(string line) { - Match match = MemberReferenceRegex.Match(line); - if (!match.Success) + Match? match = null; + try + { + match = MemberReferenceRegex.Match(line); + } + catch (RegexMatchTimeoutException) + { + } + + if (match == null || !match.Success) { throw new InvalidOperationException($"Parsing error on line: {line}"); } From b03f81ab1d9ef0d69035666d185a8f1cf7168d1f Mon Sep 17 00:00:00 2001 From: Neil Henderson <2060747+bluetarpmedia@users.noreply.github.com> Date: Mon, 12 Jul 2021 09:07:31 +1000 Subject: [PATCH 2/4] Increased RegexMatchTimeout to 5 seconds based on feedback from PR code review. Co-authored-by: Andrew Arnott --- .../CommonInterest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs b/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs index d70298300..6e5f083df 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs @@ -63,7 +63,7 @@ internal static class CommonInterest private const RegexOptions FileNamePatternRegexOptions = RegexOptions.IgnoreCase | RegexOptions.Singleline; - private static readonly TimeSpan RegexMatchTimeout = TimeSpan.FromMilliseconds(100); // Prevent expensive CPU hang in Regex.Match if backtracking occurs due to pathological input (see #485). + private static readonly TimeSpan RegexMatchTimeout = TimeSpan.FromSeconds(5); // Prevent expensive CPU hang in Regex.Match if backtracking occurs due to pathological input (see #485). private static readonly Regex NegatableTypeOrMemberReferenceRegex = new Regex(@"^(?!)?\[(?[^\[\]\:]+)+\](?:\:\:(?\S+))?\s*$", RegexOptions.Singleline | RegexOptions.CultureInvariant, RegexMatchTimeout); From 866757eee279f36e1aa8046e162e8f9854736e6d Mon Sep 17 00:00:00 2001 From: Neil Henderson Date: Mon, 12 Jul 2021 09:14:02 +1000 Subject: [PATCH 3/4] Now reports Regex.Match timeout errors distinct from other parsing failures. --- .../CommonInterest.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs b/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs index 6e5f083df..75796e34f 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs @@ -93,6 +93,7 @@ internal static IEnumerable ReadTypesAndMembers(AnalyzerOptions a } catch (RegexMatchTimeoutException) { + throw new InvalidOperationException($"Regex.Match timeout when parsing line: {line}"); } if (match == null || !match.Success) @@ -192,6 +193,7 @@ internal static QualifiedMember ParseAdditionalFileMethodLine(string line) } catch (RegexMatchTimeoutException) { + throw new InvalidOperationException($"Regex.Match timeout when parsing line: {line}"); } if (match == null || !match.Success) From 352508b68c326c006459682149f01b45f507be7d Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Tue, 17 Aug 2021 11:17:51 -0600 Subject: [PATCH 4/4] Remove unnecessary `null` check --- .../CommonInterest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs b/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs index 75796e34f..acab90219 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs @@ -96,7 +96,7 @@ internal static IEnumerable ReadTypesAndMembers(AnalyzerOptions a throw new InvalidOperationException($"Regex.Match timeout when parsing line: {line}"); } - if (match == null || !match.Success) + if (!match.Success) { throw new InvalidOperationException($"Parsing error on line: {line}"); } @@ -196,7 +196,7 @@ internal static QualifiedMember ParseAdditionalFileMethodLine(string line) throw new InvalidOperationException($"Regex.Match timeout when parsing line: {line}"); } - if (match == null || !match.Success) + if (!match.Success) { throw new InvalidOperationException($"Parsing error on line: {line}"); }