Merge branch 'main' into python/enable-summaries-from-models

ruby/extractor/Cargo.toml

@@ -10,7 +10,7 @@ edition = "2018"
 [dependencies]
 tree-sitter = "0.20"
 tree-sitter-embedded-template = { git = "https://github.com/tree-sitter/tree-sitter-embedded-template.git", rev = "203f7bd3c1bbfbd98fc19add4b8fcb213c059205" }
-tree-sitter-ruby = { git = "https://github.com/tree-sitter/tree-sitter-ruby.git", rev = "206c7077164372c596ffa8eaadb9435c28941364" }
+tree-sitter-ruby = { git = "https://github.com/tree-sitter/tree-sitter-ruby.git", rev = "2edbd437ee901b8fa95861ec538e56efe3ebd127" }
 clap = { version = "4.2", features = ["derive"] }
 tracing = "0.1"
 tracing-subscriber = { version = "0.3.3", features = ["env-filter"] }

ruby/ql/consistency-queries/AstConsistency.ql

@@ -23,3 +23,8 @@ query predicate multipleParents(AstNode node, AstNode parent, string cls) {
     one != two
   )
 }
+
+query predicate multipleToString(AstNode n, string s) {
+  s = strictconcat(n.toString(), ",") and
+  strictcount(n.toString()) > 1
+}

ruby/ql/consistency-queries/CfgConsistency.ql

@@ -1,5 +1,6 @@
 import codeql.ruby.controlflow.internal.ControlFlowGraphImplShared::Consistency
 import codeql.ruby.AST
+import codeql.ruby.CFG
 import codeql.ruby.controlflow.internal.Completion
 import codeql.ruby.controlflow.internal.ControlFlowGraphImpl
 
@@ -18,3 +19,8 @@ query predicate nonPostOrderExpr(Expr e, string cls) {
     c instanceof NormalCompletion
   )
 }
+
+query predicate multipleToString(CfgNode n, string s) {
+  s = strictconcat(n.toString(), ",") and
+  strictcount(n.toString()) > 1
+}

ruby/ql/consistency-queries/DataFlowConsistency.ql

@@ -33,3 +33,8 @@ private class MyConsistencyConfiguration extends ConsistencyConfiguration {
     )
   }
 }
+
+query predicate multipleToString(Node n, string s) {
+  s = strictconcat(n.toString(), ",") and
+  strictcount(n.toString()) > 1
+}

ruby/ql/lib/CHANGELOG.md

@@ -1,3 +1,15 @@
+## 0.6.3
+
+### Minor Analysis Improvements
+
+* Deleted many deprecated predicates and classes with uppercase `URL`, `XSS`, etc. in their names. Use the PascalCased versions instead.
+* Deleted the deprecated `getValueText` predicate from the `Expr`, `StringComponent`, and `ExprCfgNode` classes. Use `getConstantValue` instead.
+* Deleted the deprecated `VariableReferencePattern` class, use `ReferencePattern` instead.
+* Deleted all deprecated aliases in `StandardLibrary.qll`, use `codeql.ruby.frameworks.Core` and `codeql.ruby.frameworks.Stdlib` instead.
+* Support for the `sequel` gem has been added. Method calls that execute queries against a database that may be vulnerable to injection attacks will now be recognized.
+* Support for the `mysql2` gem has been added. Method calls that execute queries against an MySQL database that may be vulnerable to injection attacks will now be recognized.
+* Support for the `pg` gem has been added. Method calls that execute queries against a PostgreSQL database that may be vulnerable to injection attacks will now be recognized.
+
 ## 0.6.2
 
 ### Minor Analysis Improvements

ruby/ql/lib/change-notes/2023-05-06-mysql2.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* Support for the `mysql2` gem has been added. Method calls that execute queries against an MySQL database that may be vulnerable to injection attacks will now be recognized.

ruby/ql/lib/change-notes/2023-05-06-pg.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* Support for the `pg` gem has been added. Method calls that execute queries against a PostgreSQL database that may be vulnerable to injection attacks will now be recognized.

ruby/ql/lib/change-notes/2023-05-07-sequel.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* Support for the `sequel` gem has been added. Method calls that execute queries against a database that may be vulnerable to injection attacks will now be recognized.

ruby/ql/lib/change-notes/2023-06-02-delete-deps.md

@@ -1,7 +0,0 @@
----
-category: minorAnalysis
----
-* Deleted many deprecated predicates and classes with uppercase `URL`, `XSS`, etc. in their names. Use the PascalCased versions instead.
-* Deleted the deprecated `getValueText` predicate from the `Expr`, `StringComponent`, and `ExprCfgNode` classes. Use `getConstantValue` instead.
-* Deleted the deprecated `VariableReferencePattern` class, use `ReferencePattern` instead.
-* Deleted all deprecated aliases in `StandardLibrary.qll`, use `codeql.ruby.frameworks.Core` and `codeql.ruby.frameworks.Stdlib` instead.

ruby/ql/lib/change-notes/2023-06-08-rack-redirect.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* HTTP redirect responses from Rack applications are now recognized as a potential sink for open redirect alerts.

ruby/ql/lib/change-notes/2023-06-14-insecure-download-config.md

@@ -0,0 +1,4 @@
+---
+category: deprecated
+---
+* The `Configuration` taint flow configuration class from `codeql.ruby.security.InsecureDownloadQuery` has been deprecated. Use the `Flow` module instead.

ruby/ql/lib/change-notes/released/0.6.3.md

@@ -0,0 +1,11 @@
+## 0.6.3
+
+### Minor Analysis Improvements
+
+* Deleted many deprecated predicates and classes with uppercase `URL`, `XSS`, etc. in their names. Use the PascalCased versions instead.
+* Deleted the deprecated `getValueText` predicate from the `Expr`, `StringComponent`, and `ExprCfgNode` classes. Use `getConstantValue` instead.
+* Deleted the deprecated `VariableReferencePattern` class, use `ReferencePattern` instead.
+* Deleted all deprecated aliases in `StandardLibrary.qll`, use `codeql.ruby.frameworks.Core` and `codeql.ruby.frameworks.Stdlib` instead.
+* Support for the `sequel` gem has been added. Method calls that execute queries against a database that may be vulnerable to injection attacks will now be recognized.
+* Support for the `mysql2` gem has been added. Method calls that execute queries against an MySQL database that may be vulnerable to injection attacks will now be recognized.
+* Support for the `pg` gem has been added. Method calls that execute queries against a PostgreSQL database that may be vulnerable to injection attacks will now be recognized.

ruby/ql/lib/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.6.2
+lastReleaseVersion: 0.6.3

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl.qll

@@ -2021,7 +2021,8 @@ module Impl<FullStateConfigSig Config> {
       FlowCheckNode() {
         castNode(this.asNode()) or
         clearsContentCached(this.asNode(), _) or
-        expectsContentCached(this.asNode(), _)
+        expectsContentCached(this.asNode(), _) or
+        neverSkipInPathGraph(this.asNode())
       }
     }
 

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll

@@ -1290,10 +1290,16 @@ private import PostUpdateNodes
 
 /** A node that performs a type cast. */
 class CastNode extends Node {
-  CastNode() {
-    // ensure that all variable assignments are included in the path graph
-    this.(SsaDefinitionExtNode).getDefinitionExt() instanceof Ssa::WriteDefinition
-  }
+  CastNode() { none() }
+}
+
+/**
+ * Holds if `n` should never be skipped over in the `PathGraph` and in path
+ * explanations.
+ */
+predicate neverSkipInPathGraph(Node n) {
+  // ensure that all variable assignments are included in the path graph
+  n.(SsaDefinitionExtNode).getDefinitionExt() instanceof Ssa::WriteDefinition
 }
 
 class DataFlowExpr = CfgNodes::ExprCfgNode;

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll

@@ -1284,13 +1284,16 @@ class HashLiteralNode extends LocalSourceNode, ExprNode {
  * into calls to `Array.[]`, so this includes both desugared calls as well as
  * explicit calls.
  */
-class ArrayLiteralNode extends LocalSourceNode, ExprNode {
+class ArrayLiteralNode extends LocalSourceNode, CallNode {
   ArrayLiteralNode() { super.getExprNode() instanceof CfgNodes::ExprNodes::ArrayLiteralCfgNode }
 
   /**
    * Gets an element of the array.
    */
-  Node getAnElement() { result = this.(CallNode).getPositionalArgument(_) }
+  Node getAnElement() { result = this.getElement(_) }
+
+  /** Gets the `i`th element of the array. */
+  Node getElement(int i) { result = this.getPositionalArgument(i) }
 }
 
 /**

ruby/ql/lib/codeql/ruby/frameworks/Rack.qll

@@ -2,47 +2,13 @@
  * Provides modeling for the Rack library.
  */
 
-private import codeql.ruby.controlflow.CfgNodes::ExprNodes
-private import codeql.ruby.DataFlow
-private import codeql.ruby.typetracking.TypeTracker
-
 /**
  * Provides modeling for the Rack library.
  */
 module Rack {
-  /**
-   * A class that may be a rack application.
-   * This is a class that has a `call` method that takes a single argument
-   * (traditionally called `env`) and returns a rack-compatible response.
-   */
-  class AppCandidate extends DataFlow::ClassNode {
-    private DataFlow::MethodNode call;
+  import rack.internal.App
+  import rack.internal.Response::Public as Response
 
-    AppCandidate() {
-      call = this.getInstanceMethod("call") and
-      call.getNumberOfParameters() = 1 and
-      call.getAReturnNode() = trackRackResponse()
-    }
-
-    /**
-     * Gets the environment of the request, which is the lone parameter to the `call` method.
-     */
-    DataFlow::ParameterNode getEnv() { result = call.getParameter(0) }
-  }
-
-  private predicate isRackResponse(DataFlow::Node r) {
-    // [status, headers, body]
-    r.asExpr().(ArrayLiteralCfgNode).getNumberOfArguments() = 3
-  }
-
-  private DataFlow::LocalSourceNode trackRackResponse(TypeTracker t) {
-    t.start() and
-    isRackResponse(result)
-    or
-    exists(TypeTracker t2 | result = trackRackResponse(t2).track(t2, t))
-  }
-
-  private DataFlow::Node trackRackResponse() {
-    trackRackResponse(TypeTracker::end()).flowsTo(result)
-  }
+  /** DEPRECATED: Alias for App::AppCandidate */
+  deprecated class AppCandidate = App::AppCandidate;
 }

ruby/ql/lib/codeql/ruby/frameworks/Sinatra.qll

@@ -105,15 +105,25 @@ module Sinatra {
    * Gets the template file referred to by `erbCall`.
    * This works on the AST level to avoid non-monotonic reecursion in `ErbLocalsHashSyntheticGlobal`.
    */
+  pragma[nomagic]
   private ErbFile getTemplateFile(MethodCall erbCall) {
     erbCall.getMethodName() = "erb" and
     result.getTemplateName() = erbCall.getArgument(0).getConstantValue().getStringlikeValue() and
     result.getRelativePath().matches("%views/%")
   }
 
+  pragma[nomagic]
+  private predicate erbCallAtLocation(MethodCall erbCall, ErbFile erbFile, Location l) {
+    erbCall.getMethodName() = "erb" and
+    erbFile = getTemplateFile(erbCall) and
+    l = erbCall.getLocation()
+  }
+
   /**
    * Like `Location.toString`, but displays the relative path rather than the full path.
    */
+  bindingset[loc]
+  pragma[inline_late]
   private string locationRelativePathToString(Location loc) {
     result =
       loc.getFile().getRelativePath() + "@" + loc.getStartLine() + ":" + loc.getStartColumn() + ":" +
@@ -121,7 +131,7 @@ module Sinatra {
   }
 
   /**
-   *  A synthetic global representing the hash of local variables passed to an ERB template.
+   * A synthetic global representing the hash of local variables passed to an ERB template.
    */
   class ErbLocalsHashSyntheticGlobal extends SummaryComponent::SyntheticGlobal {
     private string id;
@@ -129,10 +139,11 @@ module Sinatra {
     private ErbFile erbFile;
 
     ErbLocalsHashSyntheticGlobal() {
-      this = "SinatraErbLocalsHash(" + id + ")" and
-      id = erbFile.getRelativePath() + "," + locationRelativePathToString(erbCall.getLocation()) and
-      erbCall.getMethodName() = "erb" and
-      erbFile = getTemplateFile(erbCall)
+      exists(Location l |
+        erbCallAtLocation(erbCall, erbFile, l) and
+        id = erbFile.getRelativePath() + "," + locationRelativePathToString(l) and
+        this = "SinatraErbLocalsHash(" + id + ")"
+      )
     }
 
     /**

ruby/ql/lib/codeql/ruby/frameworks/actiondispatch/internal/Request.qll

@@ -128,7 +128,7 @@ module Request {
     private import codeql.ruby.frameworks.Rack
 
     private class RackEnv extends Env {
-      RackEnv() { this = any(Rack::AppCandidate app).getEnv().getALocalUse() }
+      RackEnv() { this = any(Rack::App::AppCandidate app).getEnv().getALocalUse() }
     }
 
     /**

ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll

@@ -662,6 +662,17 @@ module ModelOutput {
 
   import Cached
   import Specific::ModelOutputSpecific
+  private import codeql.mad.ModelValidation as SharedModelVal
+
+  private module KindValConfig implements SharedModelVal::KindValidationConfigSig {
+    predicate summaryKind(string kind) { summaryModel(_, _, _, _, kind) }
+
+    predicate sinkKind(string kind) { sinkModel(_, _, kind) }
+
+    predicate sourceKind(string kind) { sourceModel(_, _, kind) }
+  }
+
+  private module KindVal = SharedModelVal::KindValidation<KindValConfig>;
 
   /**
    * Gets an error message relating to an invalid CSV row in a model.
@@ -707,5 +718,8 @@ module ModelOutput {
       not isValidNoArgumentTokenInIdentifyingAccessPath(token.getName()) and
       result = "Invalid token '" + token + "' is missing its arguments, in access path: " + path
     )
+    or
+    // Check for invalid model kinds
+    result = KindVal::getInvalidModelKind()
   }
 }

ruby/ql/lib/codeql/ruby/frameworks/rack/internal/App.qll

@@ -0,0 +1,53 @@
+/**
+ * Provides modeling for Rack applications.
+ */
+
+private import codeql.ruby.ApiGraphs
+private import codeql.ruby.DataFlow
+private import codeql.ruby.typetracking.TypeTracker
+private import Response::Private as RP
+
+/** A method node for a method named `call`. */
+private class CallMethodNode extends DataFlow::MethodNode {
+  CallMethodNode() { this.getMethodName() = "call" }
+}
+
+private DataFlow::LocalSourceNode trackRackResponse(TypeBackTracker t, CallMethodNode call) {
+  t.start() and
+  result = call.getAReturnNode().getALocalSource()
+  or
+  exists(TypeBackTracker t2 | result = trackRackResponse(t2, call).backtrack(t2, t))
+}
+
+private RP::PotentialResponseNode trackRackResponse(CallMethodNode call) {
+  result = trackRackResponse(TypeBackTracker::end(), call)
+}
+
+/**
+ * Provides modeling for Rack applications.
+ */
+module App {
+  /**
+   * A class that may be a rack application.
+   * This is a class that has a `call` method that takes a single argument
+   * (traditionally called `env`) and returns a rack-compatible response.
+   */
+  class AppCandidate extends DataFlow::ClassNode {
+    private CallMethodNode call;
+    private RP::PotentialResponseNode resp;
+
+    AppCandidate() {
+      call = this.getInstanceMethod("call") and
+      call.getNumberOfParameters() = 1 and
+      resp = trackRackResponse(call)
+    }
+
+    /**
+     * Gets the environment of the request, which is the lone parameter to the `call` method.
+     */
+    DataFlow::ParameterNode getEnv() { result = call.getParameter(0) }
+
+    /** Gets the response returned from a request to this application. */
+    RP::PotentialResponseNode getResponse() { result = resp }
+  }
+}

ruby/ql/lib/codeql/ruby/frameworks/rack/internal/Response.qll

@@ -0,0 +1,82 @@
+/**
+ * Provides modeling for the `Response` component of the `Rack` library.
+ */
+
+private import codeql.ruby.AST
+private import codeql.ruby.ApiGraphs
+private import codeql.ruby.Concepts
+private import codeql.ruby.controlflow.CfgNodes::ExprNodes
+private import codeql.ruby.DataFlow
+private import codeql.ruby.typetracking.TypeTracker
+private import App as A
+
+/** Contains implementation details for modeling `Rack::Response`. */
+module Private {
+  /** A `DataFlow::Node` that may be a rack response. This is detected heuristically, if something "looks like" a rack response syntactically then we consider it to be a potential response node. */
+  class PotentialResponseNode extends DataFlow::ArrayLiteralNode {
+    // [status, headers, body]
+    PotentialResponseNode() { this.getNumberOfArguments() = 3 }
+
+    /** Gets the headers returned with this response. */
+    DataFlow::Node getHeaders() { result = this.getElement(1) }
+
+    /** Gets the body of this response. */
+    DataFlow::Node getBody() { result = this.getElement(2) }
+  }
+}
+
+/**
+ * Provides modeling for the `Response` component of the `Rack` library.
+ */
+module Public {
+  bindingset[headerName]
+  private DataFlow::Node getHeaderValue(ResponseNode resp, string headerName) {
+    exists(DataFlow::Node headers | headers = resp.getHeaders() |
+      // set via `headers.<header_name>=`
+      exists(
+        DataFlow::CallNode contentTypeAssignment, Assignment assignment,
+        DataFlow::PostUpdateNode postUpdateHeaders
+      |
+        contentTypeAssignment.getMethodName() = headerName.replaceAll("-", "_").toLowerCase() + "=" and
+        assignment =
+          contentTypeAssignment.getArgument(0).(DataFlow::OperationNode).asOperationAstNode() and
+        postUpdateHeaders.(DataFlow::LocalSourceNode).flowsTo(headers) and
+        postUpdateHeaders.getPreUpdateNode() = contentTypeAssignment.getReceiver()
+      |
+        result.asExpr().getExpr() = assignment.getRightOperand()
+      )
+      or
+      // set within a hash
+      exists(DataFlow::HashLiteralNode headersHash | headersHash.flowsTo(headers) |
+        result =
+          headersHash
+              .getElementFromKey(any(ConstantValue v |
+                  v.getStringlikeValue().toLowerCase() = headerName.toLowerCase()
+                ))
+      )
+    )
+  }
+
+  /** A `DataFlow::Node` returned from a rack request. */
+  class ResponseNode extends Private::PotentialResponseNode, Http::Server::HttpResponse::Range {
+    ResponseNode() { this = any(A::App::AppCandidate app).getResponse() }
+
+    override DataFlow::Node getBody() { result = this.getElement(2) }
+
+    override DataFlow::Node getMimetypeOrContentTypeArg() {
+      result = getHeaderValue(this, "content-type")
+    }
+
+    // TODO: is there a sensible value for this?
+    override string getMimetypeDefault() { none() }
+  }
+
+  /** A `DataFlow::Node` returned from a rack request that has a redirect HTTP status code. */
+  class RedirectResponse extends ResponseNode, Http::Server::HttpRedirectResponse::Range {
+    private DataFlow::Node redirectLocation;
+
+    RedirectResponse() { redirectLocation = getHeaderValue(this, "location") }
+
+    override DataFlow::Node getRedirectLocation() { result = redirectLocation }
+  }
+}

ruby/ql/lib/codeql/ruby/regexp/internal/ParseRegExp.qll

@@ -195,8 +195,8 @@ abstract class RegExp extends Ast::StringlikeLiteral {
 
   /**
    * Holds if the character set starting at `charset_start` contains a character range
-   * with lower bound found between `start` and `lower_end`
-   * and upper bound found between `upper_start` and `end`.
+   * with lower bound found between `start` and `lowerEnd`
+   * and upper bound found between `upperStart` and `end`.
    */
   predicate charRange(int charsetStart, int start, int lowerEnd, int upperStart, int end) {
     exists(int index |
@@ -844,11 +844,11 @@ abstract class RegExp extends Ast::StringlikeLiteral {
   }
 
   /**
-   * Holds if a qualified part is found between `start` and `part_end` and the qualifier is
-   * found between `part_end` and `end`.
+   * Holds if a qualified part is found between `start` and `partEnd` and the qualifier is
+   * found between `partEnd` and `end`.
    *
-   * `maybe_empty` is true if the part is optional.
-   * `may_repeat_forever` is true if the part may be repeated unboundedly.
+   * `maybeEmpty` is true if the part is optional.
+   * `mayRepeatForever` is true if the part may be repeated unboundedly.
    */
   predicate qualifiedPart(
     int start, int partEnd, int end, boolean maybeEmpty, boolean mayRepeatForever

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

@@ -13,7 +13,7 @@ import InsecureDownloadCustomizations::InsecureDownload
 /**
  * A taint tracking configuration for download of sensitive file through insecure connection.
  */
-class Configuration extends DataFlow::Configuration {
+deprecated class Configuration extends DataFlow::Configuration {
   Configuration() { this = "InsecureDownload" }
 
   override predicate isSource(DataFlow::Node source, DataFlow::FlowState label) {
@@ -29,3 +29,30 @@ class Configuration extends DataFlow::Configuration {
     node instanceof Sanitizer
   }
 }
+
+/**
+ * A taint tracking configuration for download of sensitive file through insecure connection.
+ */
+module Config implements DataFlow::StateConfigSig {
+  class FlowState = string;
+
+  predicate isSource(DataFlow::Node source, DataFlow::FlowState label) {
+    source.(Source).getALabel() = label
+  }
+
+  predicate isSink(DataFlow::Node sink, DataFlow::FlowState label) {
+    sink.(Sink).getALabel() = label
+  }
+
+  predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
+
+  predicate isBarrier(DataFlow::Node node, FlowState state) { none() }
+
+  predicate isAdditionalFlowStep(
+    DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
+  ) {
+    none()
+  }
+}
+
+module Flow = DataFlow::GlobalWithState<Config>;

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

@@ -15,7 +15,7 @@ class Configuration extends TaintTracking::Configuration {
 
   override predicate isSource(DataFlow::Node source) { source instanceof Source }
 
-  override predicate isSink(DataFlow::Node source) { source instanceof Sink }
+  override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
 
   override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
 }

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

@@ -15,7 +15,7 @@ class Configuration extends TaintTracking::Configuration {
 
   override predicate isSource(DataFlow::Node source) { source instanceof Source }
 
-  override predicate isSink(DataFlow::Node source) { source instanceof Sink }
+  override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
 
   override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
 }

ruby/ql/lib/codeql/ruby/typetracking/TypeTrackerSpecific.qll

@@ -112,15 +112,7 @@ predicate levelStepCall(Node nodeFrom, Node nodeTo) {
 /** Holds if there is a level step from `nodeFrom` to `nodeTo`, which does not depend on the call graph. */
 pragma[nomagic]
 predicate levelStepNoCall(Node nodeFrom, Node nodeTo) {
-  exists(
-    SummarizedCallable callable, DataFlowPublic::CallNode call, SummaryComponentStack input,
-    SummaryComponentStack output
-  |
-    callable.propagatesFlow(input, output, true) and
-    call.asExpr().getExpr() = callable.getACallSimple() and
-    nodeFrom = evaluateSummaryComponentStackLocal(callable, call, input) and
-    nodeTo = evaluateSummaryComponentStackLocal(callable, call, output)
-  )
+  TypeTrackerSummaryFlow::levelStepNoCall(nodeFrom, nodeTo)
   or
   localFieldStep(nodeFrom, nodeTo)
 }
@@ -276,16 +268,7 @@ predicate returnStep(Node nodeFrom, Node nodeTo) {
 predicate basicStoreStep(Node nodeFrom, Node nodeTo, DataFlow::ContentSet contents) {
   storeStepIntoSourceNode(nodeFrom, nodeTo, contents)
   or
-  exists(
-    SummarizedCallable callable, DataFlowPublic::CallNode call, SummaryComponentStack input,
-    SummaryComponentStack output
-  |
-    hasStoreSummary(callable, contents, pragma[only_bind_into](input),
-      pragma[only_bind_into](output)) and
-    call.asExpr().getExpr() = callable.getACallSimple() and
-    nodeFrom = evaluateSummaryComponentStackLocal(callable, call, input) and
-    nodeTo = evaluateSummaryComponentStackLocal(callable, call, output)
-  )
+  TypeTrackerSummaryFlow::basicStoreStep(nodeFrom, nodeTo, contents)
 }
 
 /**
@@ -319,15 +302,7 @@ predicate basicLoadStep(Node nodeFrom, Node nodeTo, DataFlow::ContentSet content
     nodeTo.asExpr() = call
   )
   or
-  exists(
-    SummarizedCallable callable, DataFlowPublic::CallNode call, SummaryComponentStack input,
-    SummaryComponentStack output
-  |
-    hasLoadSummary(callable, contents, pragma[only_bind_into](input), pragma[only_bind_into](output)) and
-    call.asExpr().getExpr() = callable.getACallSimple() and
-    nodeFrom = evaluateSummaryComponentStackLocal(callable, call, input) and
-    nodeTo = evaluateSummaryComponentStackLocal(callable, call, output)
-  )
+  TypeTrackerSummaryFlow::basicLoadStep(nodeFrom, nodeTo, contents)
 }
 
 /**
@@ -336,48 +311,21 @@ predicate basicLoadStep(Node nodeFrom, Node nodeTo, DataFlow::ContentSet content
 predicate basicLoadStoreStep(
   Node nodeFrom, Node nodeTo, DataFlow::ContentSet loadContent, DataFlow::ContentSet storeContent
 ) {
-  exists(
-    SummarizedCallable callable, DataFlowPublic::CallNode call, SummaryComponentStack input,
-    SummaryComponentStack output
-  |
-    hasLoadStoreSummary(callable, loadContent, storeContent, pragma[only_bind_into](input),
-      pragma[only_bind_into](output)) and
-    call.asExpr().getExpr() = callable.getACallSimple() and
-    nodeFrom = evaluateSummaryComponentStackLocal(callable, call, input) and
-    nodeTo = evaluateSummaryComponentStackLocal(callable, call, output)
-  )
+  TypeTrackerSummaryFlow::basicLoadStoreStep(nodeFrom, nodeTo, loadContent, storeContent)
 }
 
 /**
  * Holds if type-tracking should step from `nodeFrom` to `nodeTo` but block flow of contents matched by `filter` through here.
  */
 predicate basicWithoutContentStep(Node nodeFrom, Node nodeTo, ContentFilter filter) {
-  exists(
-    SummarizedCallable callable, DataFlowPublic::CallNode call, SummaryComponentStack input,
-    SummaryComponentStack output
-  |
-    hasWithoutContentSummary(callable, filter, pragma[only_bind_into](input),
-      pragma[only_bind_into](output)) and
-    call.asExpr().getExpr() = callable.getACallSimple() and
-    nodeFrom = evaluateSummaryComponentStackLocal(callable, call, input) and
-    nodeTo = evaluateSummaryComponentStackLocal(callable, call, output)
-  )
+  TypeTrackerSummaryFlow::basicWithoutContentStep(nodeFrom, nodeTo, filter)
 }
 
 /**
  * Holds if type-tracking should step from `nodeFrom` to `nodeTo` if inside a content matched by `filter`.
  */
 predicate basicWithContentStep(Node nodeFrom, Node nodeTo, ContentFilter filter) {
-  exists(
-    SummarizedCallable callable, DataFlowPublic::CallNode call, SummaryComponentStack input,
-    SummaryComponentStack output
-  |
-    hasWithContentSummary(callable, filter, pragma[only_bind_into](input),
-      pragma[only_bind_into](output)) and
-    call.asExpr().getExpr() = callable.getACallSimple() and
-    nodeFrom = evaluateSummaryComponentStackLocal(callable, call, input) and
-    nodeTo = evaluateSummaryComponentStackLocal(callable, call, output)
-  )
+  TypeTrackerSummaryFlow::basicWithContentStep(nodeFrom, nodeTo, filter)
 }
 
 /**
@@ -389,121 +337,6 @@ class Boolean extends boolean {
 
 private import SummaryComponentStack
 
-pragma[nomagic]
-private predicate hasStoreSummary(
-  SummarizedCallable callable, DataFlow::ContentSet contents, SummaryComponentStack input,
-  SummaryComponentStack output
-) {
-  not isNonLocal(input.head()) and
-  not isNonLocal(output.head()) and
-  (
-    callable.propagatesFlow(input, push(SummaryComponent::content(contents), output), true)
-    or
-    // Allow the input to start with an arbitrary WithoutContent[X].
-    // Since type-tracking only tracks one content deep, and we're about to store into another content,
-    // we're already preventing the input from being in a content.
-    callable
-        .propagatesFlow(push(SummaryComponent::withoutContent(_), input),
-          push(SummaryComponent::content(contents), output), true)
-  )
-}
-
-pragma[nomagic]
-private predicate hasLoadSummary(
-  SummarizedCallable callable, DataFlow::ContentSet contents, SummaryComponentStack input,
-  SummaryComponentStack output
-) {
-  callable.propagatesFlow(push(SummaryComponent::content(contents), input), output, true) and
-  not isNonLocal(input.head()) and
-  not isNonLocal(output.head())
-}
-
-pragma[nomagic]
-private predicate hasLoadStoreSummary(
-  SummarizedCallable callable, DataFlow::ContentSet loadContents,
-  DataFlow::ContentSet storeContents, SummaryComponentStack input, SummaryComponentStack output
-) {
-  callable
-      .propagatesFlow(push(SummaryComponent::content(loadContents), input),
-        push(SummaryComponent::content(storeContents), output), true) and
-  not isNonLocal(input.head()) and
-  not isNonLocal(output.head())
-}
-
-/**
- * Gets a content filter to use for a `WithoutContent[content]` step, or has no result if
- * the step should be treated as ordinary flow.
- *
- * `WithoutContent` is often used to perform strong updates on individual collection elements, but for
- * type-tracking this is rarely beneficial and quite expensive. However, `WithoutContent` can be quite useful
- * for restricting the type of an object, and in these cases we translate it to a filter.
- */
-private ContentFilter getFilterFromWithoutContentStep(DataFlow::ContentSet content) {
-  (
-    content.isAnyElement()
-    or
-    content.isElementLowerBoundOrUnknown(_)
-    or
-    content.isElementOfTypeOrUnknown(_)
-    or
-    content.isSingleton(any(DataFlow::Content::UnknownElementContent c))
-  ) and
-  result = MkElementFilter()
-}
-
-pragma[nomagic]
-private predicate hasWithoutContentSummary(
-  SummarizedCallable callable, ContentFilter filter, SummaryComponentStack input,
-  SummaryComponentStack output
-) {
-  exists(DataFlow::ContentSet content |
-    callable.propagatesFlow(push(SummaryComponent::withoutContent(content), input), output, true) and
-    filter = getFilterFromWithoutContentStep(content) and
-    not isNonLocal(input.head()) and
-    not isNonLocal(output.head()) and
-    input != output
-  )
-}
-
-/**
- * Gets a content filter to use for a `WithContent[content]` step, or has no result if
- * the step cannot be handled by type-tracking.
- *
- * `WithContent` is often used to perform strong updates on individual collection elements (or rather
- * to preserve those that didn't get updated). But for type-tracking this is rarely beneficial and quite expensive.
- * However, `WithContent` can be quite useful for restricting the type of an object, and in these cases we translate it to a filter.
- */
-private ContentFilter getFilterFromWithContentStep(DataFlow::ContentSet content) {
-  (
-    content.isAnyElement()
-    or
-    content.isElementLowerBound(_)
-    or
-    content.isElementLowerBoundOrUnknown(_)
-    or
-    content.isElementOfType(_)
-    or
-    content.isElementOfTypeOrUnknown(_)
-    or
-    content.isSingleton(any(DataFlow::Content::ElementContent c))
-  ) and
-  result = MkElementFilter()
-}
-
-pragma[nomagic]
-private predicate hasWithContentSummary(
-  SummarizedCallable callable, ContentFilter filter, SummaryComponentStack input,
-  SummaryComponentStack output
-) {
-  exists(DataFlow::ContentSet content |
-    callable.propagatesFlow(push(SummaryComponent::withContent(content), input), output, true) and
-    filter = getFilterFromWithContentStep(content) and
-    not isNonLocal(input.head()) and
-    not isNonLocal(output.head()) and
-    input != output
-  )
-}
-
 /**
  * Holds if the given component can't be evaluated by `evaluateSummaryComponentStackLocal`.
  */
@@ -514,101 +347,95 @@ predicate isNonLocal(SummaryComponent component) {
   component = SC::withContent(_)
 }
 
-/**
- * Gets a data flow node corresponding an argument or return value of `call`,
- * as specified by `component`.
- */
-bindingset[call, component]
-private DataFlow::Node evaluateSummaryComponentLocal(
-  DataFlow::CallNode call, SummaryComponent component
-) {
-  exists(DataFlowDispatch::ParameterPosition pos |
-    component = SummaryComponent::argument(pos) and
-    argumentPositionMatch(call.asExpr(), result, pos)
-  )
-  or
-  component = SummaryComponent::return() and
-  result = call
-}
+private import internal.SummaryTypeTracker as SummaryTypeTracker
+private import codeql.ruby.dataflow.FlowSummary as FlowSummary
 
-/**
- * Holds if `callable` is relevant for type-tracking and we therefore want `stack` to
- * be evaluated locally at its call sites.
- */
-pragma[nomagic]
-private predicate dependsOnSummaryComponentStack(
-  SummarizedCallable callable, SummaryComponentStack stack
-) {
-  exists(callable.getACallSimple()) and
-  (
-    callable.propagatesFlow(stack, _, true)
-    or
-    callable.propagatesFlow(_, stack, true)
-    or
-    // include store summaries as they may skip an initial step at the input
-    hasStoreSummary(callable, _, stack, _)
-  )
-  or
-  dependsOnSummaryComponentStackCons(callable, _, stack)
-}
+private module SummaryTypeTrackerInput implements SummaryTypeTracker::Input {
+  // Dataflow nodes
+  class Node = DataFlow::Node;
 
-pragma[nomagic]
-private predicate dependsOnSummaryComponentStackCons(
-  SummarizedCallable callable, SummaryComponent head, SummaryComponentStack tail
-) {
-  dependsOnSummaryComponentStack(callable, SCS::push(head, tail))
-}
+  // Content
+  class TypeTrackerContent = DataFlowPublic::ContentSet;
 
-pragma[nomagic]
-private predicate dependsOnSummaryComponentStackConsLocal(
-  SummarizedCallable callable, SummaryComponent head, SummaryComponentStack tail
-) {
-  dependsOnSummaryComponentStackCons(callable, head, tail) and
-  not isNonLocal(head)
-}
+  class TypeTrackerContentFilter = ContentFilter;
 
-pragma[nomagic]
-private predicate dependsOnSummaryComponentStackLeaf(
-  SummarizedCallable callable, SummaryComponent leaf
-) {
-  dependsOnSummaryComponentStack(callable, SCS::singleton(leaf))
-}
+  TypeTrackerContentFilter getFilterFromWithoutContentStep(TypeTrackerContent content) {
+    (
+      content.isAnyElement()
+      or
+      content.isElementLowerBoundOrUnknown(_)
+      or
+      content.isElementOfTypeOrUnknown(_)
+      or
+      content.isSingleton(any(DataFlow::Content::UnknownElementContent c))
+    ) and
+    result = MkElementFilter()
+  }
 
-/**
- * Gets a data flow node corresponding to the local input or output of `call`
- * identified by `stack`, if possible.
- */
-pragma[nomagic]
-private DataFlow::Node evaluateSummaryComponentStackLocal(
-  SummarizedCallable callable, DataFlow::CallNode call, SummaryComponentStack stack
-) {
-  exists(SummaryComponent component |
-    dependsOnSummaryComponentStackLeaf(callable, component) and
-    stack = SCS::singleton(component) and
-    call.asExpr().getExpr() = callable.getACallSimple() and
-    result = evaluateSummaryComponentLocal(call, component)
-  )
-  or
-  exists(DataFlow::Node prev, SummaryComponent head, SummaryComponentStack tail |
-    prev = evaluateSummaryComponentStackLocal(callable, call, tail) and
-    dependsOnSummaryComponentStackConsLocal(callable, pragma[only_bind_into](head),
-      pragma[only_bind_out](tail)) and
-    stack = SCS::push(pragma[only_bind_out](head), pragma[only_bind_out](tail))
-  |
+  TypeTrackerContentFilter getFilterFromWithContentStep(TypeTrackerContent content) {
+    (
+      content.isAnyElement()
+      or
+      content.isElementLowerBound(_)
+      or
+      content.isElementLowerBoundOrUnknown(_)
+      or
+      content.isElementOfType(_)
+      or
+      content.isElementOfTypeOrUnknown(_)
+      or
+      content.isSingleton(any(DataFlow::Content::ElementContent c))
+    ) and
+    result = MkElementFilter()
+  }
+
+  // Summaries and their stacks
+  class SummaryComponent = FlowSummary::SummaryComponent;
+
+  class SummaryComponentStack = FlowSummary::SummaryComponentStack;
+
+  predicate singleton = FlowSummary::SummaryComponentStack::singleton/1;
+
+  predicate push = FlowSummary::SummaryComponentStack::push/2;
+
+  // Relating content to summaries
+  predicate content = FlowSummary::SummaryComponent::content/1;
+
+  predicate withoutContent = FlowSummary::SummaryComponent::withoutContent/1;
+
+  predicate withContent = FlowSummary::SummaryComponent::withContent/1;
+
+  predicate return = FlowSummary::SummaryComponent::return/0;
+
+  // Callables
+  class SummarizedCallable = FlowSummary::SummarizedCallable;
+
+  // Relating nodes to summaries
+  Node argumentOf(Node call, SummaryComponent arg) {
+    exists(DataFlowDispatch::ParameterPosition pos |
+      arg = SummaryComponent::argument(pos) and
+      argumentPositionMatch(call.asExpr(), result, pos)
+    )
+  }
+
+  Node parameterOf(Node callable, SummaryComponent param) {
     exists(DataFlowDispatch::ArgumentPosition apos, DataFlowDispatch::ParameterPosition ppos |
-      head = SummaryComponent::parameter(apos) and
+      param = SummaryComponent::parameter(apos) and
       DataFlowDispatch::parameterMatch(ppos, apos) and
-      result.(DataFlowPrivate::ParameterNodeImpl).isSourceParameterOf(prev.asExpr().getExpr(), ppos)
+      result
+          .(DataFlowPrivate::ParameterNodeImpl)
+          .isSourceParameterOf(callable.asExpr().getExpr(), ppos)
     )
-    or
-    head = SummaryComponent::return() and
+  }
+
+  Node returnOf(Node callable, SummaryComponent return) {
+    return = SummaryComponent::return() and
     result.(DataFlowPrivate::ReturnNode).(DataFlowPrivate::NodeImpl).getCfgScope() =
-      prev.asExpr().getExpr()
-    or
-    exists(DataFlow::ContentSet content |
-      head = SummaryComponent::withoutContent(content) and
-      not exists(getFilterFromWithoutContentStep(content)) and
-      result = prev
-    )
-  )
+      callable.asExpr().getExpr()
+  }
+
+  // Relating callables to nodes
+  Node callTo(SummarizedCallable callable) { result.asExpr().getExpr() = callable.getACallSimple() }
 }
+
+private module TypeTrackerSummaryFlow = SummaryTypeTracker::SummaryFlow<SummaryTypeTrackerInput>;

ruby/ql/lib/codeql/ruby/typetracking/internal/SummaryTypeTracker.qll

@@ -0,0 +1,391 @@
+/**
+ * Provides the implementation of type tracking steps through flow summaries.
+ * To use this, you must implement the `Input` signature. You can then use the predicates in the `Output`
+ * signature to implement the predicates of the same names inside `TypeTrackerSpecific.qll`.
+ */
+
+/** The classes and predicates needed to generate type-tracking steps from summaries. */
+signature module Input {
+  // Dataflow nodes
+  class Node;
+
+  // Content
+  class TypeTrackerContent;
+
+  class TypeTrackerContentFilter;
+
+  // Relating content and filters
+  /**
+   * Gets a content filter to use for a `WithoutContent[content]` step, (data is not allowed to be stored in `content`)
+   * or has no result if
+   * the step should be treated as ordinary flow.
+   *
+   * `WithoutContent` is often used to perform strong updates on individual collection elements, but for
+   * type-tracking this is rarely beneficial and quite expensive. However, `WithoutContent` can be quite useful
+   * for restricting the type of an object, and in these cases we translate it to a filter.
+   */
+  TypeTrackerContentFilter getFilterFromWithoutContentStep(TypeTrackerContent content);
+
+  /**
+   * Gets a content filter to use for a `WithContent[content]` step, (data must be stored in `content`)
+   * or has no result if
+   * the step cannot be handled by type-tracking.
+   *
+   * `WithContent` is often used to perform strong updates on individual collection elements (or rather
+   * to preserve those that didn't get updated). But for type-tracking this is rarely beneficial and quite expensive.
+   * However, `WithContent` can be quite useful for restricting the type of an object, and in these cases we translate it to a filter.
+   */
+  TypeTrackerContentFilter getFilterFromWithContentStep(TypeTrackerContent content);
+
+  // Summaries and their stacks
+  class SummaryComponent;
+
+  class SummaryComponentStack {
+    SummaryComponent head();
+  }
+
+  /** Gets a singleton stack containing `component`. */
+  SummaryComponentStack singleton(SummaryComponent component);
+
+  /**
+   * Gets the stack obtained by pushing `head` onto `tail`.
+   */
+  SummaryComponentStack push(SummaryComponent head, SummaryComponentStack tail);
+
+  /** Gets a singleton stack representing a return. */
+  SummaryComponent return();
+
+  // Relating content to summaries
+  /** Gets a summary component for content `c`. */
+  SummaryComponent content(TypeTrackerContent contents);
+
+  /** Gets a summary component where data is not allowed to be stored in `contents`. */
+  SummaryComponent withoutContent(TypeTrackerContent contents);
+
+  /** Gets a summary component where data must be stored in `contents`. */
+  SummaryComponent withContent(TypeTrackerContent contents);
+
+  // Callables
+  class SummarizedCallable {
+    predicate propagatesFlow(
+      SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
+    );
+  }
+
+  // Relating nodes to summaries
+  /** Gets a dataflow node respresenting the argument of `call` indicated by `arg`. */
+  Node argumentOf(Node call, SummaryComponent arg);
+
+  /** Gets a dataflow node respresenting the parameter of `callable` indicated by `param`. */
+  Node parameterOf(Node callable, SummaryComponent param);
+
+  /** Gets a dataflow node respresenting the return of `callable` indicated by `return`. */
+  Node returnOf(Node callable, SummaryComponent return);
+
+  // Relating callables to nodes
+  /** Gets a dataflow node respresenting a call to `callable`. */
+  Node callTo(SummarizedCallable callable);
+}
+
+/**
+ * The predicates provided by a summary type tracker.
+ * These are meant to be used in `TypeTrackerSpecific.qll`
+ * inside the predicates of the same names.
+ */
+signature module Output<Input I> {
+  /**
+   *  Holds if there is a level step from `nodeFrom` to `nodeTo`, which does not depend on the call graph.
+   */
+  predicate levelStepNoCall(I::Node nodeFrom, I::Node nodeTo);
+
+  /**
+   * Holds if `nodeTo` is the result of accessing the `content` content of `nodeFrom`.
+   */
+  predicate basicLoadStep(I::Node nodeFrom, I::Node nodeTo, I::TypeTrackerContent content);
+
+  /**
+   * Holds if `nodeFrom` is being written to the `content` content of the object in `nodeTo`.
+   */
+  predicate basicStoreStep(I::Node nodeFrom, I::Node nodeTo, I::TypeTrackerContent content);
+
+  /**
+   * Holds if the `loadContent` of `nodeFrom` is stored in the `storeContent` of `nodeTo`.
+   */
+  predicate basicLoadStoreStep(
+    I::Node nodeFrom, I::Node nodeTo, I::TypeTrackerContent loadContent,
+    I::TypeTrackerContent storeContent
+  );
+
+  /**
+   * Holds if type-tracking should step from `nodeFrom` to `nodeTo` but block flow of contents matched by `filter` through here.
+   */
+  predicate basicWithoutContentStep(
+    I::Node nodeFrom, I::Node nodeTo, I::TypeTrackerContentFilter filter
+  );
+
+  /**
+   * Holds if type-tracking should step from `nodeFrom` to `nodeTo` if inside a content matched by `filter`.
+   */
+  predicate basicWithContentStep(
+    I::Node nodeFrom, I::Node nodeTo, I::TypeTrackerContentFilter filter
+  );
+}
+
+/**
+ * Implementation of the summary type tracker, that is type tracking through flow summaries.
+ */
+module SummaryFlow<Input I> implements Output<I> {
+  pragma[nomagic]
+  private predicate isNonLocal(I::SummaryComponent component) {
+    component = I::content(_)
+    or
+    component = I::withContent(_)
+  }
+
+  pragma[nomagic]
+  private predicate hasLoadSummary(
+    I::SummarizedCallable callable, I::TypeTrackerContent contents, I::SummaryComponentStack input,
+    I::SummaryComponentStack output
+  ) {
+    callable.propagatesFlow(I::push(I::content(contents), input), output, true) and
+    not isNonLocal(input.head()) and
+    not isNonLocal(output.head())
+  }
+
+  pragma[nomagic]
+  private predicate hasStoreSummary(
+    I::SummarizedCallable callable, I::TypeTrackerContent contents, I::SummaryComponentStack input,
+    I::SummaryComponentStack output
+  ) {
+    not isNonLocal(input.head()) and
+    not isNonLocal(output.head()) and
+    (
+      callable.propagatesFlow(input, I::push(I::content(contents), output), true)
+      or
+      // Allow the input to start with an arbitrary WithoutContent[X].
+      // Since type-tracking only tracks one content deep, and we're about to store into another content,
+      // we're already preventing the input from being in a content.
+      callable
+          .propagatesFlow(I::push(I::withoutContent(_), input),
+            I::push(I::content(contents), output), true)
+    )
+  }
+
+  pragma[nomagic]
+  private predicate hasLoadStoreSummary(
+    I::SummarizedCallable callable, I::TypeTrackerContent loadContents,
+    I::TypeTrackerContent storeContents, I::SummaryComponentStack input,
+    I::SummaryComponentStack output
+  ) {
+    callable
+        .propagatesFlow(I::push(I::content(loadContents), input),
+          I::push(I::content(storeContents), output), true) and
+    not isNonLocal(input.head()) and
+    not isNonLocal(output.head())
+  }
+
+  pragma[nomagic]
+  private predicate hasWithoutContentSummary(
+    I::SummarizedCallable callable, I::TypeTrackerContentFilter filter,
+    I::SummaryComponentStack input, I::SummaryComponentStack output
+  ) {
+    exists(I::TypeTrackerContent content |
+      callable.propagatesFlow(I::push(I::withoutContent(content), input), output, true) and
+      filter = I::getFilterFromWithoutContentStep(content) and
+      not isNonLocal(input.head()) and
+      not isNonLocal(output.head()) and
+      input != output
+    )
+  }
+
+  pragma[nomagic]
+  private predicate hasWithContentSummary(
+    I::SummarizedCallable callable, I::TypeTrackerContentFilter filter,
+    I::SummaryComponentStack input, I::SummaryComponentStack output
+  ) {
+    exists(I::TypeTrackerContent content |
+      callable.propagatesFlow(I::push(I::withContent(content), input), output, true) and
+      filter = I::getFilterFromWithContentStep(content) and
+      not isNonLocal(input.head()) and
+      not isNonLocal(output.head()) and
+      input != output
+    )
+  }
+
+  private predicate componentLevelStep(I::SummaryComponent component) {
+    exists(I::TypeTrackerContent content |
+      component = I::withoutContent(content) and
+      not exists(I::getFilterFromWithoutContentStep(content))
+    )
+  }
+
+  /**
+   * Gets a data flow `I::Node` corresponding an argument or return value of `call`,
+   * as specified by `component`.
+   */
+  bindingset[call, component]
+  private I::Node evaluateSummaryComponentLocal(I::Node call, I::SummaryComponent component) {
+    result = I::argumentOf(call, component)
+    or
+    component = I::return() and
+    result = call
+  }
+
+  /**
+   * Holds if `callable` is relevant for type-tracking and we therefore want `stack` to
+   * be evaluated locally at its call sites.
+   */
+  pragma[nomagic]
+  private predicate dependsOnSummaryComponentStack(
+    I::SummarizedCallable callable, I::SummaryComponentStack stack
+  ) {
+    exists(I::callTo(callable)) and
+    (
+      callable.propagatesFlow(stack, _, true)
+      or
+      callable.propagatesFlow(_, stack, true)
+      or
+      // include store summaries as they may skip an initial step at the input
+      hasStoreSummary(callable, _, stack, _)
+    )
+    or
+    dependsOnSummaryComponentStackCons(callable, _, stack)
+  }
+
+  pragma[nomagic]
+  private predicate dependsOnSummaryComponentStackCons(
+    I::SummarizedCallable callable, I::SummaryComponent head, I::SummaryComponentStack tail
+  ) {
+    dependsOnSummaryComponentStack(callable, I::push(head, tail))
+  }
+
+  pragma[nomagic]
+  private predicate dependsOnSummaryComponentStackConsLocal(
+    I::SummarizedCallable callable, I::SummaryComponent head, I::SummaryComponentStack tail
+  ) {
+    dependsOnSummaryComponentStackCons(callable, head, tail) and
+    not isNonLocal(head)
+  }
+
+  pragma[nomagic]
+  private predicate dependsOnSummaryComponentStackLeaf(
+    I::SummarizedCallable callable, I::SummaryComponent leaf
+  ) {
+    dependsOnSummaryComponentStack(callable, I::singleton(leaf))
+  }
+
+  /**
+   * Gets a data flow I::Node corresponding to the local input or output of `call`
+   * identified by `stack`, if possible.
+   */
+  pragma[nomagic]
+  private I::Node evaluateSummaryComponentStackLocal(
+    I::SummarizedCallable callable, I::Node call, I::SummaryComponentStack stack
+  ) {
+    exists(I::SummaryComponent component |
+      dependsOnSummaryComponentStackLeaf(callable, component) and
+      stack = I::singleton(component) and
+      call = I::callTo(callable) and
+      result = evaluateSummaryComponentLocal(call, component)
+    )
+    or
+    exists(I::Node prev, I::SummaryComponent head, I::SummaryComponentStack tail |
+      prev = evaluateSummaryComponentStackLocal(callable, call, tail) and
+      dependsOnSummaryComponentStackConsLocal(callable, pragma[only_bind_into](head),
+        pragma[only_bind_out](tail)) and
+      stack = I::push(pragma[only_bind_out](head), pragma[only_bind_out](tail))
+    |
+      result = I::parameterOf(prev, head)
+      or
+      result = I::returnOf(prev, head)
+      or
+      componentLevelStep(head) and
+      result = prev
+    )
+  }
+
+  // Implement Output
+  predicate levelStepNoCall(I::Node nodeFrom, I::Node nodeTo) {
+    exists(
+      I::SummarizedCallable callable, I::Node call, I::SummaryComponentStack input,
+      I::SummaryComponentStack output
+    |
+      callable.propagatesFlow(input, output, true) and
+      call = I::callTo(callable) and
+      nodeFrom = evaluateSummaryComponentStackLocal(callable, call, input) and
+      nodeTo = evaluateSummaryComponentStackLocal(callable, call, output)
+    )
+  }
+
+  predicate basicLoadStep(I::Node nodeFrom, I::Node nodeTo, I::TypeTrackerContent content) {
+    exists(
+      I::SummarizedCallable callable, I::Node call, I::SummaryComponentStack input,
+      I::SummaryComponentStack output
+    |
+      hasLoadSummary(callable, content, pragma[only_bind_into](input),
+        pragma[only_bind_into](output)) and
+      call = I::callTo(callable) and
+      nodeFrom = evaluateSummaryComponentStackLocal(callable, call, input) and
+      nodeTo = evaluateSummaryComponentStackLocal(callable, call, output)
+    )
+  }
+
+  predicate basicStoreStep(I::Node nodeFrom, I::Node nodeTo, I::TypeTrackerContent content) {
+    exists(
+      I::SummarizedCallable callable, I::Node call, I::SummaryComponentStack input,
+      I::SummaryComponentStack output
+    |
+      hasStoreSummary(callable, content, pragma[only_bind_into](input),
+        pragma[only_bind_into](output)) and
+      call = I::callTo(callable) and
+      nodeFrom = evaluateSummaryComponentStackLocal(callable, call, input) and
+      nodeTo = evaluateSummaryComponentStackLocal(callable, call, output)
+    )
+  }
+
+  predicate basicLoadStoreStep(
+    I::Node nodeFrom, I::Node nodeTo, I::TypeTrackerContent loadContent,
+    I::TypeTrackerContent storeContent
+  ) {
+    exists(
+      I::SummarizedCallable callable, I::Node call, I::SummaryComponentStack input,
+      I::SummaryComponentStack output
+    |
+      hasLoadStoreSummary(callable, loadContent, storeContent, pragma[only_bind_into](input),
+        pragma[only_bind_into](output)) and
+      call = I::callTo(callable) and
+      nodeFrom = evaluateSummaryComponentStackLocal(callable, call, input) and
+      nodeTo = evaluateSummaryComponentStackLocal(callable, call, output)
+    )
+  }
+
+  predicate basicWithoutContentStep(
+    I::Node nodeFrom, I::Node nodeTo, I::TypeTrackerContentFilter filter
+  ) {
+    exists(
+      I::SummarizedCallable callable, I::Node call, I::SummaryComponentStack input,
+      I::SummaryComponentStack output
+    |
+      hasWithoutContentSummary(callable, filter, pragma[only_bind_into](input),
+        pragma[only_bind_into](output)) and
+      call = I::callTo(callable) and
+      nodeFrom = evaluateSummaryComponentStackLocal(callable, call, input) and
+      nodeTo = evaluateSummaryComponentStackLocal(callable, call, output)
+    )
+  }
+
+  predicate basicWithContentStep(
+    I::Node nodeFrom, I::Node nodeTo, I::TypeTrackerContentFilter filter
+  ) {
+    exists(
+      I::SummarizedCallable callable, I::Node call, I::SummaryComponentStack input,
+      I::SummaryComponentStack output
+    |
+      hasWithContentSummary(callable, filter, pragma[only_bind_into](input),
+        pragma[only_bind_into](output)) and
+      call = I::callTo(callable) and
+      nodeFrom = evaluateSummaryComponentStackLocal(callable, call, input) and
+      nodeTo = evaluateSummaryComponentStackLocal(callable, call, output)
+    )
+  }
+}

ruby/ql/lib/qlpack.yml

@@ -1,11 +1,12 @@
 name: codeql/ruby-all
-version: 0.6.3-dev
+version: 0.6.4-dev
 groups: ruby
 extractor: ruby
 dbscheme: ruby.dbscheme
 upgrades: upgrades
 library: true
 dependencies:
+  codeql/mad: ${workspace}
   codeql/regex: ${workspace}
   codeql/ssa: ${workspace}
   codeql/tutorial: ${workspace}

ruby/ql/src/CHANGELOG.md

@@ -1,3 +1,13 @@
+## 0.6.3
+
+### Minor Analysis Improvements
+
+* Fixed a bug that would occur when an `initialize` method returns `self` or one of its parameters.
+  In such cases, the corresponding calls to `new` would be associated with an incorrect return type.
+  This could result in inaccurate call target resolution and cause false positive alerts.
+* Fixed an issue where calls to `delete` or `assoc` with a constant-valued argument would be analyzed imprecisely,
+  as if the argument value was not a known constant.
+
 ## 0.6.2
 
 No user-facing changes.

ruby/ql/src/change-notes/2023-05-24-delete-name-clash.md

@@ -1,5 +0,0 @@
----
-category: minorAnalysis
----
-* Fixed an issue where calls to `delete` or `assoc` with a constant-valued argument would be analyzed imprecisely,
-  as if the argument value was not a known constant.

ruby/ql/src/change-notes/2023-06-16-zipslip-rename.md

@@ -0,0 +1,4 @@
+---
+category: fix
+---
+* The experimental query "Arbitrary file write during zipfile/tarfile extraction" (`ruby/zipslip`) has been renamed to "Arbitrary file access during archive extraction ("Zip Slip")."

ruby/ql/src/change-notes/released/0.6.3.md

@@ -1,6 +1,9 @@
----
-category: minorAnalysis
----
+## 0.6.3
+
+### Minor Analysis Improvements
+
 * Fixed a bug that would occur when an `initialize` method returns `self` or one of its parameters.
   In such cases, the corresponding calls to `new` would be associated with an incorrect return type.
   This could result in inaccurate call target resolution and cause false positive alerts.
+* Fixed an issue where calls to `delete` or `assoc` with a constant-valued argument would be analyzed imprecisely,
+  as if the argument value was not a known constant.

ruby/ql/src/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.6.2
+lastReleaseVersion: 0.6.3

ruby/ql/src/experimental/cwe-022-zipslip/ZipSlip.qhelp

@@ -4,16 +4,15 @@
 <qhelp>
 
 <overview>
-<p>Extracting files from a malicious tar archive without validating that the destination file path
-is within the destination directory can cause files outside the destination directory to be
-overwritten, due to the possible presence of directory traversal elements (<code>..</code>) in
-archive paths.</p>
+<p>Extracting files from a malicious zip file, or similar type of archive,
+is at risk of directory traversal attacks if filenames from the archive are
+not properly validated.</p>
 
 <p>Tar archives contain archive entries representing each file in the archive. These entries
 include a file path for the entry, but these file paths are not restricted and may contain
 unexpected special elements such as the directory traversal element (<code>..</code>). If these
-file paths are used to determine an output file to write the contents of the archive item to, then
-the file may be written to an unexpected location. This can result in sensitive information being
+file paths are used to create a filesystem path, then a file operation may happen in an 
+unexpected location. This can result in sensitive information being
 revealed or deleted, or an attacker being able to influence behavior by modifying unexpected
 files.</p>
 

ruby/ql/src/experimental/cwe-022-zipslip/ZipSlip.ql

@@ -1,8 +1,8 @@
 /**
- * @name Arbitrary file write during zipfile/tarfile extraction
- * @description Extracting files from a malicious tar archive without validating that the
- *              destination file path is within the destination directory can cause files outside
- *              the destination directory to be overwritten.
+ * @name Arbitrary file access during archive extraction ("Zip Slip")
+ * @description Extracting files from a malicious ZIP file, or similar type of archive, without
+ *              validating that the destination file path is within the destination directory
+ *              can allow an attacker to unexpectedly gain access to resources.
  * @kind path-problem
  * @id rb/zip-slip
  * @problem.severity error

ruby/ql/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/ruby-queries
-version: 0.6.3-dev
+version: 0.6.4-dev
 groups: 
   - ruby
   - queries

ruby/ql/src/queries/security/cwe-829/InsecureDownload.ql

@@ -14,9 +14,9 @@
 import codeql.ruby.AST
 import codeql.ruby.DataFlow
 import codeql.ruby.security.InsecureDownloadQuery
-import DataFlow::PathGraph
+import Flow::PathGraph
 
-from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
-where cfg.hasFlowPath(source, sink)
+from Flow::PathNode source, Flow::PathNode sink
+where Flow::flowPath(source, sink)
 select sink.getNode(), source, sink, "$@ of sensitive file from $@.",
   sink.getNode().(Sink).getDownloadCall(), "Download", source.getNode(), "HTTP source"

ruby/ql/test/TestUtilities/InlineFlowTest.qll

@@ -4,10 +4,11 @@
  * Example for a test.ql:
  * ```ql
  * import TestUtilities.InlineFlowTest
+ * import DefaultFlowTest
  * import PathGraph
  *
- * from DataFlow::PathNode source, DataFlow::PathNode sink, DefaultValueFlowConf conf
- * where conf.hasFlowPath(source, sink)
+ * from PathNode source, PathNode sink
+ * where flowPath(source, sink)
  * select sink, source, sink, "$@", source, source.toString()
  * ```
  *
@@ -20,14 +21,10 @@
  * sink(t); // $ hasTaintFlow=2
  * ```
  *
- * If you're not interested in a specific flow type, you can disable either value or taint flow expectations as follows:
- * ```ql
- * class HasFlowTest extends InlineFlowTest {
- *   override DataFlow::Configuration getTaintFlowConfig() { none() }
- *
- *   override DataFlow::Configuration getValueFlowConfig() { none() }
- * }
- * ```
+ * If you are only interested in value flow, then instead of importing `DefaultFlowTest`, you can import
+ * `ValueFlowTest<DefaultFlowConfig>`. Similarly, if you are only interested in taint flow, then instead of
+ * importing `DefaultFlowTest`, you can import `TaintFlowTest<DefaultFlowConfig>`. In both cases
+ * `DefaultFlowConfig` can be replaced by another implementation of `DataFlow::ConfigSig`.
  *
  * If you need more fine-grained tuning, consider implementing a test using `InlineExpectationsTest`.
  */
@@ -38,72 +35,62 @@ import codeql.ruby.TaintTracking
 import TestUtilities.InlineExpectationsTest
 import TestUtilities.InlineFlowTestUtil
 
-class DefaultValueFlowConf extends DataFlow::Configuration {
-  DefaultValueFlowConf() { this = "qltest:defaultValueFlowConf" }
+module DefaultFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { defaultSource(source) }
 
-  override predicate isSource(DataFlow::Node n) { defaultSource(n) }
+  predicate isSink(DataFlow::Node sink) { defaultSink(sink) }
 
-  override predicate isSink(DataFlow::Node n) { defaultSink(n) }
-
-  override int fieldFlowBranchLimit() { result = 1000 }
+  int fieldFlowBranchLimit() { result = 1000 }
 }
 
-class DefaultTaintFlowConf extends TaintTracking::Configuration {
-  DefaultTaintFlowConf() { this = "qltest:defaultTaintFlowConf" }
+private module NoFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { none() }
 
-  override predicate isSource(DataFlow::Node n) { defaultSource(n) }
-
-  override predicate isSink(DataFlow::Node n) { defaultSink(n) }
-
-  override int fieldFlowBranchLimit() { result = 1000 }
+  predicate isSink(DataFlow::Node sink) { none() }
 }
 
-class InlineFlowTest extends InlineExpectationsTest {
-  InlineFlowTest() { this = "HasFlowTest" }
+module FlowTest<DataFlow::ConfigSig ValueFlowConfig, DataFlow::ConfigSig TaintFlowConfig> {
+  module ValueFlow = DataFlow::Global<ValueFlowConfig>;
 
-  override string getARelevantTag() { result = ["hasValueFlow", "hasTaintFlow"] }
+  module TaintFlow = TaintTracking::Global<TaintFlowConfig>;
 
-  override predicate hasActualResult(Location location, string element, string tag, string value) {
-    tag = "hasValueFlow" and
-    exists(DataFlow::Node src, DataFlow::Node sink | this.getValueFlowConfig().hasFlow(src, sink) |
-      sink.getLocation() = location and
-      element = sink.toString() and
-      if exists(getSourceArgString(src)) then value = getSourceArgString(src) else value = ""
-    )
-    or
-    tag = "hasTaintFlow" and
-    exists(DataFlow::Node src, DataFlow::Node sink |
-      this.getTaintFlowConfig().hasFlow(src, sink) and
-      not this.getValueFlowConfig().hasFlow(src, sink)
-    |
-      sink.getLocation() = location and
-      element = sink.toString() and
-      if exists(getSourceArgString(src)) then value = getSourceArgString(src) else value = ""
-    )
-  }
+  private module InlineTest implements TestSig {
+    string getARelevantTag() { result = ["hasValueFlow", "hasTaintFlow"] }
 
-  DataFlow::Configuration getValueFlowConfig() { result = any(DefaultValueFlowConf config) }
-
-  DataFlow::Configuration getTaintFlowConfig() { result = any(DefaultTaintFlowConf config) }
-}
-
-module PathGraph {
-  private import DataFlow::PathGraph as PG
-
-  private class PathNode extends DataFlow::PathNode {
-    PathNode() {
-      this.getConfiguration() =
-        [any(InlineFlowTest t).getValueFlowConfig(), any(InlineFlowTest t).getTaintFlowConfig()]
+    predicate hasActualResult(Location location, string element, string tag, string value) {
+      tag = "hasValueFlow" and
+      exists(DataFlow::Node src, DataFlow::Node sink | ValueFlow::flow(src, sink) |
+        sink.getLocation() = location and
+        element = sink.toString() and
+        if exists(getSourceArgString(src)) then value = getSourceArgString(src) else value = ""
+      )
+      or
+      tag = "hasTaintFlow" and
+      exists(DataFlow::Node src, DataFlow::Node sink |
+        TaintFlow::flow(src, sink) and not ValueFlow::flow(src, sink)
+      |
+        sink.getLocation() = location and
+        element = sink.toString() and
+        if exists(getSourceArgString(src)) then value = getSourceArgString(src) else value = ""
+      )
     }
   }
 
-  /** Holds if `(a,b)` is an edge in the graph of data flow path explanations. */
-  query predicate edges(PathNode a, PathNode b) { PG::edges(a, b) }
+  import MakeTest<InlineTest>
+  import DataFlow::MergePathGraph<ValueFlow::PathNode, TaintFlow::PathNode, ValueFlow::PathGraph, TaintFlow::PathGraph>
 
-  /** Holds if `n` is a node in the graph of data flow path explanations. */
-  query predicate nodes(PathNode n, string key, string val) { PG::nodes(n, key, val) }
-
-  query predicate subpaths(PathNode arg, PathNode par, PathNode ret, PathNode out) {
-    PG::subpaths(arg, par, ret, out)
+  predicate flowPath(PathNode source, PathNode sink) {
+    ValueFlow::flowPath(source.asPathNode1(), sink.asPathNode1()) or
+    TaintFlow::flowPath(source.asPathNode2(), sink.asPathNode2())
   }
 }
+
+module DefaultFlowTest = FlowTest<DefaultFlowConfig, DefaultFlowConfig>;
+
+module ValueFlowTest<DataFlow::ConfigSig ValueFlowConfig> {
+  import FlowTest<ValueFlowConfig, NoFlowConfig>
+}
+
+module TaintFlowTest<DataFlow::ConfigSig TaintFlowConfig> {
+  import FlowTest<NoFlowConfig, TaintFlowConfig>
+}

ruby/ql/test/TestUtilities/InlineTypeTrackingFlowTest.qll

@@ -15,12 +15,10 @@ DataFlow::LocalSourceNode track(DataFlow::CallNode source) {
   result = track(TypeTracker::end(), source)
 }
 
-class TypeTrackingFlowTest extends InlineExpectationsTest {
-  TypeTrackingFlowTest() { this = "TypeTrackingFlowTest" }
+module TypeTrackingFlowTest implements TestSig {
+  string getARelevantTag() { result = "hasValueFlow" }
 
-  override string getARelevantTag() { result = "hasValueFlow" }
-
-  override predicate hasActualResult(Location location, string element, string tag, string value) {
+  predicate hasActualResult(Location location, string element, string tag, string value) {
     exists(DataFlow::Node sink, DataFlow::Node source |
       defaultSink(sink) and
       track(source).flowsTo(sink) and
@@ -31,3 +29,5 @@ class TypeTrackingFlowTest extends InlineExpectationsTest {
     )
   }
 }
+
+import MakeTest<TypeTrackingFlowTest>

ruby/ql/test/library-tests/dataflow/array-flow/array-flow.expected

@@ -1,4 +1,5 @@
 failures
+testFailures
 edges
 | array_flow.rb:2:5:2:5 | a [element 0] | array_flow.rb:3:10:3:10 | a [element 0] |
 | array_flow.rb:2:5:2:5 | a [element 0] | array_flow.rb:3:10:3:10 | a [element 0] |

ruby/ql/test/library-tests/dataflow/array-flow/array-flow.ql

@@ -4,8 +4,9 @@
 
 import codeql.ruby.AST
 import TestUtilities.InlineFlowTest
+import DefaultFlowTest
 import PathGraph
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, DefaultValueFlowConf conf
-where conf.hasFlowPath(source, sink)
+from ValueFlow::PathNode source, ValueFlow::PathNode sink
+where ValueFlow::flowPath(source, sink)
 select sink, source, sink, "$@", source, source.toString()

ruby/ql/test/library-tests/dataflow/array-flow/type-tracking-array-flow.expected

@@ -1,3 +1,5 @@
+failures
+testFailures
 | array_flow.rb:107:10:107:13 | ...[...] | Unexpected result: hasValueFlow=11.2 |
 | array_flow.rb:179:28:179:46 | # $ hasValueFlow=19 | Missing result:hasValueFlow=19 |
 | array_flow.rb:180:28:180:46 | # $ hasValueFlow=19 | Missing result:hasValueFlow=19 |

ruby/ql/test/library-tests/dataflow/call-sensitivity/call-sensitivity.expected

@@ -1,4 +1,5 @@
 failures
+testFailures
 edges
 | call_sensitivity.rb:9:7:9:13 | call to taint | call_sensitivity.rb:9:6:9:14 | ( ... ) |
 | call_sensitivity.rb:9:7:9:13 | call to taint | call_sensitivity.rb:9:6:9:14 | ( ... ) |

ruby/ql/test/library-tests/dataflow/call-sensitivity/call-sensitivity.ql

@@ -5,13 +5,14 @@
 import codeql.ruby.AST
 import codeql.ruby.DataFlow
 import TestUtilities.InlineFlowTest
-import DataFlow::PathGraph
+import DefaultFlowTest
+import PathGraph
 import codeql.ruby.dataflow.internal.DataFlowDispatch as DataFlowDispatch
 
 query predicate mayBenefitFromCallContext = DataFlowDispatch::mayBenefitFromCallContext/2;
 
 query predicate viableImplInCallContext = DataFlowDispatch::viableImplInCallContext/2;
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, DefaultTaintFlowConf conf
-where conf.hasFlowPath(source, sink)
+from TaintFlow::PathNode source, TaintFlow::PathNode sink
+where TaintFlow::flowPath(source, sink)
 select sink, source, sink, "$@", source, source.toString()

ruby/ql/test/library-tests/dataflow/flow-summaries/semantics.expected

@@ -1,4 +1,5 @@
 failures
+testFailures
 edges
 | semantics.rb:2:5:2:5 | a | semantics.rb:3:9:3:9 | a |
 | semantics.rb:2:5:2:5 | a | semantics.rb:3:9:3:9 | a |

ruby/ql/test/library-tests/dataflow/flow-summaries/semantics.ql

@@ -5,6 +5,7 @@
 
 import codeql.ruby.AST
 import TestUtilities.InlineFlowTest
+import DefaultFlowTest
 import PathGraph
 private import codeql.ruby.dataflow.FlowSummary
 

ruby/ql/test/library-tests/dataflow/global/Flow.expected

@@ -1,4 +1,5 @@
 failures
+testFailures
 edges
 | captured_variables.rb:1:24:1:24 | x | captured_variables.rb:2:20:2:20 | x |
 | captured_variables.rb:1:24:1:24 | x | captured_variables.rb:2:20:2:20 | x |

ruby/ql/test/library-tests/dataflow/global/Flow.ql

@@ -5,8 +5,9 @@
 import codeql.ruby.AST
 import codeql.ruby.DataFlow
 private import TestUtilities.InlineFlowTest
-import DataFlow::PathGraph
+import DefaultFlowTest
+import PathGraph
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, DefaultTaintFlowConf conf
-where conf.hasFlowPath(source, sink)
+from TaintFlow::PathNode source, TaintFlow::PathNode sink
+where TaintFlow::flowPath(source, sink)
 select sink, source, sink, "$@", source, source.toString()

ruby/ql/test/library-tests/dataflow/global/TypeTrackingInlineTest.expected

@@ -1,3 +1,5 @@
+failures
+testFailures
 | captured_variables.rb:9:14:9:14 | x | Fixed missing result:hasValueFlow=1.2 |
 | captured_variables.rb:16:14:16:14 | x | Fixed missing result:hasValueFlow=1.3 |
 | instance_variables.rb:20:16:20:33 | # $ hasValueFlow=7 | Missing result:hasValueFlow=7 |

ruby/ql/test/library-tests/dataflow/hash-flow/hash-flow.expected

@@ -1,4 +1,5 @@
 failures
+testFailures
 edges
 | hash_flow.rb:10:5:10:8 | hash [element 0] | hash_flow.rb:30:10:30:13 | hash [element 0] |
 | hash_flow.rb:10:5:10:8 | hash [element :a] | hash_flow.rb:22:10:22:13 | hash [element :a] |

ruby/ql/test/library-tests/dataflow/hash-flow/hash-flow.ql

@@ -4,12 +4,9 @@
 
 import codeql.ruby.AST
 import TestUtilities.InlineFlowTest
+import ValueFlowTest<DefaultFlowConfig>
 import PathGraph
 
-class HasFlowTest extends InlineFlowTest {
-  override DataFlow::Configuration getTaintFlowConfig() { none() }
-}
-
-from DataFlow::PathNode source, DataFlow::PathNode sink, DefaultValueFlowConf conf
-where conf.hasFlowPath(source, sink)
+from ValueFlow::PathNode source, ValueFlow::PathNode sink
+where ValueFlow::flowPath(source, sink)
 select sink, source, sink, "$@", source, source.toString()

ruby/ql/test/library-tests/dataflow/hash-flow/type-tracking-hash-flow.expected

@@ -1,3 +1,5 @@
+failures
+testFailures
 | hash_flow.rb:65:21:65:40 | # $ hasValueFlow=3.3 | Missing result:hasValueFlow=3.3 |
 | hash_flow.rb:66:21:66:49 | # $ SPURIOUS hasValueFlow=3.3 | Missing result:hasValueFlow=3.3 |
 | hash_flow.rb:114:10:114:17 | ...[...] | Unexpected result: hasValueFlow=7.2 |

ruby/ql/test/library-tests/dataflow/local/InlineFlowTest.expected

@@ -1,4 +1,5 @@
 failures
+testFailures
 edges
 | local_dataflow.rb:78:3:78:3 | z | local_dataflow.rb:89:8:89:8 | z |
 | local_dataflow.rb:78:12:78:20 | call to source | local_dataflow.rb:79:13:79:13 | b |

ruby/ql/test/library-tests/dataflow/local/InlineFlowTest.ql

@@ -4,8 +4,9 @@
 
 import codeql.ruby.AST
 import TestUtilities.InlineFlowTest
+import DefaultFlowTest
 import PathGraph
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, DefaultTaintFlowConf conf
-where conf.hasFlowPath(source, sink)
+from TaintFlow::PathNode source, TaintFlow::PathNode sink
+where TaintFlow::flowPath(source, sink)
 select sink, source, sink, "$@", source, source.toString()

ruby/ql/test/library-tests/dataflow/params/params-flow.expected

@@ -1,4 +1,5 @@
 failures
+testFailures
 edges
 | params_flow.rb:9:16:9:17 | p1 | params_flow.rb:10:10:10:11 | p1 |
 | params_flow.rb:9:20:9:21 | p2 | params_flow.rb:11:10:11:11 | p2 |

ruby/ql/test/library-tests/dataflow/params/params-flow.ql

@@ -4,12 +4,9 @@
 
 import codeql.ruby.AST
 import TestUtilities.InlineFlowTest
+import ValueFlowTest<DefaultFlowConfig>
 import PathGraph
 
-class HasFlowTest extends InlineFlowTest {
-  override DataFlow::Configuration getTaintFlowConfig() { none() }
-}
-
-from DataFlow::PathNode source, DataFlow::PathNode sink, DefaultValueFlowConf conf
-where conf.hasFlowPath(source, sink)
+from ValueFlow::PathNode source, ValueFlow::PathNode sink
+where ValueFlow::flowPath(source, sink)
 select sink, source, sink, "$@", source, source.toString()

ruby/ql/test/library-tests/dataflow/pathname-flow/pathame-flow.expected

@@ -1,4 +1,5 @@
 failures
+testFailures
 edges
 | pathname_flow.rb:4:5:4:6 | pn | pathname_flow.rb:5:10:5:11 | pn |
 | pathname_flow.rb:4:10:4:33 | call to new | pathname_flow.rb:4:5:4:6 | pn |

ruby/ql/test/library-tests/dataflow/pathname-flow/pathame-flow.ql

@@ -4,8 +4,9 @@
 
 import codeql.ruby.AST
 import TestUtilities.InlineFlowTest
+import DefaultFlowTest
 import PathGraph
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, DefaultValueFlowConf conf
-where conf.hasFlowPath(source, sink)
+from ValueFlow::PathNode source, ValueFlow::PathNode sink
+where ValueFlow::flowPath(source, sink)
 select sink, source, sink, "$@", source, source.toString()

ruby/ql/test/library-tests/dataflow/ssa-flow/ssa-flow.expected

@@ -1,4 +1,5 @@
 failures
+testFailures
 edges
 | ssa_flow.rb:12:9:12:9 | [post] a [element 0] | ssa_flow.rb:16:10:16:10 | a [element 0] |
 | ssa_flow.rb:12:9:12:9 | [post] a [element 0] | ssa_flow.rb:16:10:16:10 | a [element 0] |

ruby/ql/test/library-tests/dataflow/ssa-flow/ssa-flow.ql

@@ -4,8 +4,9 @@
 
 import codeql.ruby.AST
 import TestUtilities.InlineFlowTest
+import DefaultFlowTest
 import PathGraph
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, DefaultValueFlowConf conf
-where conf.hasFlowPath(source, sink)
+from ValueFlow::PathNode source, ValueFlow::PathNode sink
+where ValueFlow::flowPath(source, sink)
 select sink, source, sink, "$@", source, source.toString()

ruby/ql/test/library-tests/dataflow/string-flow/string-flow.expected

@@ -1,4 +1,5 @@
 failures
+testFailures
 | string_flow.rb:85:10:85:10 | a | Unexpected result: hasValueFlow=a |
 | string_flow.rb:227:10:227:10 | a | Unexpected result: hasValueFlow=a |
 edges

ruby/ql/test/library-tests/dataflow/string-flow/string-flow.ql

@@ -4,8 +4,9 @@
 
 import codeql.ruby.AST
 import TestUtilities.InlineFlowTest
+import DefaultFlowTest
 import PathGraph
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, DefaultValueFlowConf conf
-where conf.hasFlowPath(source, sink)
+from ValueFlow::PathNode source, ValueFlow::PathNode sink
+where ValueFlow::flowPath(source, sink)
 select sink, source, sink, "$@", source, source.toString()

ruby/ql/test/library-tests/dataflow/summaries/Summaries.expected

@@ -1,4 +1,5 @@
 failures
+testFailures
 edges
 | summaries.rb:1:1:1:7 | tainted | summaries.rb:2:6:2:12 | tainted |
 | summaries.rb:1:1:1:7 | tainted | summaries.rb:2:6:2:12 | tainted |

ruby/ql/test/library-tests/dataflow/summaries/Summaries.ql

@@ -10,7 +10,7 @@ import codeql.ruby.dataflow.internal.FlowSummaryImpl
 import codeql.ruby.dataflow.internal.AccessPathSyntax
 import codeql.ruby.frameworks.data.ModelsAsData
 import TestUtilities.InlineFlowTest
-import DataFlow::PathGraph
+import PathGraph
 
 query predicate invalidSpecComponent(SummarizedCallable sc, string s, string c) {
   (sc.propagatesFlowExt(s, _, _) or sc.propagatesFlowExt(_, s, _)) and
@@ -149,22 +149,18 @@ private class SinkFromModel extends ModelInput::SinkModelCsv {
   }
 }
 
-class CustomValueSink extends DefaultValueFlowConf {
-  override predicate isSink(DataFlow::Node sink) {
-    super.isSink(sink)
+module CustomConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { DefaultFlowConfig::isSource(source) }
+
+  predicate isSink(DataFlow::Node sink) {
+    DefaultFlowConfig::isSink(sink)
     or
     sink = ModelOutput::getASinkNode("test-sink").asSink()
   }
 }
 
-class CustomTaintSink extends DefaultTaintFlowConf {
-  override predicate isSink(DataFlow::Node sink) {
-    super.isSink(sink)
-    or
-    sink = ModelOutput::getASinkNode("test-sink").asSink()
-  }
-}
+import FlowTest<CustomConfig, CustomConfig>
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, DataFlow::Configuration conf
-where conf.hasFlowPath(source, sink)
+from PathNode source, PathNode sink
+where flowPath(source, sink)
 select sink, source, sink, "$@", source, source.toString()

ruby/ql/test/library-tests/frameworks/action_controller/params-flow.expected

@@ -1,4 +1,5 @@
 failures
+testFailures
 | filter_flow.rb:21:10:21:13 | @foo | Unexpected result: hasTaintFlow= |
 | filter_flow.rb:38:10:38:13 | @foo | Unexpected result: hasTaintFlow= |
 | filter_flow.rb:55:10:55:13 | @foo | Unexpected result: hasTaintFlow= |

ruby/ql/test/library-tests/frameworks/action_controller/params-flow.ql

@@ -7,12 +7,14 @@ import TestUtilities.InlineFlowTest
 import PathGraph
 import codeql.ruby.frameworks.Rails
 
-class ParamsTaintFlowConf extends DefaultTaintFlowConf {
-  override predicate isSource(DataFlow::Node n) {
-    n.asExpr().getExpr() instanceof Rails::ParamsCall
-  }
+module ParamsTaintFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node n) { n.asExpr().getExpr() instanceof Rails::ParamsCall }
+
+  predicate isSink(DataFlow::Node n) { DefaultFlowConfig::isSink(n) }
 }
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, ParamsTaintFlowConf conf
-where conf.hasFlowPath(source, sink)
+import FlowTest<DefaultFlowConfig, ParamsTaintFlowConfig>
+
+from TaintFlow::PathNode source, TaintFlow::PathNode sink
+where TaintFlow::flowPath(source, sink)
 select sink, source, sink, "$@", source, source.toString()

ruby/ql/test/library-tests/frameworks/action_mailer/params-flow.expected

@@ -1,4 +1,5 @@
 failures
+testFailures
 edges
 | mailer.rb:3:10:3:15 | call to params | mailer.rb:3:10:3:21 | ...[...] |
 nodes

ruby/ql/test/library-tests/frameworks/action_mailer/params-flow.ql

@@ -7,12 +7,14 @@ import TestUtilities.InlineFlowTest
 import PathGraph
 import codeql.ruby.frameworks.Rails
 
-class ParamsTaintFlowConf extends DefaultTaintFlowConf {
-  override predicate isSource(DataFlow::Node n) {
-    n.asExpr().getExpr() instanceof Rails::ParamsCall
-  }
+module ParamsTaintFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node n) { n.asExpr().getExpr() instanceof Rails::ParamsCall }
+
+  predicate isSink(DataFlow::Node n) { DefaultFlowConfig::isSink(n) }
 }
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, ParamsTaintFlowConf conf
-where conf.hasFlowPath(source, sink)
+import FlowTest<DefaultFlowConfig, ParamsTaintFlowConfig>
+
+from TaintFlow::PathNode source, TaintFlow::PathNode sink
+where TaintFlow::flowPath(source, sink)
 select sink, source, sink, "$@", source, source.toString()

ruby/ql/test/library-tests/frameworks/active_support/ActiveSupportDataFlow.expected

@@ -1,4 +1,5 @@
 failures
+testFailures
 | hash_extensions.rb:126:10:126:19 | call to sole | Unexpected result: hasValueFlow=b |
 edges
 | active_support.rb:10:5:10:5 | x | active_support.rb:11:10:11:10 | x |

ruby/ql/test/library-tests/frameworks/active_support/ActiveSupportDataFlow.ql

@@ -5,8 +5,9 @@
 import codeql.ruby.AST
 import TestUtilities.InlineFlowTest
 import codeql.ruby.Frameworks
+import DefaultFlowTest
 import PathGraph
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, DefaultValueFlowConf conf
-where conf.hasFlowPath(source, sink)
+from ValueFlow::PathNode source, ValueFlow::PathNode sink
+where ValueFlow::flowPath(source, sink)
 select sink, source, sink, "$@", source, source.toString()

ruby/ql/test/library-tests/frameworks/arel/Arel.expected

@@ -1,3 +1,4 @@
 failures
+testFailures
 #select
 | arel.rb:3:8:3:18 | call to sql | arel.rb:2:7:2:14 | call to source | arel.rb:3:8:3:18 | call to sql | $@ | arel.rb:2:7:2:14 | call to source | call to source |

ruby/ql/test/library-tests/frameworks/arel/Arel.ql

@@ -5,7 +5,8 @@
 import codeql.ruby.frameworks.Arel
 import codeql.ruby.AST
 import TestUtilities.InlineFlowTest
+import DefaultFlowTest
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, DefaultTaintFlowConf conf
-where conf.hasFlowPath(source, sink)
+from TaintFlow::PathNode source, TaintFlow::PathNode sink
+where TaintFlow::flowPath(source, sink)
 select sink, source, sink, "$@", source, source.toString()

ruby/ql/test/library-tests/frameworks/json/JsonDataFlow.expected

@@ -1,4 +1,5 @@
 failures
+testFailures
 edges
 | json.rb:1:17:1:26 | call to source | json.rb:1:6:1:27 | call to parse |
 | json.rb:2:18:2:27 | call to source | json.rb:2:6:2:28 | call to parse! |

ruby/ql/test/library-tests/frameworks/json/JsonDataFlow.ql

@@ -4,4 +4,5 @@
 
 import TestUtilities.InlineFlowTest
 import codeql.ruby.Frameworks
+import DefaultFlowTest
 import PathGraph

ruby/ql/test/library-tests/frameworks/rack/Rack.expected

@@ -1,4 +1,11 @@
-| rack.rb:1:1:5:3 | HelloWorld | rack.rb:2:12:2:14 | env |
-| rack.rb:7:1:16:3 | Proxy | rack.rb:12:12:12:18 | the_env |
-| rack.rb:18:1:31:3 | Logger | rack.rb:24:12:24:14 | env |
-| rack.rb:45:1:61:3 | Baz | rack.rb:46:12:46:14 | env |
+rackApps
+| rack.rb:1:1:10:3 | HelloWorld | rack.rb:2:12:2:14 | env |
+| rack.rb:12:1:22:3 | Proxy | rack.rb:17:12:17:18 | the_env |
+| rack.rb:24:1:37:3 | Logger | rack.rb:30:12:30:14 | env |
+| rack.rb:39:1:45:3 | Redirector | rack.rb:40:12:40:14 | env |
+| rack.rb:59:1:75:3 | Baz | rack.rb:60:12:60:14 | env |
+rackResponseContentTypes
+| rack.rb:8:5:8:38 | call to [] | rack.rb:7:34:7:45 | "text/plain" |
+| rack.rb:20:5:20:27 | call to [] | rack.rb:19:28:19:38 | "text/html" |
+redirectResponses
+| rack.rb:43:5:43:45 | call to [] | rack.rb:42:30:42:40 | "/foo.html" |

ruby/ql/test/library-tests/frameworks/rack/Rack.ql

@@ -1,4 +1,17 @@
+private import codeql.ruby.AST
 private import codeql.ruby.frameworks.Rack
 private import codeql.ruby.DataFlow
 
-query predicate rackApps(Rack::AppCandidate c, DataFlow::ParameterNode env) { env = c.getEnv() }
+query predicate rackApps(Rack::App::AppCandidate c, DataFlow::ParameterNode env) {
+  env = c.getEnv()
+}
+
+query predicate rackResponseContentTypes(
+  Rack::Response::ResponseNode resp, DataFlow::Node contentType
+) {
+  contentType = resp.getMimetypeOrContentTypeArg()
+}
+
+query predicate redirectResponses(Rack::Response::RedirectResponse resp, DataFlow::Node location) {
+  location = resp.getRedirectLocation()
+}

ruby/ql/test/library-tests/frameworks/rack/rack.rb

@@ -1,6 +1,11 @@
 class HelloWorld
   def call(env)
-    [200, {'Content-Type' => 'text/plain'}, ['Hello World']]
+    status = 200
+    if something_goes_wrong(env)
+      status = 500
+    end
+    headers = {'Content-Type' => 'text/plain'}
+    [status, headers, ['Hello World']]
   end
 end
 
@@ -11,6 +16,7 @@ class Proxy
 
   def call(the_env)
     status, headers, body = @app.call(the_env)
+    headers.content_type = "text/html"
     [status, headers, body]
   end
 end
@@ -30,6 +36,14 @@ class Logger
   end
 end
 
+class Redirector
+  def call(env)
+    status = 302
+    headers = {'location' => '/foo.html'}
+    [status, headers, ['this is a redirect']]
+  end
+end
+
 class Foo
   def not_call(env)
     [1, 2, 3]

ruby/ql/test/library-tests/frameworks/sinatra/Flow.expected

@@ -1,4 +1,5 @@
 failures
+testFailures
 | views/index.erb:2:10:2:12 | call to foo | Unexpected result: hasTaintFlow= |
 edges
 | app.rb:75:5:75:8 | [post] self [@foo] | app.rb:76:32:76:35 | self [@foo] |

ruby/ql/test/library-tests/frameworks/sinatra/Flow.ql

@@ -8,12 +8,16 @@ import PathGraph
 import codeql.ruby.frameworks.Sinatra
 import codeql.ruby.Concepts
 
-class SinatraConf extends DefaultTaintFlowConf {
-  override predicate isSource(DataFlow::Node source) {
+module SinatraConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
     source instanceof Http::Server::RequestInputAccess::Range
   }
+
+  predicate isSink(DataFlow::Node sink) { DefaultFlowConfig::isSink(sink) }
 }
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, SinatraConf conf
-where conf.hasFlowPath(source, sink)
+import FlowTest<DefaultFlowConfig, SinatraConfig>
+
+from TaintFlow::PathNode source, TaintFlow::PathNode sink
+where TaintFlow::flowPath(source, sink)
 select sink, source, sink, "$@", source, source.toString()

ruby/ql/test/query-tests/security/cwe-829/InsecureDownload.expected

@@ -18,6 +18,7 @@ nodes
 | insecure_download.rb:43:22:43:56 | "http://example.org/unsafe.unk..." | semmle.label | "http://example.org/unsafe.unk..." |
 | insecure_download.rb:53:65:53:78 | "/myscript.sh" | semmle.label | "/myscript.sh" |
 subpaths
+testFailures
 #select
 | insecure_download.rb:27:15:27:45 | "http://example.org/unsafe.APK" | insecure_download.rb:27:15:27:45 | "http://example.org/unsafe.APK" | insecure_download.rb:27:15:27:45 | "http://example.org/unsafe.APK" | $@ | insecure_download.rb:27:15:27:45 | "http://example.org/unsafe.APK" | "http://example.org/unsafe.APK" |
 | insecure_download.rb:27:15:27:45 | "http://example.org/unsafe.APK" | insecure_download.rb:27:15:27:45 | "http://example.org/unsafe.APK" | insecure_download.rb:27:15:27:45 | "http://example.org/unsafe.APK" | $@ | insecure_download.rb:27:15:27:45 | "http://example.org/unsafe.APK" | "http://example.org/unsafe.APK" |

ruby/ql/test/query-tests/security/cwe-829/InsecureDownload.ql

@@ -1,22 +1,25 @@
 import codeql.ruby.AST
 import codeql.ruby.DataFlow
-import PathGraph
-import TestUtilities.InlineFlowTest
 import codeql.ruby.security.InsecureDownloadQuery
+import Flow::PathGraph
+import TestUtilities.InlineExpectationsTest
+import TestUtilities.InlineFlowTestUtil
 
-class FlowTest extends InlineFlowTest {
-  override DataFlow::Configuration getValueFlowConfig() { result = any(Configuration config) }
+module FlowTest implements TestSig {
+  string getARelevantTag() { result = "BAD" }
 
-  override DataFlow::Configuration getTaintFlowConfig() { none() }
-
-  override string getARelevantTag() { result = "BAD" }
-
-  override predicate hasActualResult(Location location, string element, string tag, string value) {
+  predicate hasActualResult(Location location, string element, string tag, string value) {
     tag = "BAD" and
-    super.hasActualResult(location, element, "hasValueFlow", value)
+    exists(DataFlow::Node src, DataFlow::Node sink | Flow::flow(src, sink) |
+      sink.getLocation() = location and
+      element = sink.toString() and
+      if exists(getSourceArgString(src)) then value = getSourceArgString(src) else value = ""
+    )
   }
 }
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, Configuration conf
-where conf.hasFlowPath(source, sink)
+import MakeTest<FlowTest>
+
+from Flow::PathNode source, Flow::PathNode sink
+where Flow::flowPath(source, sink)
 select sink, source, sink, "$@", source, source.toString()