Merge pull request #9432 from thiggy1342/experimental-decompression-api

RB: Adding decompression-api to experimental ruleset

Author: aibaars

ruby/ql/src/experimental/decompression-api/DecompressionApi.qhelp

@@ -0,0 +1,27 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+  <overview>
+    <p>
+      Decompression of user-controlled data without taking proper precaution can
+      result in uncontrolled and massive decompression on the server, resulting 
+      in a denial of service.
+    </p>
+  </overview>
+  <recommendation>
+    <p>
+      When decompressing files supplied by the user, make sure that you're checking
+      the size of the incoming data chunks before writing to an output.
+    </p>
+  </recommendation>
+  <example>
+    <p>
+      In this example, the size of the input buffer chunks and total size are checked before each chunk is written to the output.
+    </p>
+    <sample src="examples/decompress.rb" />
+  </example>
+  
+  <references>
+  </references>
+</qhelp>

ruby/ql/src/experimental/decompression-api/DecompressionApi.ql

@@ -0,0 +1,50 @@
+/**
+ * @name User-controlled file decompression
+ * @description User-controlled data that flows into decompression library APIs could be dangerous
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 7.8
+ * @precision medium
+ * @id rb/user-controlled-file-decompression
+ * @tags security external/cwe/cwe-409
+ */
+
+import ruby
+import codeql.ruby.ApiGraphs
+import codeql.ruby.DataFlow
+import codeql.ruby.dataflow.RemoteFlowSources
+import codeql.ruby.TaintTracking
+import DataFlow::PathGraph
+
+class DecompressionApiUse extends DataFlow::Node {
+  private DataFlow::CallNode call;
+
+  // this should find the first argument in calls to Zlib::Inflate.inflate or Zip::File.open_buffer
+  DecompressionApiUse() {
+    this = call.getArgument(0) and
+    (
+      call = API::getTopLevelMember("Zlib").getMember("Inflate").getAMethodCall("inflate") or
+      call = API::getTopLevelMember("Zip").getMember("File").getAMethodCall("open_buffer")
+    )
+  }
+
+  // returns calls to Zlib::Inflate.inflate or Zip::File.open_buffer
+  DataFlow::CallNode getCall() { result = call }
+}
+
+class Configuration extends TaintTracking::Configuration {
+  Configuration() { this = "DecompressionApiUse" }
+
+  // this predicate will be used to constrain our query to find instances where only remote user-controlled data flows to the sink
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  // our Decompression APIs defined above will be the sinks we use for this query
+  override predicate isSink(DataFlow::Node sink) { sink instanceof DecompressionApiUse }
+}
+
+from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink.getNode().(DecompressionApiUse), source, sink,
+  "This call to $@ is unsafe because user-controlled data is used to set the object being decompressed, which could lead to a denial of service attack or malicious code extracted from an unknown source.",
+  sink.getNode().(DecompressionApiUse).getCall(),
+  sink.getNode().(DecompressionApiUse).getCall().getMethodName()

ruby/ql/src/experimental/decompression-api/examples/decompress.rb

@@ -0,0 +1,17 @@
+class UsersController < ActionController::Base
+  def example_zlib_inflate
+    MAX_ALLOWED_CHUNK_SIZE = 256
+    MAX_ALLOWED_TOTAL_SIZE = 1024
+
+    user_data = params[:data]
+    output = []
+    outsize = 0
+
+    Zlib::Inflate.inflate(user_data) { |chunk|
+      outsize += chunk.size
+      if chunk.size < MAX_ALLOWED_CHUNK_SIZE && outsize < MAX_ALLOWED_TOTAL_SIZE
+        output << chunk
+      end
+    }
+  end
+end

ruby/ql/test/query-tests/security/decompression-api/DecompressionApi.expected

@@ -0,0 +1,14 @@
+edges
+| decompression_api.rb:4:16:4:21 | call to params :  | decompression_api.rb:4:16:4:28 | ...[...] :  |
+| decompression_api.rb:4:16:4:28 | ...[...] :  | decompression_api.rb:5:31:5:34 | path |
+| decompression_api.rb:15:31:15:36 | call to params :  | decompression_api.rb:15:31:15:43 | ...[...] |
+nodes
+| decompression_api.rb:4:16:4:21 | call to params :  | semmle.label | call to params :  |
+| decompression_api.rb:4:16:4:28 | ...[...] :  | semmle.label | ...[...] :  |
+| decompression_api.rb:5:31:5:34 | path | semmle.label | path |
+| decompression_api.rb:15:31:15:36 | call to params :  | semmle.label | call to params :  |
+| decompression_api.rb:15:31:15:43 | ...[...] | semmle.label | ...[...] |
+subpaths
+#select
+| decompression_api.rb:5:31:5:34 | path | decompression_api.rb:4:16:4:21 | call to params :  | decompression_api.rb:5:31:5:34 | path | This call to $@ is unsafe because user-controlled data is used to set the object being decompressed, which could lead to a denial of service attack or malicious code extracted from an unknown source. | decompression_api.rb:5:9:5:35 | call to inflate | inflate |
+| decompression_api.rb:15:31:15:43 | ...[...] | decompression_api.rb:15:31:15:36 | call to params :  | decompression_api.rb:15:31:15:43 | ...[...] | This call to $@ is unsafe because user-controlled data is used to set the object being decompressed, which could lead to a denial of service attack or malicious code extracted from an unknown source. | decompression_api.rb:15:9:15:44 | call to open_buffer | open_buffer |

ruby/ql/test/query-tests/security/decompression-api/DecompressionApi.qlref

@@ -0,0 +1 @@
+experimental/decompression-api/DecompressionApi.ql

ruby/ql/test/query-tests/security/decompression-api/decompression_api.rb

@@ -0,0 +1,22 @@
+class TestController < ActionController::Base
+    # this should get picked up
+    def unsafe_zlib_unzip
+        path = params[:file]
+        Zlib::Inflate.inflate(path)
+    end
+
+    # this should not get picked up
+    def safe_zlib_unzip
+        Zlib::Inflate.inflate(file)
+    end
+
+    # this should get picked up
+    def unsafe_zlib_unzip
+        Zip::File.open_buffer(params[:file])
+    end
+
+    # this should not get picked up
+    def safe_zlib_unzip
+        Zip::File.open_buffer(file)
+    end
+end