Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix possible RegexMatchTimeoutException #1525

Merged
merged 1 commit into from
Jan 16, 2024

Conversation

mus65
Copy link
Contributor

@mus65 mus65 commented Jan 11, 2024

6bda890 introduced a Timeout on RegEx compilation. We hit this timeout a few times since this change.

Removed the timeout so it uses the default like before.

Stacktrace:

System.Text.RegularExpressions.RegexMatchTimeoutException: The Regex engine has timed out while trying to match a pattern to an input string. This can occur for many reasons, including very large inputs or excessive backtracking caused by nested quantifiers, back-references and other factors.
   at System.Text.RegularExpressions.RegexRunner.<CheckTimeout>g__ThrowRegexTimeout|25_0()
   at Regex5_TryMatchAtCurrentPosition(RegexRunner, ReadOnlySpan`1)
   at Regex5_Scan(RegexRunner, ReadOnlySpan`1)
   at System.Text.RegularExpressions.Regex.RunAllMatchesWithCallback[TState](String inputString, ReadOnlySpan`1 inputSpan, Int32 startat, TState& state, MatchCallback`1 callback, RegexRunnerMode mode, Boolean reuseMatchObject)
   at System.Text.RegularExpressions.RegexReplacement.ReplaceSimpleText(Regex regex, String input, String replacement, Int32 count, Int32 startat)
   at System.Text.RegularExpressions.Regex.Replace(String input, String replacement)
   at Microsoft.OpenApi.Validations.Rules.OpenApiPathsRules.<>c.<get_PathMustBeUnique>b__4_0(IValidationContext context, OpenApiPaths item)
   at Microsoft.OpenApi.Validations.ValidationRule`1.Evaluate(IValidationContext context, Object item)
   at Microsoft.OpenApi.Validations.OpenApiValidator.Validate(Object item, Type type)
   at Microsoft.OpenApi.Validations.OpenApiValidator.Validate[T](T item)
   at Microsoft.OpenApi.Validations.OpenApiValidator.Visit(OpenApiPaths item)
   at Microsoft.OpenApi.Services.OpenApiWalker.Walk(OpenApiPaths paths)
   at Microsoft.OpenApi.Services.OpenApiWalker.<>c__DisplayClass4_0.<Walk>b__2()
   at Microsoft.OpenApi.Services.OpenApiWalker.Walk(String context, Action walk)
   at Microsoft.OpenApi.Services.OpenApiWalker.Walk(OpenApiDocument doc)
   at Microsoft.OpenApi.Services.OpenApiWalker.Walk(IOpenApiElement element)
   at Microsoft.OpenApi.Extensions.OpenApiElementExtensions.Validate(IOpenApiElement element, ValidationRuleSet ruleSet)
   at Microsoft.OpenApi.Readers.OpenApiYamlDocumentReader.Read(YamlDocument input, OpenApiDiagnostic& diagnostic)
   at Microsoft.OpenApi.Readers.OpenApiTextReaderReader.Read(TextReader input, OpenApiDiagnostic& diagnostic)
   at Microsoft.OpenApi.Readers.OpenApiStringReader.Read(String input, OpenApiDiagnostic& diagnostic)

Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mus65,
Thanks for the contribution.
The timeout is a security requirement here. Depending on where the library is used, attackers could inject an extremely long path to DDoS a service if there was no timeout.
Instead of removing the timeout, I encourage you to explore the following route:

  • replace the regex by string operations (index of, substring, etc)
  • remove captures (if not necessary)
  • reduce back-propagation.

@baywet baywet mentioned this pull request Jan 11, 2024
@mus65 mus65 force-pushed the regexmatchtimeout branch from e3ce8ce to 3df0b7a Compare January 13, 2024 11:06
@mus65
Copy link
Contributor Author

mus65 commented Jan 13, 2024

@baywet replaced the regex with IndexOf/Substring and rebased to take #1504 into account.

@mus65 mus65 force-pushed the regexmatchtimeout branch from 3df0b7a to 0a19160 Compare January 13, 2024 12:23
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update, here are a few recommendations

@mus65

This comment was marked as outdated.

@mus65
Copy link
Contributor Author

mus65 commented Jan 15, 2024

@microsoft-github-policy-service agree company="TIS GmbH"

@mus65 mus65 force-pushed the regexmatchtimeout branch from 0a19160 to a99baf2 Compare January 15, 2024 16:20
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for making the changes

@baywet
Copy link
Member

baywet commented Jan 15, 2024

@MaggieKimani1 for final review and merge

return path;
}

path = path.Substring(0, openBrace + 1) + path.Substring(closeBrace);

Check notice

Code scanning / CodeQL

String concatenation in loop Note

String concatenation in loop: use 'StringBuilder'.
@MaggieKimani1 MaggieKimani1 merged commit a97138f into microsoft:vnext Jan 16, 2024
6 checks passed
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.

3 participants