tweaks and add Zip::File.open_buffer to query

Author: thiggy1342

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

@@ -18,11 +18,18 @@ import codeql.ruby.TaintTracking
 import DataFlow::PathGraph
 
 class DecompressionApiUse extends DataFlow::Node {
+  private DataFlow::CallNode call;
+
   // this should find the first argument of Zlib::Inflate.inflate
   DecompressionApiUse() {
-    this =
-      API::getTopLevelMember("Zlib").getMember("Inflate").getAMethodCall("inflate").getArgument(0)
+    this = call.getArgument(0) and
+    (
+      call = API::getTopLevelMember("Zlib").getMember("Inflate").getAMethodCall("inflate") or
+      call = API::getTopLevelMember("Zip").getMember("File").getAMethodCall("open_buffer")
+    )
   }
+
+  DataFlow::CallNode getCall() { result = call }
 }
 
 class Configuration extends TaintTracking::Configuration {
@@ -32,12 +39,14 @@ class Configuration extends TaintTracking::Configuration {
   override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
 
   // our Decompression APIs defined above will the the sinks we use for this query
-  override predicate isSink(DataFlow::Node sink) { sink instanceof DecompressionApiUse }
+  override predicate isSink(DataFlow::Node sink) {
+    sink.(DataFlow::CallNode) instanceof DecompressionApiUse
+  }
 }
 
 from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
 where config.hasFlowPath(source, sink)
-select sink.getNode(), 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().asExpr().getExpr().getParent(),
-  sink.getNode().asExpr().getExpr().getParent().toString()
+  sink.getNode().(DecompressionApiUse).getCall(),
+  sink.getNode().(DecompressionApiUse).getCall().getMethodName()

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

@@ -1,12 +1,12 @@
 edges
-| decompression_api.rb:3:31:3:36 | call to params :  | decompression_api.rb:3:31:3:43 | ...[...] |
-| decompression_api.rb:13:44:13:49 | call to params :  | decompression_api.rb:13:44:13:56 | ...[...] |
+| decompression_api.rb:4:31:4:36 | call to params :  | decompression_api.rb:4:31:4:43 | ...[...] |
+| decompression_api.rb:14:31:14:36 | call to params :  | decompression_api.rb:14:31:14:43 | ...[...] |
 nodes
-| decompression_api.rb:3:31:3:36 | call to params :  | semmle.label | call to params :  |
-| decompression_api.rb:3:31:3:43 | ...[...] | semmle.label | ...[...] |
-| decompression_api.rb:13:44:13:49 | call to params :  | semmle.label | call to params :  |
-| decompression_api.rb:13:44:13:56 | ...[...] | semmle.label | ...[...] |
+| decompression_api.rb:4:31:4:36 | call to params :  | semmle.label | call to params :  |
+| decompression_api.rb:4:31:4:43 | ...[...] | semmle.label | ...[...] |
+| decompression_api.rb:14:31:14:36 | call to params :  | semmle.label | call to params :  |
+| decompression_api.rb:14:31:14:43 | ...[...] | semmle.label | ...[...] |
 subpaths
 #select
-| decompression_api.rb:3:31:3:43 | ...[...] | decompression_api.rb:3:31:3:36 | call to params :  | decompression_api.rb:3:31:3: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:3:9:3:44 | call to inflate | call to inflate |
-| decompression_api.rb:13:44:13:56 | ...[...] | decompression_api.rb:13:44:13:49 | call to params :  | decompression_api.rb:13:44:13:56 | ...[...] | 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:13:9:13:57 | call to inflate | call to inflate |
+| decompression_api.rb:4:31:4:43 | ...[...] | decompression_api.rb:4:31:4:36 | call to params :  | decompression_api.rb:4:31:4: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:4:9:4:44 | call to inflate | inflate |
+| decompression_api.rb:14:31:14:43 | ...[...] | decompression_api.rb:14:31:14:36 | call to params :  | decompression_api.rb:14:31:14: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:14:9:14:44 | call to open_buffer | open_buffer |

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

@@ -1,15 +1,21 @@
 class TestController < ActionController::Base
+    # this should get picked up
     def unsafe_zlib_unzip
         Zlib::Inflate.inflate(params[:file])
     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
 
-    DECOMPRESSION_LIB = Zlib
-    def unsafe_zlib_unzip_const
-        DECOMPRESSION_LIB::Inflate.inflate(params[:file])
+    # this should not get picked up
+    def safe_zlib_unzip
+        Zip::File.open_buffer(file)
     end
 end