Increased query robustness and test coverage

Author: thiggy1342

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

@@ -22,12 +22,14 @@ class DecompressionAPIUse extends DataFlow::Node {
     // this should find the first argument of Zlib::Inflate.inflate or Zip::File.extract
     DecompressionAPIUse() {
         this = API::getTopLevelMember("Zlib").getMember("Inflate").getAMethodCall("inflate").getArgument(0) or
-        this = API::getTopLevelMember("Zip").getMember("File").getAMethodCall("extract").getArgument(0)
+        this = API::getTopLevelMember("Zip").getMember("File").getAMethodCall("open").getArgument(0) or
+        this = API::getTopLevelMember("Zip").getMember("Entry").getAMethodCall("extract").getArgument(0)
     }
+
 }
 
 class Configuration extends TaintTracking::Configuration {
-    Configuration() { this = "DecompressionAPI" }
+    Configuration() { this = "DecompressionAPIUse" }
 
     // this predicate will be used to contstrain our query to find instances where only remote user-controlled data flows to the sink
     override predicate isSource(DataFlow::Node source) {

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

@@ -1,12 +1,20 @@
 edges
 | decompression_api.rb:3:31:3:36 | call to params :  | decompression_api.rb:3:31:3:43 | ...[...] |
 | decompression_api.rb:12:35:12:40 | call to params :  | decompression_api.rb:12:35:12:47 | ...[...] |
+| decompression_api.rb:17:27:17:32 | call to params :  | decompression_api.rb:17:27:17:39 | ...[...] |
+| decompression_api.rb:26:31:26:36 | call to params :  | decompression_api.rb:26:31:26: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:12:35:12:40 | call to params :  | semmle.label | call to params :  |
 | decompression_api.rb:12:35:12:47 | ...[...] | semmle.label | ...[...] |
+| decompression_api.rb:17:27:17:32 | call to params :  | semmle.label | call to params :  |
+| decompression_api.rb:17:27:17:39 | ...[...] | semmle.label | ...[...] |
+| decompression_api.rb:26:31:26:36 | call to params :  | semmle.label | call to params :  |
+| decompression_api.rb:26:31:26: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:12:35:12:47 | ...[...] | decompression_api.rb:12:35:12:40 | call to params :  | decompression_api.rb:12:35:12:47 | ...[...] | 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:17:27:17:39 | ...[...] | decompression_api.rb:17:27:17:32 | call to params :  | decompression_api.rb:17:27:17:39 | ...[...] | 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:26:31:26:43 | ...[...] | decompression_api.rb:26:31:26:36 | call to params :  | decompression_api.rb:26:31:26: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. |

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

@@ -7,8 +7,32 @@ def safe_zlib_unzip
         Zlib::Inflate.inflate("testfile.gz")
     end
 
+
+    DECOMPRESSION_LIB = Zlib
+    def unsafe_zlib_unzip_const
+        DECOMPRESSION_LIB::Inflate.inflate(params[:path])
+    end
+
+    def unsafe_zlib_unzip
+        Zip::File.open(params[:file]) do |zip_file|
+            zip_file.each do |entry|
+                entry.extract(entry.name)
+            end
+        end
+    end
+
+    def safe_zlib_unzip
+        Zip::Entry.extract("testfile.gz")
+    end
+
     def sanitized_zlib_unzip
-        if params[:path].in ["safe_file1.gz", "safe_file2.gz"]
+        if "safe_file.gz" == params[:path]
+            Zlib::Inflate.inflate(params[:path])
+        end
+    end
+
+    def sanitized_array_zlib_unzip
+        if ["safe_file1.gz", "safe_file2.gz"].include? params[:path]
             Zlib::Inflate.inflate(params[:path])
         end
     end