Ruby: associate ContentSets with store/load edges in type tracker

Author: asgerf

ruby/ql/lib/codeql/ruby/ApiGraphs.qll

@@ -269,11 +269,8 @@ module API {
      * Gets a node representing the `contents` stored on the base object.
      */
     Node getContents(DataFlow::ContentSet contents) {
-      this instanceof API::Use and
-      result = this.getContent(contents.getAStoreContent()) // The library has stored a value of interest in `contents`
-      or
-      this instanceof API::Def and
-      result = this.getContent(contents.getAReadContent()) // The library going to read a value stored in `contents`
+      // We already use getAStoreContent when generating the graph, and we always use getAReadContent when querying the graph.
+      result = this.getContent(contents.getAReadContent())
     }
 
     /** Gets a node representing the instance field of the given `name`, which must include the `@` character. */
@@ -523,9 +520,9 @@ module API {
         read = c.getExpr()
       )
       or
-      exists(TypeTrackerSpecific::TypeTrackerContent c |
+      exists(TypeTrackerSpecific::TypeTrackerContentSet c |
         TypeTrackerSpecific::basicLoadStep(node, ref, c) and
-        lbl = Label::content(c)
+        lbl = Label::content(c.getAStoreContent())
       )
       // note: method calls are not handled here as there is no DataFlow::Node for the intermediate MkMethodAccessNode API node
     }
@@ -535,9 +532,9 @@ module API {
      * from a def node that is reachable from `node`.
      */
     private predicate defStep(Label::ApiLabel lbl, DataFlow::Node node, DataFlow::Node rhs) {
-      exists(TypeTrackerSpecific::TypeTrackerContent c |
+      exists(TypeTrackerSpecific::TypeTrackerContentSet c |
         TypeTrackerSpecific::basicStoreStep(rhs, node, c) and
-        lbl = Label::content(c)
+        lbl = Label::content(c.getAStoreContent())
       )
     }
 

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

@@ -12,30 +12,36 @@ private module Cached {
     LevelStep() or
     CallStep() or
     ReturnStep() or
-    StoreStep(TypeTrackerContent content) or
-    LoadStep(TypeTrackerContent content) or
+    StoreStep(TypeTrackerContentSet contents) or
+    LoadStep(TypeTrackerContentSet contents) or
     JumpStep()
 
   /** Gets the summary resulting from appending `step` to type-tracking summary `tt`. */
   cached
   TypeTracker append(TypeTracker tt, StepSummary step) {
-    exists(Boolean hasCall, OptionalTypeTrackerContent content |
-      tt = MkTypeTracker(hasCall, content)
+    exists(Boolean hasCall, OptionalTypeTrackerContent currentContent |
+      tt = MkTypeTracker(hasCall, currentContent)
     |
       step = LevelStep() and result = tt
       or
-      step = CallStep() and result = MkTypeTracker(true, content)
+      step = CallStep() and result = MkTypeTracker(true, currentContent)
       or
       step = ReturnStep() and hasCall = false and result = tt
       or
-      step = LoadStep(content) and result = MkTypeTracker(hasCall, noContent())
+      exists(TypeTrackerContentSet contents |
+        step = LoadStep(contents) and
+        currentContent = contents.getAReadContent() and
+        result = MkTypeTracker(hasCall, noContent())
+      )
       or
-      exists(TypeTrackerContent p |
-        step = StoreStep(p) and content = noContent() and result = MkTypeTracker(hasCall, p)
+      exists(TypeTrackerContentSet contents |
+        step = StoreStep(contents) and
+        currentContent = noContent() and
+        result = MkTypeTracker(hasCall, contents.getAStoreContent())
       )
       or
       step = JumpStep() and
-      result = MkTypeTracker(false, content)
+      result = MkTypeTracker(false, currentContent)
     )
   }
 
@@ -51,11 +57,17 @@ private module Cached {
       or
       step = ReturnStep() and result = MkTypeBackTracker(true, content)
       or
-      exists(TypeTrackerContent p |
-        step = LoadStep(p) and content = noContent() and result = MkTypeBackTracker(hasReturn, p)
+      exists(TypeTrackerContentSet contents |
+        step = LoadStep(contents) and
+        content = noContent() and
+        result = MkTypeBackTracker(hasReturn, contents.getAStoreContent())
       )
       or
-      step = StoreStep(content) and result = MkTypeBackTracker(hasReturn, noContent())
+      exists(TypeTrackerContentSet contents |
+        step = StoreStep(contents) and
+        content = contents.getAReadContent() and
+        result = MkTypeBackTracker(hasReturn, noContent())
+      )
       or
       step = JumpStep() and
       result = MkTypeBackTracker(false, content)
@@ -99,9 +111,11 @@ class StepSummary extends TStepSummary {
     or
     this instanceof ReturnStep and result = "return"
     or
-    exists(TypeTrackerContent content | this = StoreStep(content) | result = "store " + content)
+    exists(TypeTrackerContentSet contents | this = StoreStep(contents) |
+      result = "store " + contents
+    )
     or
-    exists(TypeTrackerContent content | this = LoadStep(content) | result = "load " + content)
+    exists(TypeTrackerContentSet contents | this = LoadStep(contents) | result = "load " + contents)
     or
     this instanceof JumpStep and result = "jump"
   }
@@ -115,11 +129,11 @@ private predicate smallstepNoCall(Node nodeFrom, TypeTrackingNode nodeTo, StepSu
   levelStep(nodeFrom, nodeTo) and
   summary = LevelStep()
   or
-  exists(TypeTrackerContent content |
-    StepSummary::localSourceStoreStep(nodeFrom, nodeTo, content) and
-    summary = StoreStep(content)
+  exists(TypeTrackerContentSet contents |
+    StepSummary::localSourceStoreStep(nodeFrom, nodeTo, contents) and
+    summary = StoreStep(contents)
     or
-    basicLoadStep(nodeFrom, nodeTo, content) and summary = LoadStep(content)
+    basicLoadStep(nodeFrom, nodeTo, contents) and summary = LoadStep(contents)
   )
 }
 
@@ -189,8 +203,10 @@ module StepSummary {
    * function. This means we will track the fact that `x.attr` can have the type of `y` into the
    * assignment to `z` inside `bar`, even though this attribute write happens _after_ `bar` is called.
    */
-  predicate localSourceStoreStep(Node nodeFrom, TypeTrackingNode nodeTo, TypeTrackerContent content) {
-    exists(Node obj | nodeTo.flowsTo(obj) and basicStoreStep(nodeFrom, obj, content))
+  predicate localSourceStoreStep(
+    Node nodeFrom, TypeTrackingNode nodeTo, TypeTrackerContentSet contents
+  ) {
+    exists(Node obj | nodeTo.flowsTo(obj) and basicStoreStep(nodeFrom, obj, contents))
   }
 }
 

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

@@ -22,6 +22,8 @@ private newtype TOptionalTypeTrackerContent =
 
 class TypeTrackerContent = DataFlowPublic::Content;
 
+class TypeTrackerContentSet = DataFlowPublic::ContentSet;
+
 predicate noContent = DataFlowPublic::Content::noContent/0;
 
 /** Holds if there is a simple local flow step from `nodeFrom` to `nodeTo` */
@@ -150,34 +152,30 @@ predicate returnStep(Node nodeFrom, Node nodeTo) {
  * to `z` inside `bar`, even though this content write happens _after_ `bar` is
  * called.
  */
-predicate basicStoreStep(Node nodeFrom, Node nodeTo, TypeTrackerContent content) {
-  postUpdateStoreStep(nodeFrom, nodeTo, content)
+predicate basicStoreStep(Node nodeFrom, Node nodeTo, TypeTrackerContentSet contents) {
+  postUpdateStoreStep(nodeFrom, nodeTo, contents)
   or
-  exists(
-    DataFlowPublic::CallNode call, SummaryComponent input, DataFlowPublic::ContentSet contents,
-    SummaryComponent output
-  |
+  exists(DataFlowPublic::CallNode call, SummaryComponent input, SummaryComponent output |
     summarizableCall(call.asExpr().getExpr(), //
       SummaryComponentStack::singleton(input),
       SummaryComponentStack::push(SummaryComponent::content(contents),
         SummaryComponentStack::singleton(output))) and
     nodeFrom = evaluateSummaryComponentLocal(call, input) and
-    nodeTo = evaluateSummaryComponentLocal(call, output) and
-    content = contents.getAStoreContent()
+    nodeTo = evaluateSummaryComponentLocal(call, output)
   )
 }
 
 /**
  * A `content`-store step from `nodeFrom -> nodeTo` where the destination node is a post-update
  * node that should be treated as a local source node.
  */
-predicate postUpdateStoreStep(Node nodeFrom, Node nodeTo, TypeTrackerContent content) {
+predicate postUpdateStoreStep(Node nodeFrom, Node nodeTo, TypeTrackerContentSet contents) {
   // TODO: support SetterMethodCall inside TuplePattern
   exists(ExprNodes::MethodCallCfgNode call |
-    content =
-      DataFlowPublic::Content::getAttributeName(call.getExpr()
-            .(Ast::SetterMethodCall)
-            .getTargetName()) and
+    contents
+        .isSingleton(DataFlowPublic::Content::getAttributeName(call.getExpr()
+                .(Ast::SetterMethodCall)
+                .getTargetName())) and
     nodeTo.(DataFlowPublic::PostUpdateNode).getPreUpdateNode().asExpr() = call.getReceiver() and
     call.getExpr() instanceof Ast::SetterMethodCall and
     call.getArgument(call.getNumberOfArguments() - 1) =
@@ -188,25 +186,21 @@ predicate postUpdateStoreStep(Node nodeFrom, Node nodeTo, TypeTrackerContent con
 /**
  * Holds if `nodeTo` is the result of accessing the `content` content of `nodeFrom`.
  */
-predicate basicLoadStep(Node nodeFrom, Node nodeTo, TypeTrackerContent content) {
+predicate basicLoadStep(Node nodeFrom, Node nodeTo, TypeTrackerContentSet contents) {
   exists(ExprNodes::MethodCallCfgNode call |
     call.getExpr().getNumberOfArguments() = 0 and
-    content = DataFlowPublic::Content::getAttributeName(call.getExpr().getMethodName()) and
+    contents.isSingleton(DataFlowPublic::Content::getAttributeName(call.getExpr().getMethodName())) and
     nodeFrom.asExpr() = call.getReceiver() and
     nodeTo.asExpr() = call
   )
   or
-  exists(
-    DataFlowPublic::CallNode call, SummaryComponent input, DataFlowPublic::ContentSet contents,
-    SummaryComponent output
-  |
+  exists(DataFlowPublic::CallNode call, SummaryComponent input, SummaryComponent output |
     summarizableCall(call.asExpr().getExpr(), //
       SummaryComponentStack::push(SummaryComponent::content(contents),
         SummaryComponentStack::singleton(input)), //
       SummaryComponentStack::singleton(output)) and
     nodeFrom = evaluateSummaryComponentLocal(call, input) and
-    nodeTo = evaluateSummaryComponentLocal(call, output) and
-    content = contents.getAReadContent()
+    nodeTo = evaluateSummaryComponentLocal(call, output)
   )
 }