add detection of sources directly used with blocks

Author: gregxsunday

ruby/ql/lib/codeql/ruby/security/ZipSlipCustomizations.qll

@@ -45,7 +45,11 @@ module ZipSlip {
    */
   private class GzipReaderOpen extends Source {
     GzipReaderOpen() {
-      this = API::getTopLevelMember("Zlib").getMember("GzipReader").getReturn("open").asSource() and
+      (
+        this = API::getTopLevelMember("Zlib").getMember("GzipReader").getReturn("open").asSource()
+        or
+        this = API::getTopLevelMember("Zlib").getMember("GzipReader").getInstance().asSource()
+      ) and
       // If argument refers to a string object, then it's a hardcoded path and
       // this file is safe.
       not this.(DataFlow::CallNode)
@@ -61,19 +65,17 @@ module ZipSlip {
    */
   private class TarReaderInstance extends Source {
     TarReaderInstance() {
-      this =
-        API::getTopLevelMember("Gem")
-            .getMember("Package")
-            .getMember("TarReader")
-            .getInstance()
-            .asSource() and
-      // If argument refers to a string object, then it's a hardcoded path and
-      // this file is safe.
-      not this.(DataFlow::CallNode)
-          .getArgument(0)
-          .getALocalSource()
-          .getConstantValue()
-          .isStringlikeValue(_)
+      exists(API::MethodAccessNode newTarReader |
+        newTarReader =
+          API::getTopLevelMember("Gem").getMember("Package").getMember("TarReader").getMethod("new")
+      |
+        // Unlike in two other modules, there's no check for the constant path because TarReader class is called with an `io` object and not filepath.
+        // This, of course, can be modeled but probably in the internal IO.qll file
+        // For now, I'm leaving this prone to false-positives
+        not exists(newTarReader.getBlock()) and this = newTarReader.getReturn().asSource()
+        or
+        this = newTarReader.getBlock().getParameter(0).asSource()
+      )
     }
   }
 
@@ -82,14 +84,23 @@ module ZipSlip {
    */
   private class ZipFileOpen extends Source {
     ZipFileOpen() {
-      this = API::getTopLevelMember("Zip").getMember("File").getReturn("open").asSource() and
-      // If argument refers to a string object, then it's a hardcoded path and
-      // this file is safe.
-      not this.(DataFlow::CallNode)
-          .getArgument(0)
-          .getALocalSource()
-          .getConstantValue()
-          .isStringlikeValue(_)
+      exists(API::MethodAccessNode zipOpen |
+        zipOpen = API::getTopLevelMember("Zip").getMember("File").getMethod("open") and
+        // If argument refers to a string object, then it's a hardcoded path and
+        // this file is safe.
+        not zipOpen
+            .getCallNode()
+            .getArgument(0)
+            .getALocalSource()
+            .getConstantValue()
+            .isStringlikeValue(_)
+      |
+        // the case with variable assignment `zip_file = Zip::File.open(path)`
+        not exists(zipOpen.getBlock()) and this = zipOpen.getReturn().asSource()
+        or
+        // the case with direct block`Zip::File.open(path) do |zip_file|` case
+        this = zipOpen.getBlock().getParameter(0).asSource()
+      )
     }
   }