Merge pull request #10650 from asgerf/rb/summarize-more

Ruby: more type-tracking steps

Author: asgerf

python/ql/lib/semmle/python/dataflow/new/internal/TypeTracker.qll

@@ -14,6 +14,11 @@ private module Cached {
     ReturnStep() or
     StoreStep(TypeTrackerContent content) { basicStoreStep(_, _, content) } or
     LoadStep(TypeTrackerContent content) { basicLoadStep(_, _, content) } or
+    LoadStoreStep(TypeTrackerContent load, TypeTrackerContent store) {
+      basicLoadStoreStep(_, _, load, store)
+    } or
+    WithContent(ContentFilter filter) { basicWithContentStep(_, _, filter) } or
+    WithoutContent(ContentFilter filter) { basicWithoutContentStep(_, _, filter) } or
     JumpStep()
 
   cached
@@ -25,7 +30,11 @@ private module Cached {
       // We can't rely on `basicStoreStep` since `startInContent` might be used with
       // a content that has no corresponding store.
       exists(TypeTrackerContent loadContents |
-        basicLoadStep(_, _, loadContents) and
+        (
+          basicLoadStep(_, _, loadContents)
+          or
+          basicLoadStoreStep(_, _, loadContents, _)
+        ) and
         compatibleContents(content, loadContents)
       )
     }
@@ -37,7 +46,11 @@ private module Cached {
       or
       // As in MkTypeTracker, restrict `content` to those that might eventually match a store.
       exists(TypeTrackerContent storeContent |
-        basicStoreStep(_, _, storeContent) and
+        (
+          basicStoreStep(_, _, storeContent)
+          or
+          basicLoadStoreStep(_, _, _, storeContent)
+        ) and
         compatibleContents(storeContent, content)
       )
     }
@@ -61,6 +74,14 @@ private module Cached {
       or
       step = JumpStep() and
       result = MkTypeTracker(false, currentContents)
+      or
+      exists(ContentFilter filter | result = tt |
+        step = WithContent(filter) and
+        currentContents = filter.getAMatchingContent()
+        or
+        step = WithoutContent(filter) and
+        not currentContents = filter.getAMatchingContent()
+      )
     )
     or
     exists(TypeTrackerContent storeContents, boolean hasCall |
@@ -75,6 +96,16 @@ private module Cached {
       tt = noContentTypeTracker(hasCall) and
       result = MkTypeTracker(hasCall, storeContents)
     )
+    or
+    exists(
+      TypeTrackerContent currentContent, TypeTrackerContent store, TypeTrackerContent load,
+      boolean hasCall
+    |
+      step = LoadStoreStep(pragma[only_bind_into](load), pragma[only_bind_into](store)) and
+      compatibleContents(pragma[only_bind_into](currentContent), load) and
+      tt = MkTypeTracker(pragma[only_bind_into](hasCall), currentContent) and
+      result = MkTypeTracker(pragma[only_bind_out](hasCall), store)
+    )
   }
 
   pragma[nomagic]
@@ -96,6 +127,14 @@ private module Cached {
       or
       step = JumpStep() and
       result = MkTypeBackTracker(false, content)
+      or
+      exists(ContentFilter filter | result = tbt |
+        step = WithContent(filter) and
+        content = filter.getAMatchingContent()
+        or
+        step = WithoutContent(filter) and
+        not content = filter.getAMatchingContent()
+      )
     )
     or
     exists(TypeTrackerContent loadContents, boolean hasReturn |
@@ -110,6 +149,16 @@ private module Cached {
       tbt = noContentTypeBackTracker(hasReturn) and
       result = MkTypeBackTracker(hasReturn, loadContents)
     )
+    or
+    exists(
+      TypeTrackerContent currentContent, TypeTrackerContent store, TypeTrackerContent load,
+      boolean hasCall
+    |
+      step = LoadStoreStep(pragma[only_bind_into](load), pragma[only_bind_into](store)) and
+      compatibleContents(store, pragma[only_bind_into](currentContent)) and
+      tbt = MkTypeBackTracker(pragma[only_bind_into](hasCall), currentContent) and
+      result = MkTypeBackTracker(pragma[only_bind_out](hasCall), load)
+    )
   }
 
   /**
@@ -146,6 +195,19 @@ private module Cached {
       or
       basicLoadStep(nodeFrom, nodeTo, content) and summary = LoadStep(content)
     )
+    or
+    exists(TypeTrackerContent loadContent, TypeTrackerContent storeContent |
+      flowsToLoadStoreStep(nodeFrom, nodeTo, loadContent, storeContent) and
+      summary = LoadStoreStep(loadContent, storeContent)
+    )
+    or
+    exists(ContentFilter filter |
+      basicWithContentStep(nodeFrom, nodeTo, filter) and
+      summary = WithContent(filter)
+      or
+      basicWithoutContentStep(nodeFrom, nodeTo, filter) and
+      summary = WithoutContent(filter)
+    )
   }
 
   cached
@@ -190,6 +252,18 @@ private predicate flowsToStoreStep(
   exists(Node obj | nodeTo.flowsTo(obj) and basicStoreStep(nodeFrom, obj, content))
 }
 
+/**
+ * Holds if `loadContent` is loaded from `nodeFrom` and written to `storeContent` of `nodeTo`.
+ */
+private predicate flowsToLoadStoreStep(
+  Node nodeFrom, TypeTrackingNode nodeTo, TypeTrackerContent loadContent,
+  TypeTrackerContent storeContent
+) {
+  exists(Node obj |
+    nodeTo.flowsTo(obj) and basicLoadStoreStep(nodeFrom, obj, loadContent, storeContent)
+  )
+}
+
 /**
  * INTERNAL: Use `TypeTracker` or `TypeBackTracker` instead.
  *
@@ -208,6 +282,11 @@ class StepSummary extends TStepSummary {
     or
     exists(TypeTrackerContent content | this = LoadStep(content) | result = "load " + content)
     or
+    exists(TypeTrackerContent load, TypeTrackerContent store |
+      this = LoadStoreStep(load, store) and
+      result = "load-store " + load + " -> " + store
+    )
+    or
     this instanceof JumpStep and result = "jump"
   }
 }

python/ql/lib/semmle/python/dataflow/new/internal/TypeTrackerSpecific.qll

@@ -28,6 +28,14 @@ class TypeTrackerContent extends OptionalTypeTrackerContent {
 /** Gets the content string representing no value. */
 OptionalTypeTrackerContent noContent() { result = "" }
 
+/**
+ * A label to use for `WithContent` and `WithoutContent` steps, restricting
+ * which `ContentSet` may pass through. Not currently used in Python.
+ */
+class ContentFilter extends Unit {
+  TypeTrackerContent getAMatchingContent() { none() }
+}
+
 pragma[inline]
 predicate compatibleContents(TypeTrackerContent storeContent, TypeTrackerContent loadContent) {
   storeContent = loadContent
@@ -110,6 +118,23 @@ predicate basicLoadStep(Node nodeFrom, Node nodeTo, string content) {
   )
 }
 
+/**
+ * Holds if the `loadContent` of `nodeFrom` is stored in the `storeContent` of `nodeTo`.
+ */
+predicate basicLoadStoreStep(Node nodeFrom, Node nodeTo, string loadContent, string storeContent) {
+  none()
+}
+
+/**
+ * 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) { none() }
+
+/**
+ * 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) { none() }
+
 /**
  * A utility class that is equivalent to `boolean` but does not require type joining.
  */

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

@@ -620,6 +620,11 @@ private predicate localFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo,
   summary.toString() = "level"
 }
 
+pragma[nomagic]
+private predicate hasAdjacentTypeCheckedReads(DataFlow::Node node) {
+  hasAdjacentTypeCheckedReads(_, _, node.asExpr(), _)
+}
+
 /**
  * We exclude steps into `self` parameters and type checked variables. For those,
  * we instead rely on the type of the enclosing module resp. the type being checked
@@ -629,12 +634,12 @@ private predicate localFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo,
 pragma[nomagic]
 private DataFlow::Node trackInstanceRec(Module tp, TypeTracker t, boolean exact, StepSummary summary) {
   exists(DataFlow::Node mid | mid = trackInstance(tp, exact, t) |
-    StepSummary::smallstep(mid, result, summary)
+    StepSummary::smallstep(mid, result, summary) and
+    not result instanceof SelfParameterNode
     or
-    localFlowStep(mid, result, summary)
-  ) and
-  not result instanceof SelfParameterNode and
-  not hasAdjacentTypeCheckedReads(_, _, result.asExpr(), _)
+    localFlowStep(mid, result, summary) and
+    not hasAdjacentTypeCheckedReads(result)
+  )
 }
 
 pragma[nomagic]

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

@@ -366,8 +366,7 @@ private module Cached {
 
   cached
   predicate isLocalSourceNode(Node n) {
-    n instanceof ParameterNode and
-    not n instanceof SynthHashSplatParameterNode
+    n instanceof ParameterNode
     or
     // Expressions that can't be reached from another entry definition or expression
     n instanceof ExprNode and
@@ -381,7 +380,7 @@ private module Cached {
     n instanceof SynthReturnNode
     or
     // Needed for stores in type tracking
-    TypeTrackerSpecific::postUpdateStoreStep(_, n, _)
+    TypeTrackerSpecific::storeStepIntoSourceNode(_, n, _)
   }
 
   cached
@@ -1010,6 +1009,31 @@ private ContentSet getKeywordContent(string name) {
   )
 }
 
+/**
+ * Subset of `storeStep` that should be shared with type-tracking.
+ */
+predicate storeStepCommon(Node node1, ContentSet c, Node node2) {
+  // Wrap all key-value arguments in a synthesized hash-splat argument node
+  exists(CfgNodes::ExprNodes::CallCfgNode call | node2 = TSynthHashSplatArgumentNode(call) |
+    // symbol key
+    exists(ArgumentPosition keywordPos, string name |
+      node1.asExpr().(Argument).isArgumentOf(call, keywordPos) and
+      keywordPos.isKeyword(name) and
+      c = getKeywordContent(name)
+    )
+    or
+    // non-symbol key
+    exists(CfgNodes::ExprNodes::PairCfgNode pair, CfgNodes::ExprCfgNode key, ConstantValue cv |
+      node1.asExpr() = pair.getValue() and
+      pair = call.getAnArgument() and
+      key = pair.getKey() and
+      cv = key.getConstantValue() and
+      not cv.isSymbol(_) and
+      c.isSingleton(TKnownElementContent(cv))
+    )
+  )
+}
+
 /**
  * Holds if data can flow from `node1` to `node2` via an assignment to
  * content `c`.
@@ -1040,25 +1064,7 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
   or
   FlowSummaryImpl::Private::Steps::summaryStoreStep(node1, c, node2)
   or
-  // Wrap all key-value arguments in a synthesized hash-splat argument node
-  exists(CfgNodes::ExprNodes::CallCfgNode call | node2 = TSynthHashSplatArgumentNode(call) |
-    // symbol key
-    exists(ArgumentPosition keywordPos, string name |
-      node1.asExpr().(Argument).isArgumentOf(call, keywordPos) and
-      keywordPos.isKeyword(name) and
-      c = getKeywordContent(name)
-    )
-    or
-    // non-symbol key
-    exists(CfgNodes::ExprNodes::PairCfgNode pair, CfgNodes::ExprCfgNode key, ConstantValue cv |
-      node1.asExpr() = pair.getValue() and
-      pair = call.getAnArgument() and
-      key = pair.getKey() and
-      cv = key.getConstantValue() and
-      not cv.isSymbol(_) and
-      c.isSingleton(TKnownElementContent(cv))
-    )
-  )
+  storeStepCommon(node1, c, node2)
 }
 
 /**

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

@@ -77,7 +77,7 @@ private class SplatSummary extends SummarizedCallable {
 private class HashSplatSummary extends SummarizedCallable {
   HashSplatSummary() { this = "**(hash-splat)" }
 
-  override HashSplatExpr getACall() { any() }
+  override HashSplatExpr getACallSimple() { any() }
 
   override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
     input = "Argument[self].WithElement[any]" and