Merge pull request #10375 from asgerf/rb/summarize-loads-v2

Ruby: type-tracking and API edges through simple library callables

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

@@ -5,12 +5,13 @@
 
 private import python
 private import internal.TypeTracker as Internal
+private import internal.TypeTrackerSpecific as InternalSpecific
 
 /** A string that may appear as the name of an attribute or access path. */
-class AttributeName = Internal::ContentName;
+class AttributeName = InternalSpecific::TypeTrackerContent;
 
 /** An attribute name, or the empty string (representing no attribute). */
-class OptionalAttributeName = Internal::OptionalContentName;
+class OptionalAttributeName = InternalSpecific::OptionalTypeTrackerContent;
 
 /**
  * The summary of the steps needed to track a value to a given dataflow node.

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

@@ -2,27 +2,6 @@
 
 private import TypeTrackerSpecific
 
-/**
- * A string that may appear as the name of a piece of content. This will usually include things like:
- * - Attribute names (in Python)
- * - Property names (in JavaScript)
- *
- * In general, this can also be used to model things like stores to specific list indices. To ensure
- * correctness, it is important that
- *
- * - different types of content do not have overlapping names, and
- * - the empty string `""` is not a valid piece of content, as it is used to indicate the absence of
- *   content instead.
- */
-class ContentName extends string {
-  ContentName() { this = getPossibleContentName() }
-}
-
-/** A content name, or the empty string (representing no content). */
-class OptionalContentName extends string {
-  OptionalContentName() { this instanceof ContentName or this = "" }
-}
-
 cached
 private module Cached {
   /**
@@ -33,48 +12,78 @@ private module Cached {
     LevelStep() or
     CallStep() or
     ReturnStep() or
-    StoreStep(ContentName content) or
-    LoadStep(ContentName content) or
+    StoreStep(TypeTrackerContent content) { basicStoreStep(_, _, content) } or
+    LoadStep(TypeTrackerContent content) { basicLoadStep(_, _, content) } or
     JumpStep()
 
+  pragma[nomagic]
+  private TypeTracker noContentTypeTracker(boolean hasCall) {
+    result = MkTypeTracker(hasCall, noContent())
+  }
+
   /** Gets the summary resulting from appending `step` to type-tracking summary `tt`. */
   cached
   TypeTracker append(TypeTracker tt, StepSummary step) {
-    exists(Boolean hasCall, OptionalContentName content | tt = MkTypeTracker(hasCall, content) |
+    exists(Boolean hasCall, OptionalTypeTrackerContent currentContents |
+      tt = MkTypeTracker(hasCall, currentContents)
+    |
       step = LevelStep() and result = tt
       or
-      step = CallStep() and result = MkTypeTracker(true, content)
+      step = CallStep() and result = MkTypeTracker(true, currentContents)
       or
       step = ReturnStep() and hasCall = false and result = tt
       or
-      step = LoadStep(content) and result = MkTypeTracker(hasCall, "")
-      or
-      exists(string p | step = StoreStep(p) and content = "" and result = MkTypeTracker(hasCall, p))
-      or
       step = JumpStep() and
-      result = MkTypeTracker(false, content)
+      result = MkTypeTracker(false, currentContents)
     )
+    or
+    exists(TypeTrackerContent storeContents, boolean hasCall |
+      exists(TypeTrackerContent loadContents |
+        step = LoadStep(pragma[only_bind_into](loadContents)) and
+        tt = MkTypeTracker(hasCall, storeContents) and
+        compatibleContents(storeContents, loadContents) and
+        result = noContentTypeTracker(hasCall)
+      )
+      or
+      step = StoreStep(pragma[only_bind_into](storeContents)) and
+      tt = noContentTypeTracker(hasCall) and
+      result = MkTypeTracker(hasCall, storeContents)
+    )
+  }
+
+  pragma[nomagic]
+  private TypeBackTracker noContentTypeBackTracker(boolean hasReturn) {
+    result = MkTypeBackTracker(hasReturn, noContent())
   }
 
   /** Gets the summary resulting from prepending `step` to this type-tracking summary. */
   cached
   TypeBackTracker prepend(TypeBackTracker tbt, StepSummary step) {
-    exists(Boolean hasReturn, string content | tbt = MkTypeBackTracker(hasReturn, content) |
+    exists(Boolean hasReturn, OptionalTypeTrackerContent content |
+      tbt = MkTypeBackTracker(hasReturn, content)
+    |
       step = LevelStep() and result = tbt
       or
       step = CallStep() and hasReturn = false and result = tbt
       or
       step = ReturnStep() and result = MkTypeBackTracker(true, content)
       or
-      exists(string p |
-        step = LoadStep(p) and content = "" and result = MkTypeBackTracker(hasReturn, p)
-      )
-      or
-      step = StoreStep(content) and result = MkTypeBackTracker(hasReturn, "")
-      or
       step = JumpStep() and
       result = MkTypeBackTracker(false, content)
     )
+    or
+    exists(TypeTrackerContent loadContents, boolean hasReturn |
+      exists(TypeTrackerContent storeContents |
+        step = StoreStep(pragma[only_bind_into](storeContents)) and
+        tbt = MkTypeBackTracker(hasReturn, loadContents) and
+        compatibleContents(storeContents, loadContents) and
+        result = noContentTypeBackTracker(hasReturn)
+      )
+      or
+      step = LoadStep(pragma[only_bind_into](loadContents)) and
+      tbt = noContentTypeBackTracker(hasReturn) and
+      result = MkTypeBackTracker(hasReturn, loadContents)
+    )
   }
 
   /**
@@ -114,9 +123,9 @@ class StepSummary extends TStepSummary {
     or
     this instanceof ReturnStep and result = "return"
     or
-    exists(string content | this = StoreStep(content) | result = "store " + content)
+    exists(TypeTrackerContent content | this = StoreStep(content) | result = "store " + content)
     or
-    exists(string content | this = LoadStep(content) | result = "load " + content)
+    exists(TypeTrackerContent content | this = LoadStep(content) | result = "load " + content)
     or
     this instanceof JumpStep and result = "jump"
   }
@@ -130,7 +139,7 @@ private predicate smallstepNoCall(Node nodeFrom, TypeTrackingNode nodeTo, StepSu
   levelStep(nodeFrom, nodeTo) and
   summary = LevelStep()
   or
-  exists(string content |
+  exists(TypeTrackerContent content |
     StepSummary::localSourceStoreStep(nodeFrom, nodeTo, content) and
     summary = StoreStep(content)
     or
@@ -180,7 +189,7 @@ module StepSummary {
   }
 
   /**
-   * Holds if `nodeFrom` is being written to the `content` content of the object in `nodeTo`.
+   * Holds if `nodeFrom` is being written to the `content` of the object in `nodeTo`.
    *
    * Note that `nodeTo` will always be a local source node that flows to the place where the content
    * is written in `basicStoreStep`. This may lead to the flow of information going "back in time"
@@ -204,12 +213,23 @@ 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, string content) {
+  predicate localSourceStoreStep(Node nodeFrom, TypeTrackingNode nodeTo, TypeTrackerContent content) {
     exists(Node obj | nodeTo.flowsTo(obj) and basicStoreStep(nodeFrom, obj, content))
   }
 }
 
-private newtype TTypeTracker = MkTypeTracker(Boolean hasCall, OptionalContentName content)
+private newtype TTypeTracker =
+  MkTypeTracker(Boolean hasCall, OptionalTypeTrackerContent content) {
+    content = noContent()
+    or
+    // Restrict `content` to those that might eventually match a load.
+    // 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
+      compatibleContents(content, loadContents)
+    )
+  }
 
 /**
  * A summary of the steps needed to track a value to a given dataflow node.
@@ -240,7 +260,7 @@ private newtype TTypeTracker = MkTypeTracker(Boolean hasCall, OptionalContentNam
  */
 class TypeTracker extends TTypeTracker {
   Boolean hasCall;
-  OptionalContentName content;
+  OptionalTypeTrackerContent content;
 
   TypeTracker() { this = MkTypeTracker(hasCall, content) }
 
@@ -251,7 +271,11 @@ class TypeTracker extends TTypeTracker {
   string toString() {
     exists(string withCall, string withContent |
       (if hasCall = true then withCall = "with" else withCall = "without") and
-      (if content != "" then withContent = " with content " + content else withContent = "") and
+      (
+        if content != noContent()
+        then withContent = " with content " + content
+        else withContent = ""
+      ) and
       result = "type tracker " + withCall + " call steps" + withContent
     )
   }
@@ -259,24 +283,26 @@ class TypeTracker extends TTypeTracker {
   /**
    * Holds if this is the starting point of type tracking.
    */
-  predicate start() { hasCall = false and content = "" }
+  predicate start() { hasCall = false and content = noContent() }
 
   /**
    * Holds if this is the starting point of type tracking, and the value starts in the content named `contentName`.
    * The type tracking only ends after the content has been loaded.
    */
-  predicate startInContent(ContentName contentName) { hasCall = false and content = contentName }
+  predicate startInContent(TypeTrackerContent contentName) {
+    hasCall = false and content = contentName
+  }
 
   /**
    * Holds if this is the starting point of type tracking
    * when tracking a parameter into a call, but not out of it.
    */
-  predicate call() { hasCall = true and content = "" }
+  predicate call() { hasCall = true and content = noContent() }
 
   /**
    * Holds if this is the end point of type tracking.
    */
-  predicate end() { content = "" }
+  predicate end() { content = noContent() }
 
   /**
    * INTERNAL. DO NOT USE.
@@ -290,7 +316,7 @@ class TypeTracker extends TTypeTracker {
    *
    * Gets the content associated with this type tracker.
    */
-  string getContent() { result = content }
+  OptionalTypeTrackerContent getContent() { result = content }
 
   /**
    * Gets a type tracker that starts where this one has left off to allow continued
@@ -298,7 +324,7 @@ class TypeTracker extends TTypeTracker {
    *
    * This predicate is only defined if the type is not associated to a piece of content.
    */
-  TypeTracker continue() { content = "" and result = this }
+  TypeTracker continue() { content = noContent() and result = this }
 
   /**
    * Gets the summary that corresponds to having taken a forwards
@@ -356,7 +382,16 @@ module TypeTracker {
   TypeTracker end() { result.end() }
 }
 
-private newtype TTypeBackTracker = MkTypeBackTracker(Boolean hasReturn, OptionalContentName content)
+private newtype TTypeBackTracker =
+  MkTypeBackTracker(Boolean hasReturn, OptionalTypeTrackerContent content) {
+    content = noContent()
+    or
+    // As in MkTypeTracker, restrict `content` to those that might eventually match a store.
+    exists(TypeTrackerContent storeContent |
+      basicStoreStep(_, _, storeContent) and
+      compatibleContents(storeContent, content)
+    )
+  }
 
 /**
  * A summary of the steps needed to back-track a use of a value to a given dataflow node.
@@ -390,7 +425,7 @@ private newtype TTypeBackTracker = MkTypeBackTracker(Boolean hasReturn, Optional
  */
 class TypeBackTracker extends TTypeBackTracker {
   Boolean hasReturn;
-  string content;
+  OptionalTypeTrackerContent content;
 
   TypeBackTracker() { this = MkTypeBackTracker(hasReturn, content) }
 
@@ -401,7 +436,11 @@ class TypeBackTracker extends TTypeBackTracker {
   string toString() {
     exists(string withReturn, string withContent |
       (if hasReturn = true then withReturn = "with" else withReturn = "without") and
-      (if content != "" then withContent = " with content " + content else withContent = "") and
+      (
+        if content != noContent()
+        then withContent = " with content " + content
+        else withContent = ""
+      ) and
       result = "type back-tracker " + withReturn + " return steps" + withContent
     )
   }
@@ -409,12 +448,12 @@ class TypeBackTracker extends TTypeBackTracker {
   /**
    * Holds if this is the starting point of type tracking.
    */
-  predicate start() { hasReturn = false and content = "" }
+  predicate start() { hasReturn = false and content = noContent() }
 
   /**
    * Holds if this is the end point of type tracking.
    */
-  predicate end() { content = "" }
+  predicate end() { content = noContent() }
 
   /**
    * INTERNAL. DO NOT USE.
@@ -429,7 +468,7 @@ class TypeBackTracker extends TTypeBackTracker {
    *
    * This predicate is only defined if the type has not been tracked into a piece of content.
    */
-  TypeBackTracker continue() { content = "" and result = this }
+  TypeBackTracker continue() { content = noContent() and result = this }
 
   /**
    * Gets the summary that corresponds to having taken a backwards

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

@@ -11,6 +11,28 @@ class Node = DataFlowPublic::Node;
 
 class TypeTrackingNode = DataFlowPublic::TypeTrackingNode;
 
+/** A content name for use by type trackers, or the empty string. */
+class OptionalTypeTrackerContent extends string {
+  OptionalTypeTrackerContent() {
+    this = ""
+    or
+    this = getPossibleContentName()
+  }
+}
+
+/** A content name for use by type trackers. */
+class TypeTrackerContent extends OptionalTypeTrackerContent {
+  TypeTrackerContent() { this != "" }
+}
+
+/** Gets the content string representing no value. */
+OptionalTypeTrackerContent noContent() { result = "" }
+
+pragma[inline]
+predicate compatibleContents(TypeTrackerContent storeContent, TypeTrackerContent loadContent) {
+  storeContent = loadContent
+}
+
 predicate simpleLocalFlowStep = DataFlowPrivate::simpleLocalFlowStepForTypetracking/2;
 
 predicate jumpStep = DataFlowPrivate::jumpStepSharedWithTypeTracker/2;