Merge pull request #20915 from hvitved/content-flow-ap-limit

Shared: Improvements to content-sensitive model generation

shared/dataflow/codeql/dataflow/internal/ContentDataFlowImpl.qll

@@ -75,6 +75,9 @@ module MakeImplContentDataFlow<LocationSig Location, InputSig<Location> Lang> {
     /** Gets a limit on the number of reads out of sources and number of stores into sinks. */
     default int accessPathLimit() { result = Lang::accessPathLimit() }
 
+    /** Gets the access path limit used in the internal invocation of the standard data flow library. */
+    default int accessPathLimitInternal() { result = Lang::accessPathLimit() }
+
     /** Holds if `c` is relevant for reads out of sources or stores into sinks. */
     default predicate isRelevantContent(ContentSet c) { any() }
   }
@@ -110,7 +113,7 @@ module MakeImplContentDataFlow<LocationSig Location, InputSig<Location> Lang> {
 
       FlowFeature getAFeature() { result = ContentConfig::getAFeature() }
 
-      predicate accessPathLimit = ContentConfig::accessPathLimit/0;
+      predicate accessPathLimit = ContentConfig::accessPathLimitInternal/0;
 
       // needed to record reads/stores inside summarized callables
       predicate includeHiddenNodes() { any() }
@@ -274,6 +277,16 @@ module MakeImplContentDataFlow<LocationSig Location, InputSig<Location> Lang> {
         )
       }
 
+      /**
+       * Gets the length of this access path.
+       */
+      int length() {
+        this = TAccessPathNil() and
+        result = 0
+        or
+        result = this.getTail().length() + 1
+      }
+
       /**
        * Gets the content set at index `i` in this access path, if any.
        */

shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll

@@ -277,6 +277,16 @@ module MakeModelGeneratorFactory<
      */
     predicate isAdditionalContentFlowStep(Lang::Node nodeFrom, Lang::Node nodeTo);
 
+    /**
+     * Gets the access path limit for content flow analysis.
+     */
+    default int contentAccessPathLimit() { result = 2 }
+
+    /**
+     * Gets the internal access path limit for content flow analysis.
+     */
+    default int contentAccessPathLimitInternal() { result = Lang::accessPathLimit() }
+
     /**
      * Holds if the content set `c` is field like.
      */
@@ -650,7 +660,10 @@ module MakeModelGeneratorFactory<
           exists(Type t | t = n.(NodeExtended).getType() and not isRelevantType(t))
         }
 
-        int accessPathLimit() { result = 2 }
+        predicate accessPathLimit = SummaryModelGeneratorInput::contentAccessPathLimit/0;
+
+        predicate accessPathLimitInternal =
+          SummaryModelGeneratorInput::contentAccessPathLimitInternal/0;
 
         predicate isRelevantContent(DataFlow::ContentSet s) { isRelevantContent0(s) }
 
@@ -703,14 +716,17 @@ module MakeModelGeneratorFactory<
       }
 
       /**
-       * Holds if the access path `ap` is not a parameter or returnvalue of a callback
-       * stored in a field.
+       * Holds if `ap` is valid for generating summary models.
        *
-       * That is, we currently don't include summaries that rely on parameters or return values
-       * of callbacks stored in fields.
+       * We currently don't include summaries that rely on parameters or return values
+       * of callbacks stored in fields, as those are not supported by the data flow
+       * library.
+       *
+       * We also exclude access paths with contents not supported by `printContent`.
        */
       private predicate validateAccessPath(PropagateContentFlow::AccessPath ap) {
-        not (mentionsField(ap) and mentionsCallback(ap))
+        not (mentionsField(ap) and mentionsCallback(ap)) and
+        forall(int i | i in [0 .. ap.length() - 1] | exists(getContent(ap, i)))
       }
 
       private predicate apiFlow(
@@ -720,7 +736,9 @@ module MakeModelGeneratorFactory<
       ) {
         PropagateContentFlow::flow(p, reads, returnNodeExt, stores, preservesValue) and
         getEnclosingCallable(returnNodeExt) = api and
-        getEnclosingCallable(p) = api
+        getEnclosingCallable(p) = api and
+        validateAccessPath(reads) and
+        validateAccessPath(stores)
       }
 
       /**
@@ -763,9 +781,7 @@ module MakeModelGeneratorFactory<
         PropagateContentFlow::AccessPath reads, ReturnNodeExt returnNodeExt,
         PropagateContentFlow::AccessPath stores, boolean preservesValue
       ) {
-        PropagateContentFlow::flow(p, reads, returnNodeExt, stores, preservesValue) and
-        getEnclosingCallable(returnNodeExt) = api and
-        getEnclosingCallable(p) = api and
+        apiFlow(api, p, reads, returnNodeExt, stores, preservesValue) and
         p = api.getARelevantParameterNode()
       }
 
@@ -956,8 +972,6 @@ module MakeModelGeneratorFactory<
           input = parameterNodeAsExactInput(p) + printReadAccessPath(reads) and
           output = getExactOutput(returnNodeExt) + printStoreAccessPath(stores) and
           input != output and
-          validateAccessPath(reads) and
-          validateAccessPath(stores) and
           (
             if mentionsField(reads) or mentionsField(stores)
             then lift = false and api.isRelevant()