Skip to content

ReDoS: add another example to the qhelp in poly-redos, showing how to just limit the length of the input #13164

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
May 23, 2023
Merged

ReDoS: add another example to the qhelp in poly-redos, showing how to just limit the length of the input #13164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d989359
add another example to the qhelp in poly-redos, showing how to just l…
erik-krogh May 15, 2023
83ca149
trim the whitespace in the poly-redos examples
erik-krogh May 15, 2023
480e71f
avoid contractions
erik-krogh May 17, 2023
10bf17c
Merge branch 'main' into polyQhelp
erik-krogh May 21, 2023
710b309
apply suggestions from doc review
erik-krogh May 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 29 additions & 4 deletions java/ql/src/Security/CWE/CWE-730/PolynomialReDoS.qhelp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@
</p>

<sample language="java">
Pattern.compile("^\\s+|\\s+$").matcher(text).replaceAll("") // BAD
</sample>
Pattern.compile("^\\s+|\\s+$").matcher(text).replaceAll("") // BAD</sample>

<p>

Expand Down Expand Up @@ -71,8 +70,7 @@
</p>

<sample language="java">
"^0\\.\\d+E?\\d+$""
</sample>
"^0\\.\\d+E?\\d+$"" </sample>

<p>

Expand Down Expand Up @@ -103,6 +101,33 @@

</example>

<example>
<p>
Sometimes it is unclear how a regular expression can be rewritten to
avoid the problem. In such cases, it often suffices to limit the
length of the input string. For instance, the following
regular expression is used to match numbers, and on some non-number
inputs it can have quadratic time complexity:
</p>

<sample language="java">
Pattern.matches("^(\\+|-)?(\\d+|(\\d*\\.\\d*))?(E|e)?([-+])?(\\d+)?$", str); </sample>

<p>
It is not immediately obvious how to rewrite this regular expression
to avoid the problem. However, you can mitigate performance issues by limiting the length
to 1000 characters, which will always finish in a reasonable amount
of time.
</p>

<sample language="java">
if (str.length() &gt; 1000) {
throw new IllegalArgumentException("Input too long");
}

Pattern.matches("^(\\+|-)?(\\d+|(\\d*\\.\\d*))?(E|e)?([-+])?(\\d+)?$", str); </sample>
</example>

<include src="ReDoSReferences.inc.qhelp"/>

</qhelp>
33 changes: 29 additions & 4 deletions javascript/ql/src/Performance/PolynomialReDoS.qhelp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@
</p>

<sample language="javascript">
text.replace(/^\s+|\s+$/g, ''); // BAD
</sample>
text.replace(/^\s+|\s+$/g, ''); // BAD</sample>

<p>

Expand Down Expand Up @@ -71,8 +70,7 @@
</p>

<sample language="javascript">
/^0\.\d+E?\d+$/.test(str) // BAD
</sample>
/^0\.\d+E?\d+$/.test(str) // BAD</sample>

<p>

Expand Down Expand Up @@ -103,6 +101,33 @@

</example>

<example>
<p>
Sometimes it is unclear how a regular expression can be rewritten to
avoid the problem. In such cases, it often suffices to limit the
length of the input string. For instance, the following
regular expression is used to match numbers, and on some non-number
inputs it can have quadratic time complexity:
</p>

<sample language="javascript">
/^(\+|-)?(\d+|(\d*\.\d*))?(E|e)?([-+])?(\d+)?$/.test(str) // BAD</sample>

<p>
It is not immediately obvious how to rewrite this regular expression
to avoid the problem. However, you can mitigate performance issues by limiting the length
to 1000 characters, which will always finish in a reasonable amount
of time.
</p>

<sample language="javascript">
if (str.length &gt; 1000) {
throw new Error("Input too long");
}

/^(\+|-)?(\d+|(\d*\.\d*))?(E|e)?([-+])?(\d+)?$/.test(str)</sample>
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we add a line of whitespace here to match the other examples?

Suggested change
/^(\+|-)?(\d+|(\d*\.\d*))?(E|e)?([-+])?(\d+)?$/.test(str)</sample>
/^(\+|-)?(\d+|(\d*\.\d*))?(E|e)?([-+])?(\d+)?$/.test(str)</sample>

</example>

<include src="ReDoSReferences.inc.qhelp"/>

</qhelp>
32 changes: 28 additions & 4 deletions python/ql/src/Security/CWE-730/PolynomialReDoS.qhelp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@
</p>

<sample language="python">
re.sub(r"^\s+|\s+$", "", text) # BAD
</sample>
re.sub(r"^\s+|\s+$", "", text) # BAD</sample>

<p>

Expand Down Expand Up @@ -71,8 +70,7 @@
</p>

<sample language="python">
^0\.\d+E?\d+$ # BAD
</sample>
^0\.\d+E?\d+$ # BAD</sample>

<p>

Expand Down Expand Up @@ -103,6 +101,32 @@

</example>

<example>
<p>
Sometimes it is unclear how a regular expression can be rewritten to
avoid the problem. In such cases, it often suffices to limit the
length of the input string. For instance, the following
regular expression is used to match numbers, and on some non-number
inputs it can have quadratic time complexity:
</p>

<sample language="python">
match = re.search(r'^(\+|-)?(\d+|(\d*\.\d*))?(E|e)?([-+])?(\d+)?$', str) </sample>

<p>
It is not immediately obvious how to rewrite this regular expression
to avoid the problem. However, you can mitigate performance issues by limiting the length
to 1000 characters, which will always finish in a reasonable amount
of time.
</p>

<sample language="python">
if len(str) &gt; 1000:
raise ValueError("Input too long")

match = re.search(r'^(\+|-)?(\d+|(\d*\.\d*))?(E|e)?([-+])?(\d+)?$', str) </sample>
</example>

<include src="ReDoSReferences.inc.qhelp"/>

</qhelp>
33 changes: 29 additions & 4 deletions ruby/ql/src/queries/security/cwe-1333/PolynomialReDoS.qhelp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@
</p>

<sample language="ruby">
text.gsub!(/^\s+|\s+$/, '') # BAD
</sample>
text.gsub!(/^\s+|\s+$/, '') # BAD</sample>

<p>

Expand Down Expand Up @@ -74,8 +73,7 @@
</p>

<sample language="ruby">
/^0\.\d+E?\d+$/ # BAD
</sample>
/^0\.\d+E?\d+$/ # BAD</sample>

<p>

Expand Down Expand Up @@ -108,6 +106,33 @@

</example>

<example>
<p>
Sometimes it is unclear how a regular expression can be rewritten to
avoid the problem. In such cases, it often suffices to limit the
length of the input string. For instance, the following
regular expression is used to match numbers, and on some non-number
inputs it can have quadratic time complexity:
</p>

<sample language="ruby">
is_matching = /^(\+|-)?(\d+|(\d*\.\d*))?(E|e)?([-+])?(\d+)?$/.match?(str)</sample>

<p>
It is not immediately obvious how to rewrite this regular expression
to avoid the problem. However, you can mitigate performance issues by limiting the length
to 1000 characters, which will always finish in a reasonable amount
of time.
</p>

<sample language="ruby">
if str.length &gt; 1000
raise ArgumentError, "Input too long"
end

is_matching = /^(\+|-)?(\d+|(\d*\.\d*))?(E|e)?([-+])?(\d+)?$/.match?(str)</sample>
</example>

<include src="ReDoSReferences.inc.qhelp"/>

</qhelp>