Ruby: add regex injection query #6978

nickrolfe · 2021-10-27T15:18:17Z

No description provided.

github-actions · 2021-10-27T15:18:49Z

QHelp previews:

ruby/ql/src/queries/security/cwe-1333/RegExpInjection.qhelp

github-actions · 2021-10-28T10:10:05Z

QHelp previews:

ruby/ql/src/queries/security/cwe-1333/RegExpInjection.qhelp

Regular expression injection

Constructing a regular expression with unsanitized user input is dangerous, since a malicious user may be able to modify the meaning of the expression. In particular, such a user may be able to provide a regular expression fragment that takes exponential time in the worst case, and use that to perform a Denial of Service attack.

Recommendation

Before embedding user input into a regular expression, use a sanitization function such as Regexp.escape to escape meta-characters that have special meaning.

Example

The following examples construct regular expressions from an HTTP request parameter without sanitizing it first:

class UsersController < ActionController::Base
  def first_example
    # BAD: Unsanitized user input is used to construct a regular expression
    regex = /#{ params[:key] }/
  end

  def second_example
    # BAD: Unsanitized user input is used to construct a regular expression
    regex = Regexp.new(params[:key])
  end
end

Instead, the request parameter should be sanitized first. This ensures that the user cannot insert characters that have special meanings in regular expressions.

class UsersController < ActionController::Base
  def example
    # GOOD: User input is sanitized before constructing the regular expression
    regex = Regexp.new(Regex.escape(params[:key]))
  end
end

References

OWASP: Regular expression Denial of Service - ReDoS.
Wikipedia: ReDoS.
Ruby: Regexp.escape.
Common Weakness Enumeration: CWE-1333.
Common Weakness Enumeration: CWE-730.
Common Weakness Enumeration: CWE-400.

nickrolfe · 2021-10-29T10:18:01Z

I still need to get a successful DCA run to check performance, but otherwise I think this ready for review.

github-actions · 2021-10-29T10:29:14Z

QHelp previews:

ruby/ql/src/queries/security/cwe-1333/RegExpInjection.qhelp

Regular expression injection

Constructing a regular expression with unsanitized user input is dangerous, since a malicious user may be able to modify the meaning of the expression. In particular, such a user may be able to provide a regular expression fragment that takes exponential time in the worst case, and use that to perform a Denial of Service attack.

Recommendation

Before embedding user input into a regular expression, use a sanitization function such as Regexp.escape to escape meta-characters that have special meaning.

Example

The following examples construct regular expressions from an HTTP request parameter without sanitizing it first:

class UsersController < ActionController::Base
  def first_example
    # BAD: Unsanitized user input is used to construct a regular expression
    regex = /#{ params[:key] }/
  end

  def second_example
    # BAD: Unsanitized user input is used to construct a regular expression
    regex = Regexp.new(params[:key])
  end
end

Instead, the request parameter should be sanitized first. This ensures that the user cannot insert characters that have special meanings in regular expressions.

class UsersController < ActionController::Base
  def example
    # GOOD: User input is sanitized before constructing the regular expression
    regex = Regexp.new(Regex.escape(params[:key]))
  end
end

References

OWASP: Regular expression Denial of Service - ReDoS.
Wikipedia: ReDoS.
Ruby: Regexp.escape.
Common Weakness Enumeration: CWE-1333.
Common Weakness Enumeration: CWE-730.
Common Weakness Enumeration: CWE-400.

…tion

github-actions · 2021-11-03T11:56:33Z

QHelp previews:

ruby/ql/src/queries/security/cwe-1333/RegExpInjection.qhelp

Regular expression injection

Constructing a regular expression with unsanitized user input is dangerous, since a malicious user may be able to modify the meaning of the expression. In particular, such a user may be able to provide a regular expression fragment that takes exponential time in the worst case, and use that to perform a Denial of Service attack.

Recommendation

Before embedding user input into a regular expression, use a sanitization function such as Regexp.escape to escape meta-characters that have special meaning.

Example

The following examples construct regular expressions from an HTTP request parameter without sanitizing it first:

class UsersController < ActionController::Base
  def first_example
    # BAD: Unsanitized user input is used to construct a regular expression
    regex = /#{ params[:key] }/
  end

  def second_example
    # BAD: Unsanitized user input is used to construct a regular expression
    regex = Regexp.new(params[:key])
  end
end

Instead, the request parameter should be sanitized first. This ensures that the user cannot insert characters that have special meanings in regular expressions.

class UsersController < ActionController::Base
  def example
    # GOOD: User input is sanitized before constructing the regular expression
    regex = Regexp.new(Regex.escape(params[:key]))
  end
end

References

OWASP: Regular expression Denial of Service - ReDoS.
Wikipedia: ReDoS.
Ruby: Regexp.escape.
Common Weakness Enumeration: CWE-1333.
Common Weakness Enumeration: CWE-730.
Common Weakness Enumeration: CWE-400.

nickrolfe · 2021-11-04T11:29:35Z

I still need to add a missing QLDoc comment.

More worryingly, the DCA runs show that this query causes significant slowdowns. I'll have to investigate when I have time.

github-actions · 2021-11-12T11:14:25Z

QHelp previews:

ruby/ql/src/queries/security/cwe-1333/RegExpInjection.qhelp

Regular expression injection

Constructing a regular expression with unsanitized user input is dangerous, since a malicious user may be able to modify the meaning of the expression. In particular, such a user may be able to provide a regular expression fragment that takes exponential time in the worst case, and use that to perform a Denial of Service attack.

Recommendation

Before embedding user input into a regular expression, use a sanitization function such as Regexp.escape to escape meta-characters that have special meaning.

Example

The following examples construct regular expressions from an HTTP request parameter without sanitizing it first:

class UsersController < ActionController::Base
  def first_example
    # BAD: Unsanitized user input is used to construct a regular expression
    regex = /#{ params[:key] }/
  end

  def second_example
    # BAD: Unsanitized user input is used to construct a regular expression
    regex = Regexp.new(params[:key])
  end
end

Instead, the request parameter should be sanitized first. This ensures that the user cannot insert characters that have special meanings in regular expressions.

class UsersController < ActionController::Base
  def example
    # GOOD: User input is sanitized before constructing the regular expression
    regex = Regexp.new(Regex.escape(params[:key]))
  end
end

References

OWASP: Regular expression Denial of Service - ReDoS.
Wikipedia: ReDoS.
Ruby: Regexp.escape.
Common Weakness Enumeration: CWE-1333.
Common Weakness Enumeration: CWE-730.
Common Weakness Enumeration: CWE-400.

nickrolfe · 2021-11-15T19:16:48Z

As far as I can tell from DCA, the new query takes about the same amount of time as any other dataflow query.

hmac · 2021-11-22T15:33:39Z

ruby/ql/lib/codeql/ruby/frameworks/StandardLibrary.qll

@@ -335,3 +336,18 @@ class ModuleEvalCallCodeExecution extends CodeExecution::Range, DataFlow::CallNo

  override DataFlow::Node getCode() { result = this.getArgument(0) }
 }
+
+/** Flow summary for `Regexp.escape`. */


Not one for this PR, but I wonder if we ought to split out StandardLibrary.qll soon? Maybe something like (following the structure of https://ruby-doc.com/):

ql/lib/codeql/ruby/frameworks ├── core │ ├── Kernel.qll │ └── Object.qll │ └── Regexp.qll └── std-lib ├── Date.qll └── Open3.qll

hmac · 2021-11-22T15:52:51Z

ruby/ql/lib/codeql/ruby/frameworks/StandardLibrary.qll

+  RegexpEscapeSummary() { this = "Regexp.escape" }
+
+  override MethodCall getACall() {
+    result = API::getTopLevelMember("Regexp").getAMethodCall("escape").asExpr().getExpr()


Regexp.quote is an alias for Regexp.escape, so we could add that here too (and in RegExpInjectionCustomizations).

hmac · 2021-11-22T16:00:22Z

ruby/ql/lib/codeql/ruby/regexp/RegExpInjectionCustomizations.qll

+  class ConstructedRegExpAsSink extends Sink {
+    ConstructedRegExpAsSink() {
+      this =
+        API::getTopLevelMember("Regexp").getAnInstantiation().(DataFlow::CallNode).getArgument(0)


There are a few other ways I've found for creating regexes:

Regexp.compile, which is an alias for Regexp.new

~~Regexp.union, which will create a regex matching a union of its arguments~~

Ah but Regexp.union escapes its arguments, so we don't have to bother with that.

hmac

This looks great to me! I only have a couple of small suggestions for additional sinks/sanitizers.

…tion

github-actions · 2021-11-22T18:03:37Z

QHelp previews:

ruby/ql/src/queries/security/cwe-1333/RegExpInjection.qhelp

Regular expression injection

Constructing a regular expression with unsanitized user input is dangerous, since a malicious user may be able to modify the meaning of the expression. In particular, such a user may be able to provide a regular expression fragment that takes exponential time in the worst case, and use that to perform a Denial of Service attack.

Recommendation

Before embedding user input into a regular expression, use a sanitization function such as Regexp.escape to escape meta-characters that have special meaning.

Example

The following examples construct regular expressions from an HTTP request parameter without sanitizing it first:

class UsersController < ActionController::Base
  def first_example
    # BAD: Unsanitized user input is used to construct a regular expression
    regex = /#{ params[:key] }/
  end

  def second_example
    # BAD: Unsanitized user input is used to construct a regular expression
    regex = Regexp.new(params[:key])
  end
end

Instead, the request parameter should be sanitized first. This ensures that the user cannot insert characters that have special meanings in regular expressions.

class UsersController < ActionController::Base
  def example
    # GOOD: User input is sanitized before constructing the regular expression
    regex = Regexp.new(Regex.escape(params[:key]))
  end
end

References

OWASP: Regular expression Denial of Service - ReDoS.
Wikipedia: ReDoS.
Ruby: Regexp.escape.
Common Weakness Enumeration: CWE-1333.
Common Weakness Enumeration: CWE-730.
Common Weakness Enumeration: CWE-400.

github-actions · 2021-11-23T11:07:07Z

QHelp previews:

ruby/ql/src/queries/security/cwe-1333/RegExpInjection.qhelp

Regular expression injection

Constructing a regular expression with unsanitized user input is dangerous, since a malicious user may be able to modify the meaning of the expression. In particular, such a user may be able to provide a regular expression fragment that takes exponential time in the worst case, and use that to perform a Denial of Service attack.

Recommendation

Before embedding user input into a regular expression, use a sanitization function such as Regexp.escape to escape meta-characters that have special meaning.

Example

The following examples construct regular expressions from an HTTP request parameter without sanitizing it first:

class UsersController < ActionController::Base
  def first_example
    # BAD: Unsanitized user input is used to construct a regular expression
    regex = /#{ params[:key] }/
  end

  def second_example
    # BAD: Unsanitized user input is used to construct a regular expression
    regex = Regexp.new(params[:key])
  end
end

Instead, the request parameter should be sanitized first. This ensures that the user cannot insert characters that have special meanings in regular expressions.

class UsersController < ActionController::Base
  def example
    # GOOD: User input is sanitized before constructing the regular expression
    regex = Regexp.new(Regex.escape(params[:key]))
  end
end

References

OWASP: Regular expression Denial of Service - ReDoS.
Wikipedia: ReDoS.
Ruby: Regexp.escape.
Common Weakness Enumeration: CWE-1333.
Common Weakness Enumeration: CWE-730.
Common Weakness Enumeration: CWE-400.

hmac · 2021-11-23T11:44:06Z

@nickrolfe it looks like some consistency tests are failing now, which I assume is unrelated to this change. Not sure what we can do to fix them, but ping me if you get this green and I'll 👍 it.

…tion

github-actions · 2021-11-23T15:43:20Z

QHelp previews:

ruby/ql/src/queries/security/cwe-1333/RegExpInjection.qhelp

Regular expression injection

Constructing a regular expression with unsanitized user input is dangerous, since a malicious user may be able to modify the meaning of the expression. In particular, such a user may be able to provide a regular expression fragment that takes exponential time in the worst case, and use that to perform a Denial of Service attack.

Recommendation

Before embedding user input into a regular expression, use a sanitization function such as Regexp.escape to escape meta-characters that have special meaning.

Example

The following examples construct regular expressions from an HTTP request parameter without sanitizing it first:

class UsersController < ActionController::Base
  def first_example
    # BAD: Unsanitized user input is used to construct a regular expression
    regex = /#{ params[:key] }/
  end

  def second_example
    # BAD: Unsanitized user input is used to construct a regular expression
    regex = Regexp.new(params[:key])
  end
end

Instead, the request parameter should be sanitized first. This ensures that the user cannot insert characters that have special meanings in regular expressions.

class UsersController < ActionController::Base
  def example
    # GOOD: User input is sanitized before constructing the regular expression
    regex = Regexp.new(Regex.escape(params[:key]))
  end
end

References

OWASP: Regular expression Denial of Service - ReDoS.
Wikipedia: ReDoS.
Ruby: Regexp.escape.
Common Weakness Enumeration: CWE-1333.
Common Weakness Enumeration: CWE-730.
Common Weakness Enumeration: CWE-400.

nickrolfe · 2021-11-23T16:06:00Z

@hmac it's all green now!

Ruby: add regex injection query

11154a9

nickrolfe added the Ruby label Oct 27, 2021

github-actions bot added the documentation label Oct 27, 2021

intrigus-lgtm reviewed Oct 27, 2021

View reviewed changes

ruby/ql/src/queries/security/cwe-1333/RegExpInjection.qhelp Outdated Show resolved Hide resolved

Ruby: fix qhelp

bd92403

nickrolfe marked this pull request as ready for review October 29, 2021 10:17

nickrolfe requested a review from a team as a code owner October 29, 2021 10:17

Ruby: add change note for rb/regexp-injection

fed0a06

Merge remote-tracking branch 'origin/main' into nickrolfe/regex_injec…

dd17271

…tion

Ruby: add file-level qldoc

9034d74

hmac reviewed Nov 22, 2021

View reviewed changes

hmac previously approved these changes Nov 22, 2021

View reviewed changes

nickrolfe added 4 commits November 22, 2021 17:05

Merge remote-tracking branch 'origin/main' into nickrolfe/regex_injec…

752b126

…tion

Ruby: fix up import path

5b11cfe

Ruby: handle Regexp.quote wherever we handle Regexp.escape

4b42c44

Ruby: add Regexp.compile as sink for regexp injection query

13459c8

nickrolfe dismissed hmac’s stale review via 13459c8 November 22, 2021 18:02

Ruby: add Regexp.{compile,quote} to regex injection test

e5f4730

Merge remote-tracking branch 'origin/main' into nickrolfe/regex_injec…

1a90b38

…tion

hmac approved these changes Nov 23, 2021

View reviewed changes

nickrolfe merged commit bb38c4d into main Nov 23, 2021

nickrolfe deleted the nickrolfe/regex_injection branch November 23, 2021 16:22

Ruby: add regex injection query #6978

Ruby: add regex injection query #6978

Uh oh!

Conversation

nickrolfe commented Oct 27, 2021

Uh oh!

github-actions bot commented Oct 27, 2021

Uh oh!

Uh oh!

github-actions bot commented Oct 28, 2021

Regular expression injection

Recommendation

Example

References

Uh oh!

nickrolfe commented Oct 29, 2021

Uh oh!

github-actions bot commented Oct 29, 2021

Regular expression injection

Recommendation

Example

References

Uh oh!

github-actions bot commented Nov 3, 2021

Regular expression injection

Recommendation

Example

References

Uh oh!

nickrolfe commented Nov 4, 2021

Uh oh!

github-actions bot commented Nov 12, 2021

Regular expression injection

Recommendation

Example

References

Uh oh!

nickrolfe commented Nov 15, 2021

Uh oh!

hmac Nov 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hmac Nov 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hmac Nov 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hmac Nov 22, 2021

Choose a reason for hiding this comment

Uh oh!

hmac left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 22, 2021

Regular expression injection

Recommendation

Example

References

Uh oh!

github-actions bot commented Nov 23, 2021

Regular expression injection

Recommendation

Example

References

Uh oh!

hmac commented Nov 23, 2021

Uh oh!

github-actions bot commented Nov 23, 2021

Regular expression injection

Recommendation

Example

References

Uh oh!

nickrolfe commented Nov 23, 2021

Uh oh!

Uh oh!

hmac Nov 22, 2021 •

edited

Loading

hmac Nov 22, 2021 •

edited

Loading

hmac Nov 22, 2021 •

edited

Loading