Merge pull request github#10782 from erik-krogh/rbPoly

Ruby: add library input as a source for `rb/polynomial-redos`

Author: erik-krogh

ruby/ql/lib/codeql/ruby/frameworks/core/Gem.qll

@@ -85,9 +85,13 @@ module Gem {
 
     /** Gets a parameter from an exported method, which is an input to this gem. */
     DataFlow::ParameterNode getAnInputParameter() {
-      exists(MethodBase method | method = getAPublicModule().getAMethod() |
-        result.getParameter() = method.getAParameter() and
+      exists(MethodBase method |
+        method = getAPublicModule().getAMethod() and
+        result.getParameter() = method.getAParameter()
+      |
         method.isPublic()
+        or
+        method.isProtected()
       )
     }
   }

ruby/ql/lib/codeql/ruby/security/regexp/PolynomialReDoSCustomizations.qll

@@ -24,7 +24,13 @@ module PolynomialReDoS {
   /**
    * A data flow source node for polynomial regular expression denial-of-service vulnerabilities.
    */
-  abstract class Source extends DataFlow::Node { }
+  abstract class Source extends DataFlow::Node {
+    /**
+     * Gets a string that describes the source.
+     * For use in the alert message.
+     */
+    string describe() { result = "user-provided value" }
+  }
 
   /**
    * A data flow sink node for polynomial regular expression denial-of-service vulnerabilities.
@@ -55,6 +61,15 @@ module PolynomialReDoS {
    */
   class RemoteFlowSourceAsSource extends Source, RemoteFlowSource { }
 
+  private import codeql.ruby.frameworks.core.Gem::Gem as Gem
+
+  /** A library input, considered as a flow source. */
+  class LibraryInputAsSource extends Source {
+    LibraryInputAsSource() { this = Gem::getALibraryInput() }
+
+    override string describe() { result = "library input" }
+  }
+
   /**
    * A regexp match against a superlinear backtracking term, seen as a sink for
    * polynomial regular expression denial-of-service vulnerabilities.

ruby/ql/src/change-notes/2022-10-11-poly-redos-lib.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The `rb/polynomial-redos` query now considers the entrypoints of the API of a gem as sources.

ruby/ql/src/queries/security/cwe-1333/PolynomialReDoS.ql

@@ -27,4 +27,4 @@ where
 select sinkNode.getHighlight(), source, sink,
   "This $@ that depends on a $@ may run slow on strings " + regexp.getPrefixMessage() +
     "with many repetitions of '" + regexp.getPumpString() + "'.", regexp, "regular expression",
-  source.getNode(), "user-provided value"
+  source.getNode(), source.getNode().(PolynomialReDoS::Source).describe()

ruby/ql/test/query-tests/security/cwe-1333-polynomial-redos/PolynomialReDoS.expected

@@ -34,6 +34,8 @@ edges
 | PolynomialReDoS.rb:70:12:70:24 | ...[...] :  | PolynomialReDoS.rb:73:32:73:35 | name :  |
 | PolynomialReDoS.rb:73:32:73:35 | name :  | PolynomialReDoS.rb:76:35:76:39 | input :  |
 | PolynomialReDoS.rb:76:35:76:39 | input :  | PolynomialReDoS.rb:77:5:77:9 | input |
+| lib/index.rb:2:11:2:11 | x :  | lib/index.rb:4:13:4:13 | x |
+| lib/index.rb:8:13:8:13 | x :  | lib/index.rb:9:15:9:15 | x |
 nodes
 | PolynomialReDoS.rb:4:12:4:17 | call to params :  | semmle.label | call to params :  |
 | PolynomialReDoS.rb:4:12:4:24 | ...[...] :  | semmle.label | ...[...] :  |
@@ -76,6 +78,10 @@ nodes
 | PolynomialReDoS.rb:73:32:73:35 | name :  | semmle.label | name :  |
 | PolynomialReDoS.rb:76:35:76:39 | input :  | semmle.label | input :  |
 | PolynomialReDoS.rb:77:5:77:9 | input | semmle.label | input |
+| lib/index.rb:2:11:2:11 | x :  | semmle.label | x :  |
+| lib/index.rb:4:13:4:13 | x | semmle.label | x |
+| lib/index.rb:8:13:8:13 | x :  | semmle.label | x :  |
+| lib/index.rb:9:15:9:15 | x | semmle.label | x |
 subpaths
 #select
 | PolynomialReDoS.rb:10:5:10:17 | ... =~ ... | PolynomialReDoS.rb:4:12:4:17 | call to params :  | PolynomialReDoS.rb:10:5:10:8 | name | This $@ that depends on a $@ may run slow on strings with many repetitions of ' '. | PolynomialReDoS.rb:7:19:7:21 | \\s+ | regular expression | PolynomialReDoS.rb:4:12:4:17 | call to params | user-provided value |
@@ -101,3 +107,5 @@ subpaths
 | PolynomialReDoS.rb:62:5:62:22 | call to gsub | PolynomialReDoS.rb:54:12:54:17 | call to params :  | PolynomialReDoS.rb:62:5:62:9 | input | This $@ that depends on a $@ may run slow on strings with many repetitions of ' '. | PolynomialReDoS.rb:56:31:56:33 | \\s+ | regular expression | PolynomialReDoS.rb:54:12:54:17 | call to params | user-provided value |
 | PolynomialReDoS.rb:66:5:66:34 | call to match? | PolynomialReDoS.rb:54:12:54:17 | call to params :  | PolynomialReDoS.rb:66:5:66:9 | input | This $@ that depends on a $@ may run slow on strings with many repetitions of ' '. | PolynomialReDoS.rb:58:30:58:32 | \\s+ | regular expression | PolynomialReDoS.rb:54:12:54:17 | call to params | user-provided value |
 | PolynomialReDoS.rb:77:5:77:22 | call to gsub | PolynomialReDoS.rb:70:12:70:17 | call to params :  | PolynomialReDoS.rb:77:5:77:9 | input | This $@ that depends on a $@ may run slow on strings with many repetitions of ' '. | PolynomialReDoS.rb:72:28:72:30 | \\s+ | regular expression | PolynomialReDoS.rb:70:12:70:17 | call to params | user-provided value |
+| lib/index.rb:4:13:4:26 | call to match | lib/index.rb:2:11:2:11 | x :  | lib/index.rb:4:13:4:13 | x | This $@ that depends on a $@ may run slow on strings with many repetitions of 'a'. | lib/index.rb:4:22:4:23 | a+ | regular expression | lib/index.rb:2:11:2:11 | x | library input |
+| lib/index.rb:9:15:9:28 | call to match | lib/index.rb:8:13:8:13 | x :  | lib/index.rb:9:15:9:15 | x | This $@ that depends on a $@ may run slow on strings with many repetitions of 'a'. | lib/index.rb:9:24:9:25 | a+ | regular expression | lib/index.rb:8:13:8:13 | x | library input |

ruby/ql/test/query-tests/security/cwe-1333-polynomial-redos/lib/index.rb

@@ -0,0 +1,11 @@
+module Foo 
+  def bar(x) 
+    # Run the /a+$/ regex on the input x.
+    match = x.match(/a+$/)
+  end
+
+  protected
+    def baz(x)
+      match = x.match(/a+$/)
+    end
+end

ruby/ql/test/query-tests/security/cwe-1333-polynomial-redos/poly-redos.gemspec

@@ -0,0 +1,5 @@
+Gem::Specification.new do |s|
+    s.name = 'poly-redos'
+    s.require_path = "lib"
+  end
+