Only follow full DFG flow in followNextDFGEdgesUntilHit and similar (…

…#1473)

* Only follow full DFG flow in `followNextDFGEdgesUntilHit` and similar

We had a problem that the extension functions, such as `followNextDFGEdgesUntilHit` were following all DFG flows, including partial ones. This lead to unexpected results.

* Renamed prev/nextDFG to prev/nextFullDFG functions

---------

Co-authored-by: Konrad Weiss <[email protected]>

cpg-analysis/src/main/kotlin/de/fraunhofer/aisec/cpg/query/Query.kt

@@ -30,7 +30,6 @@ import de.fraunhofer.aisec.cpg.analysis.NumberSet
 import de.fraunhofer.aisec.cpg.analysis.SizeEvaluator
 import de.fraunhofer.aisec.cpg.analysis.ValueEvaluator
 import de.fraunhofer.aisec.cpg.graph.*
-import de.fraunhofer.aisec.cpg.graph.edge.FullDataflowGranularity
 import de.fraunhofer.aisec.cpg.graph.statements.expressions.CallExpression
 import de.fraunhofer.aisec.cpg.graph.statements.expressions.Expression
 import de.fraunhofer.aisec.cpg.graph.statements.expressions.Literal
@@ -202,7 +201,7 @@ fun dataFlow(
     findAllPossiblePaths: Boolean = true
 ): QueryTree<Boolean> {
     val evalRes =
-        from.followNextDFGEdgesUntilHit(collectFailedPaths, findAllPossiblePaths) { it == to }
+        from.followNextFullDFGEdgesUntilHit(collectFailedPaths, findAllPossiblePaths) { it == to }
     val allPaths = evalRes.fulfilled.map { QueryTree(it) }.toMutableList()
     if (collectFailedPaths) allPaths.addAll(evalRes.failed.map { QueryTree(it) })
     return QueryTree(
@@ -223,7 +222,7 @@ fun dataFlow(
     findAllPossiblePaths: Boolean = true
 ): QueryTree<Boolean> {
     val evalRes =
-        from.followNextDFGEdgesUntilHit(collectFailedPaths, findAllPossiblePaths, predicate)
+        from.followNextFullDFGEdgesUntilHit(collectFailedPaths, findAllPossiblePaths, predicate)
     val allPaths = evalRes.fulfilled.map { QueryTree(it) }.toMutableList()
     if (collectFailedPaths) allPaths.addAll(evalRes.failed.map { QueryTree(it) })
     return QueryTree(
@@ -381,12 +380,7 @@ fun allNonLiteralsFromFlowTo(from: Node, to: Node, allPaths: List<List<Node>>):
         else -> {
             // We go one step back to see if that one goes into to but also check that no assignment
             // to from happens in the paths between from and to
-            val prevQTs =
-                from.prevDFGEdges
-                    .filter { it.granularity is FullDataflowGranularity }
-                    .map { it.start }
-                    .map { dataFlow(it, to) }
-                    .toMutableSet()
+            val prevQTs = from.prevFullDFG.map { dataFlow(it, to) }.toMutableSet()
             // The base flows into a MemberExpression, but we don't care about such a partial
             // flow and are only interested in the prevDFG setting the field (if it exists). So, if
             // there are multiple edges, we filter out partial edges.

cpg-analysis/src/test/kotlin/de/fraunhofer/aisec/cpg/query/QueryTest.kt

@@ -448,7 +448,9 @@ class QueryTest {
                     max(it.subscriptExpression) <
                         min(
                             it.arrayExpression
-                                .followPrevDFGEdgesUntilHit { node -> node is NewArrayExpression }
+                                .followPrevFullDFGEdgesUntilHit { node ->
+                                    node is NewArrayExpression
+                                }
                                 .fulfilled
                                 .map { it2 -> (it2.last() as NewArrayExpression).dimensions[0] }
                         ) && min(it.subscriptExpression) > 0
@@ -462,7 +464,9 @@ class QueryTest {
                     (max(it.subscriptExpression) lt
                         min(
                             it.arrayExpression
-                                .followPrevDFGEdgesUntilHit { node -> node is NewArrayExpression }
+                                .followPrevFullDFGEdgesUntilHit { node ->
+                                    node is NewArrayExpression
+                                }
                                 .fulfilled
                                 .map { it2 -> (it2.last() as NewArrayExpression).dimensions[0] }
                         )) and (min(it.subscriptExpression) ge 0)

cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/Extensions.kt

@@ -27,6 +27,7 @@ package de.fraunhofer.aisec.cpg.graph
 
 import de.fraunhofer.aisec.cpg.TranslationResult
 import de.fraunhofer.aisec.cpg.graph.declarations.*
+import de.fraunhofer.aisec.cpg.graph.edge.FullDataflowGranularity
 import de.fraunhofer.aisec.cpg.graph.edge.Properties
 import de.fraunhofer.aisec.cpg.graph.edge.PropertyEdge
 import de.fraunhofer.aisec.cpg.graph.statements.IfStatement
@@ -218,28 +219,28 @@ class FulfilledAndFailedPaths(val fulfilled: List<List<Node>>, val failed: List<
 
 /**
  * Returns an instance of [FulfilledAndFailedPaths] where [FulfilledAndFailedPaths.fulfilled]
- * contains all possible shortest data flow paths between the end node [this] and the starting node
- * fulfilling [predicate]. The paths are represented as lists of nodes. Paths which do not end at
- * such a node are included in [FulfilledAndFailedPaths.failed].
+ * contains all possible shortest data flow paths (with [FullDataflowGranularity]) between the end
+ * node [this] and the starting node fulfilling [predicate]. The paths are represented as lists of
+ * nodes. Paths which do not end at such a node are included in [FulfilledAndFailedPaths.failed].
  *
  * Hence, if "fulfilled" is a non-empty list, a data flow from [this] to such a node is **possible
  * but not mandatory**. If the list "failed" is empty, the data flow is mandatory.
  */
-fun Node.followPrevDFGEdgesUntilHit(predicate: (Node) -> Boolean): FulfilledAndFailedPaths {
+fun Node.followPrevFullDFGEdgesUntilHit(predicate: (Node) -> Boolean): FulfilledAndFailedPaths {
     val fulfilledPaths = mutableListOf<List<Node>>()
     val failedPaths = mutableListOf<List<Node>>()
     val worklist = mutableListOf<List<Node>>()
     worklist.add(listOf(this))
 
     while (worklist.isNotEmpty()) {
         val currentPath = worklist.removeFirst()
-        if (currentPath.last().prevDFG.isEmpty()) {
+        if (currentPath.last().prevFullDFG.isEmpty()) {
             // No further nodes in the path and the path criteria are not satisfied.
             failedPaths.add(currentPath)
             continue
         }
 
-        for (prev in currentPath.last().prevDFG) {
+        for (prev in currentPath.last().prevFullDFG) {
             // Copy the path for each outgoing DFG edge and add the prev node
             val nextPath = mutableListOf<Node>()
             nextPath.addAll(currentPath)
@@ -262,14 +263,14 @@ fun Node.followPrevDFGEdgesUntilHit(predicate: (Node) -> Boolean): FulfilledAndF
 
 /**
  * Returns an instance of [FulfilledAndFailedPaths] where [FulfilledAndFailedPaths.fulfilled]
- * contains all possible shortest data flow paths between the starting node [this] and the end node
- * fulfilling [predicate]. The paths are represented as lists of nodes. Paths which do not end at
- * such a node are included in [FulfilledAndFailedPaths.failed].
+ * contains all possible shortest data flow paths (with [FullDataflowGranularity]) between the
+ * starting node [this] and the end node fulfilling [predicate]. The paths are represented as lists
+ * of nodes. Paths which do not end at such a node are included in [FulfilledAndFailedPaths.failed].
  *
  * Hence, if "fulfilled" is a non-empty list, a data flow from [this] to such a node is **possible
  * but not mandatory**. If the list "failed" is empty, the data flow is mandatory.
  */
-fun Node.followNextDFGEdgesUntilHit(
+fun Node.followNextFullDFGEdgesUntilHit(
     collectFailedPaths: Boolean = true,
     findAllPossiblePaths: Boolean = true,
     predicate: (Node) -> Boolean
@@ -292,13 +293,12 @@ fun Node.followNextDFGEdgesUntilHit(
         alreadySeenNodes.add(currentNode)
         // The last node of the path is where we continue. We get all of its outgoing DFG edges and
         // follow them
-        if (currentNode.nextDFG.isEmpty()) {
+        if (currentNode.nextFullDFG.isEmpty()) {
             // No further nodes in the path and the path criteria are not satisfied.
             if (collectFailedPaths) failedPaths.add(currentPath)
-            continue
         }
 
-        for (next in currentNode.nextDFG) {
+        for (next in currentNode.nextFullDFG) {
             // Copy the path for each outgoing DFG edge and add the next node
             val nextPath = currentPath.toMutableList()
             nextPath.add(next)
@@ -500,23 +500,23 @@ fun Node.followPrevEOG(predicate: (PropertyEdge<*>) -> Boolean): List<PropertyEd
 }
 
 /**
- * Returns a list of nodes which are a data flow path between the starting node [this] and the end
- * node fulfilling [predicate]. If the return value is not `null`, a data flow from [this] to such a
- * node is **possible but not mandatory**.
+ * Returns a list of nodes which are a data flow path (with [FullDataflowGranularity]) between the
+ * starting node [this] and the end node fulfilling [predicate]. If the return value is not `null`,
+ * a data flow from [this] to such a node is **possible but not mandatory**.
  *
  * It returns only a single possible path even if multiple paths are possible.
  */
-fun Node.followPrevDFG(predicate: (Node) -> Boolean): MutableList<Node>? {
+fun Node.followPrevFullDFG(predicate: (Node) -> Boolean): MutableList<Node>? {
     val path = mutableListOf<Node>()
 
-    for (prev in this.prevDFG) {
+    for (prev in this.prevFullDFG) {
         path.add(prev)
 
         if (predicate(prev)) {
             return path
         }
 
-        val subPath = prev.followPrevDFG(predicate)
+        val subPath = prev.followPrevFullDFG(predicate)
         if (subPath != null) {
             path.addAll(subPath)
         }

cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/Node.kt

@@ -172,6 +172,15 @@ open class Node : IVisitable<Node>, Persistable, LanguageProvider, ScopeProvider
     @PopulatedByPass(DFGPass::class, ControlFlowSensitiveDFGPass::class)
     var prevDFG: MutableSet<Node> by PropertyEdgeSetDelegate(Node::prevDFGEdges, false)
 
+    /** Virtual property for accessing [nextDFGEdges] that have a [FullDataflowGranularity]. */
+    @PopulatedByPass(DFGPass::class, ControlFlowSensitiveDFGPass::class)
+    val prevFullDFG: List<Node>
+        get() {
+            return prevDFGEdges
+                .filter { it.granularity is FullDataflowGranularity }
+                .map { it.start }
+        }
+
     /** Outgoing data flow edges */
     @PopulatedByPass(DFGPass::class, ControlFlowSensitiveDFGPass::class)
     @Relationship(value = "DFG", direction = Relationship.Direction.OUTGOING)
@@ -182,6 +191,13 @@ open class Node : IVisitable<Node>, Persistable, LanguageProvider, ScopeProvider
     @PopulatedByPass(DFGPass::class, ControlFlowSensitiveDFGPass::class)
     var nextDFG: MutableSet<Node> by PropertyEdgeSetDelegate(Node::nextDFGEdges, true)
 
+    /** Virtual property for accessing [nextDFGEdges] that have a [FullDataflowGranularity]. */
+    @PopulatedByPass(DFGPass::class, ControlFlowSensitiveDFGPass::class)
+    val nextFullDFG: List<Node>
+        get() {
+            return nextDFGEdges.filter { it.granularity is FullDataflowGranularity }.map { it.end }
+        }
+
     /** Outgoing Program Dependence Edges. */
     @PopulatedByPass(ProgramDependenceGraphPass::class)
     @Relationship(value = "PDG", direction = Relationship.Direction.OUTGOING)

cpg-core/src/test/kotlin/de/fraunhofer/aisec/cpg/graph/ShortcutsTest.kt

@@ -55,7 +55,7 @@ class ShortcutsTest {
                 ?.byNameOrNull<MethodDeclaration>("print")
 
         val (fulfilled, failed) =
-            toStringCall.followNextDFGEdgesUntilHit { it == printDecl?.parameters?.first() }
+            toStringCall.followNextFullDFGEdgesUntilHit { it == printDecl?.parameters?.first() }
 
         assertEquals(1, fulfilled.size)
         assertEquals(0, failed.size)
@@ -267,7 +267,7 @@ class ShortcutsTest {
                 .lhs
                 .first()
 
-        val paramPassed2 = aAssignment2.followPrevDFGEdgesUntilHit { it is Literal<*> }
+        val paramPassed2 = aAssignment2.followPrevFullDFGEdgesUntilHit { it is Literal<*> }
         assertEquals(1, paramPassed2.fulfilled.size)
         assertEquals(0, paramPassed2.failed.size)
         assertEquals(5, (paramPassed2.fulfilled[0].last() as? Literal<*>)?.value)
@@ -282,7 +282,7 @@ class ShortcutsTest {
                 .lhs
                 .first()
 
-        val paramPassed = attrAssignment.followPrevDFGEdgesUntilHit { it is Literal<*> }
+        val paramPassed = attrAssignment.followPrevFullDFGEdgesUntilHit { it is Literal<*> }
         assertEquals(1, paramPassed.fulfilled.size)
         assertEquals(0, paramPassed.failed.size)
         assertEquals(3, (paramPassed.fulfilled[0].last() as? Literal<*>)?.value)
@@ -346,7 +346,7 @@ class ShortcutsTest {
                 .lhs
                 .first()
 
-        val paramPassed = attrAssignment.followPrevDFG { it is Literal<*> }
+        val paramPassed = attrAssignment.followPrevFullDFG { it is Literal<*> }
         assertNotNull(paramPassed)
         assertEquals(3, (paramPassed.last() as? Literal<*>)?.value)
     }

cpg-language-cxx/src/test/kotlin/de/fraunhofer/aisec/cpg/frontends/cxx/CDataflowTest.kt

@@ -62,7 +62,7 @@ class CDataflowTest {
         // In this example we want to have the list of all fields of "ctx" that
         // are written to in the start function itself. For this to achieve we can follow the
         // "FULL" dfg flow until the end and collect partial writes on the way.
-        val result = startVariable.followNextDFGEdgesUntilHit { it.nextDFG.isEmpty() }
+        val result = startVariable.followNextFullDFGEdgesUntilHit { it.nextDFG.isEmpty() }
         val flow = result.fulfilled.singleOrNull()
         assertNotNull(flow)
         val fields =

cpg-language-cxx/src/test/kotlin/de/fraunhofer/aisec/cpg/frontends/cxx/CXXLanguageFrontendTest.kt

@@ -1365,7 +1365,7 @@ internal class CXXLanguageFrontendTest : BaseTest() {
         assertIs<Reference>(refCount)
         assertRefersTo(refCount, count)
 
-        var paths = addr.followPrevDFGEdgesUntilHit { it == refCount }
+        var paths = addr.followPrevFullDFGEdgesUntilHit { it == refCount }
         assertTrue(paths.fulfilled.isNotEmpty())
         assertTrue(paths.failed.isEmpty())
 
@@ -1374,7 +1374,7 @@ internal class CXXLanguageFrontendTest : BaseTest() {
 
         val assign = tu.assignments.firstOrNull { it.value is UnaryOperator }
         assertNotNull(assign)
-        paths = assign.value.followPrevDFGEdgesUntilHit { it == refKey }
+        paths = assign.value.followPrevFullDFGEdgesUntilHit { it == refKey }
         assertTrue(paths.fulfilled.isNotEmpty())
         assertTrue(paths.failed.isEmpty())
     }
@@ -1527,15 +1527,15 @@ internal class CXXLanguageFrontendTest : BaseTest() {
         val noParam = main.variables["no_param"]
         assertNotNull(noParam)
         assertTrue(
-            noParam.followPrevDFGEdgesUntilHit { it == targetNoParam }.fulfilled.isNotEmpty()
+            noParam.followPrevFullDFGEdgesUntilHit { it == targetNoParam }.fulfilled.isNotEmpty()
         )
 
         // ensure that our function pointer variable is connected to the method declaration via DFG
         val singleParam = main.variables["single_param"]
         assertNotNull(singleParam)
         assertTrue(
             singleParam
-                .followPrevDFGEdgesUntilHit { it == targetSingleParam }
+                .followPrevFullDFGEdgesUntilHit { it == targetSingleParam }
                 .fulfilled
                 .isNotEmpty()
         )

cpg-language-go/src/test/kotlin/de/fraunhofer/aisec/cpg/frontends/golang/ExpressionTest.kt

@@ -195,7 +195,7 @@ class ExpressionTest {
         val x = tu.refs("x").lastOrNull()
         assertNotNull(x)
 
-        val paths = x.followPrevDFGEdgesUntilHit { it == lit5 }
+        val paths = x.followPrevFullDFGEdgesUntilHit { it == lit5 }
         assertEquals(3, paths.fulfilled.firstOrNull()?.size)
 
         assertEquals(5, x.evaluate())

cpg-language-go/src/test/kotlin/de/fraunhofer/aisec/cpg/frontends/golang/GoLanguageFrontendTest.kt

@@ -96,7 +96,7 @@ class GoLanguageFrontendTest : BaseTest() {
 
         // We should be able to follow the DFG backwards from the declaration to the individual
         // key/value expressions
-        val path = data.firstAssignment?.followPrevDFG { it is KeyValueExpression }
+        val path = data.firstAssignment?.followPrevFullDFG { it is KeyValueExpression }
 
         assertNotNull(path)
         assertEquals(2, path.size)