From a4c42aa14bb8e8401abf87e8ca28a873ffc8d5aa Mon Sep 17 00:00:00 2001
From: erik-krogh <erik-krogh@github.com>
Date: Mon, 30 Jan 2023 16:46:13 +0100
Subject: [PATCH] more custom array steps from unsafe-code-construction to a
 utility predicate

---
 .../lib/codeql/ruby/frameworks/core/Array.qll | 19 +++++++++++++++++++
 .../security/UnsafeCodeConstructionQuery.qll  | 14 ++------------
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/ruby/ql/lib/codeql/ruby/frameworks/core/Array.qll b/ruby/ql/lib/codeql/ruby/frameworks/core/Array.qll
index b2fbf7870ce..ba84b489bde 100644
--- a/ruby/ql/lib/codeql/ruby/frameworks/core/Array.qll
+++ b/ruby/ql/lib/codeql/ruby/frameworks/core/Array.qll
@@ -1815,6 +1815,25 @@ module Array {
       preservesValue = true
     }
   }
+
+  /**
+   * Holds if there an array element `pred` might taint the array defined by `succ`.
+   * This is used for queries where we consider an entire array to be tainted if any of its elements are tainted.
+   */
+  predicate taintedArrayObjectSteps(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(DataFlow::CallNode call |
+      call.getMethodName() = ["<<", "push", "append"] and
+      call.getReceiver() = succ and
+      pred = call.getArgument(0) and
+      call.getNumberOfArguments() = 1
+    )
+    or
+    exists(DataFlow::CallNode call |
+      call.getMethodName() = "[]" and
+      succ = call and
+      pred = call.getArgument(_)
+    )
+  }
 }
 
 /**
diff --git a/ruby/ql/lib/codeql/ruby/security/UnsafeCodeConstructionQuery.qll b/ruby/ql/lib/codeql/ruby/security/UnsafeCodeConstructionQuery.qll
index 68e8c56100c..f4907a993b1 100644
--- a/ruby/ql/lib/codeql/ruby/security/UnsafeCodeConstructionQuery.qll
+++ b/ruby/ql/lib/codeql/ruby/security/UnsafeCodeConstructionQuery.qll
@@ -10,6 +10,7 @@ import codeql.ruby.DataFlow
 import UnsafeCodeConstructionCustomizations::UnsafeCodeConstruction
 private import codeql.ruby.TaintTracking
 private import codeql.ruby.dataflow.BarrierGuards
+private import codeql.ruby.frameworks.core.Array
 
 /**
  * A taint-tracking configuration for detecting code constructed from library input vulnerabilities.
@@ -33,17 +34,6 @@ class Configuration extends TaintTracking::Configuration {
 
   override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
     // if an array element gets tainted, then we treat the entire array as tainted
-    exists(DataFlow::CallNode call |
-      call.getMethodName() = ["<<", "push", "append"] and
-      call.getReceiver() = succ and
-      pred = call.getArgument(0) and
-      call.getNumberOfArguments() = 1
-    )
-    or
-    exists(DataFlow::CallNode call |
-      call.getMethodName() = "[]" and
-      succ = call and
-      pred = call.getArgument(_)
-    )
+    Array::taintedArrayObjectSteps(pred, succ)
   }
 }