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

Request for Adding Regular Expression Denial of Service Check to ASVS #1562

Closed
ImanSharaf opened this issue Feb 28, 2023 · 9 comments · Fixed by #1712
Closed

Request for Adding Regular Expression Denial of Service Check to ASVS #1562

ImanSharaf opened this issue Feb 28, 2023 · 9 comments · Fixed by #1712
Labels
6) PR awaiting review V10 tmp code-review Temporary label for grouping code review or program code related issues _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@ImanSharaf
Copy link
Collaborator

I noticed that the ASVS is missing a check for Regular Expression Denial of Service (ReDoS) vulnerabilities. As you know, ReDoS attacks can have a serious impact on application availability and performance, and are often overlooked in security testing.

ReDoS attacks occur when an attacker can manipulate an application's input in such a way that causes a regular expression pattern to take an extremely long time to process, leading to a denial of service condition. Since regular expressions are commonly used in applications for input validation, search functions, and more, it is crucial that ReDoS vulnerabilities are detected and mitigated.

I strongly recommend that the ASVS be updated to include a ReDoS check, a sample check could be the following:

  • Verify that regular expressions used in the application have a reasonable upper limit on the length and complexity of input they can accept.
@ImanSharaf ImanSharaf changed the title Request for Adding Regular Expression Denial of Service Check in ASVS Request for Adding Regular Expression Denial of Service Check to ASVS Feb 28, 2023
@elarlang
Copy link
Collaborator

elarlang commented Mar 1, 2023

Just a comment - keep adding those issues, we will analyze them when can find reasonable amount of time for that, this is nice input material! :)

@ImanSharaf
Copy link
Collaborator Author

@elarlang I am an Application Security Lead [certified OSWE and AWS Security expert] at Forward Security (based in Vancouver - @jmanico knows us) and we use ASVS all the time in our engagements and I want to appreciate all your efforts for this awesome piece of work.

@elarlang elarlang added the V10 tmp code-review Temporary label for grouping code review or program code related issues label Mar 10, 2023
@tghosth
Copy link
Collaborator

tghosth commented Mar 15, 2023

(Just to second Elar's point, really appreciate all the effort you are putting in @ImanSharaf, thanks!!!)

What do you think about the following:

Verify that regular expressions in use do not include unsafe elements which could lead to exponential processing and that untrusted input is not included in expressions without sanitization. This is to prevent a ReDOS / Runaway Regex attack.

Do you have a good link reference that could go with this as it is a painfully complicated topic?

@tghosth tghosth added 2) Awaiting response Awaiting a response from the original poster _5.0 - prep This needs to be addressed to prepare 5.0 labels Mar 15, 2023
@ImanSharaf
Copy link
Collaborator Author

Thank you for your positive feedback and for considering the addition of a ReDoS check to ASVS. I appreciate your dedication to making the standard as comprehensive as possible. For the reference, what do you think about this one?

In a ReDoS attack, an attacker exploits weaknesses in a poorly designed regular expression by providing input that takes an extremely long time to process. This can cause the application to become unresponsive or slow, consuming excessive computational resources and potentially leading to a denial of service condition.

The main reason behind the vulnerability to ReDoS attacks lies in the use of "greedy" or "catastrophic backtracking" regular expressions. These expressions can lead to an exponential increase in processing time when matching certain inputs, especially those containing multiple repeating characters.

To simplify, imagine a poorly designed regular expression that is trying to match a pattern in a user's input. If the input contains repeating characters or a specific sequence that triggers the vulnerability, the regular expression engine might take an excessive amount of time to process the input, causing other users to experience delays or even making the application unresponsive.

To prevent ReDoS attacks, it is important to:

  1. Use efficient regular expressions that avoid catastrophic backtracking and do not have nested quantifiers.
  2. Limit the maximum length and complexity of input that is processed by regular expressions.
  3. Sanitize untrusted input before it is used in regular expressions.
  4. Implement timeouts or resource limits to prevent excessive processing.

To detect ReDoS vulnerabilities in JavaScript code, follow these steps:

  1. Locate regular expressions: Look for the use of RegExp.test(), String.match(), String.search(), or other regular expression functions (in JS). These functions are where regular expressions are being used and could potentially be vulnerable.
  2. Identify nested quantifiers: Examine the regular expression patterns for nested quantifiers, such as *,+, or {m,n}. Nested quantifiers can lead to excessive backtracking and are a common cause of ReDoS vulnerabilities. In the example above, the pattern ^(a+)+$ has nested quantifiers with the two + signs.
  3. Check for greedy matching: Greedy matching can also cause ReDoS vulnerabilities when the regular expression engine tries to consume as much input as possible. Look for patterns that use greedy matching, such as .*, .+, or .{m,n}.
  4. Test with crafted inputs: To confirm that a specific regular expression is vulnerable to ReDoS, you can test it with crafted inputs that exploit the vulnerability. For example, with the pattern ^(a+)+$, you could test it with a string like aaaaaaaaaaaaaaaaaaaaaaaaaaaaaab. If the regular expression engine takes a long time to process this input, it's an indication of a ReDoS vulnerability.

Also, we always can use SASTs to help us find suspicious regexes.

@tghosth
Copy link
Collaborator

tghosth commented May 31, 2023

Ok so @ImanSharaf it would be good if you could look through the document you linked to and open a PR here with any changes/enhancements you feel are necessary based on your comment here. Maybe even consider transforming it into an official OWASP Cheat Sheet?

@elarlang
Copy link
Collaborator

For category it waits: #1643

@tghosth
Copy link
Collaborator

tghosth commented Jun 15, 2023

Ok so @ImanSharaf it would be good if you could look through the document you linked to and open a PR here with any changes/enhancements you feel are necessary based on your comment here. Maybe even consider transforming it into an official OWASP Cheat Sheet?

Hi @ImanSharaf, are you ok with this and would you accept my suggested proposal for now:

Verify that regular expressions in use do not include unsafe elements which could lead to exponential processing and that untrusted input is not included in expressions without sanitization. This is to prevent a ReDOS / Runaway Regex attack.

@jmanico
Copy link
Member

jmanico commented Aug 30, 2023

Josh I think this is a great idea for a new requirement. I made it more concise.

Verify that regular expressions are free from elements causing exponential backtracking, and ensure untrusted input is sanitized to mitigate ReDOS / Runaway Regex attacks.

tghosth added a commit that referenced this issue Sep 7, 2023
@tghosth tghosth linked a pull request Sep 7, 2023 that will close this issue
@tghosth
Copy link
Collaborator

tghosth commented Sep 7, 2023

Created #1712 with Jim's wording. CWE-1333 seems correct and as per my comment here I think it neatly fits into the sanitization category,

@tghosth tghosth added 6) PR awaiting review and removed 2) Awaiting response Awaiting a response from the original poster labels Sep 7, 2023
elarlang pushed a commit that referenced this issue Sep 7, 2023
elarlang pushed a commit to elarlang/ASVS that referenced this issue Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6) PR awaiting review V10 tmp code-review Temporary label for grouping code review or program code related issues _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants