Merge pull request #9190 from hvitved/dataflow/summary-arg-param-no-materialize

aschackmull · web-flow · commit 1d3b3204df29 · 2022-05-18T09:17:57.000+02:00
Data flow: Do not materialize `summaryArgParam`
diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll
@@ -781,11 +781,12 @@ module Private {
       )
     }
 
-    pragma[nomagic]
-    private ParamNode summaryArgParam(ArgNode arg, ReturnKindExt rk, OutNodeExt out) {
-      exists(DataFlowCall call |
+    bindingset[ret]
+    private ParamNode summaryArgParam(ArgNode arg, ReturnNodeExt ret, OutNodeExt out) {
+      exists(DataFlowCall call, ReturnKindExt rk |
         result = summaryArgParam0(call, arg) and
-        out = rk.getAnOutNode(call)
+        pragma[only_bind_out](ret).getKind() = pragma[only_bind_into](rk) and
+        out = pragma[only_bind_into](rk).getAnOutNode(call)
       )
     }
 
@@ -797,9 +798,8 @@ module Private {
      * be useful to include in the exposed local data-flow/taint-tracking relations.
      */
     predicate summaryThroughStep(ArgNode arg, Node out, boolean preservesValue) {
-      exists(ReturnKindExt rk, ReturnNodeExt ret |
-        summaryLocalStep(summaryArgParam(arg, rk, out), ret, preservesValue) and
-        ret.getKind() = rk
+      exists(ReturnNodeExt ret |
+        summaryLocalStep(summaryArgParam(arg, ret, out), ret, preservesValue)
       )
     }
 
@@ -811,10 +811,9 @@ module Private {
      * be useful to include in the exposed local data-flow/taint-tracking relations.
      */
     predicate summaryGetterStep(ArgNode arg, ContentSet c, Node out) {
-      exists(ReturnKindExt rk, Node mid, ReturnNodeExt ret |
-        summaryReadStep(summaryArgParam(arg, rk, out), c, mid) and
-        summaryLocalStep(mid, ret, _) and
-        ret.getKind() = rk
+      exists(Node mid, ReturnNodeExt ret |
+        summaryReadStep(summaryArgParam(arg, ret, out), c, mid) and
+        summaryLocalStep(mid, ret, _)
       )
     }
 
@@ -826,10 +825,9 @@ module Private {
      * be useful to include in the exposed local data-flow/taint-tracking relations.
      */
     predicate summarySetterStep(ArgNode arg, ContentSet c, Node out) {
-      exists(ReturnKindExt rk, Node mid, ReturnNodeExt ret |
-        summaryLocalStep(summaryArgParam(arg, rk, out), mid, _) and
-        summaryStoreStep(mid, c, ret) and
-        ret.getKind() = rk
+      exists(Node mid, ReturnNodeExt ret |
+        summaryLocalStep(summaryArgParam(arg, ret, out), mid, _) and
+        summaryStoreStep(mid, c, ret)
       )
     }
   }
diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll b/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll
@@ -781,11 +781,12 @@ module Private {
       )
     }
 
-    pragma[nomagic]
-    private ParamNode summaryArgParam(ArgNode arg, ReturnKindExt rk, OutNodeExt out) {
-      exists(DataFlowCall call |
+    bindingset[ret]
+    private ParamNode summaryArgParam(ArgNode arg, ReturnNodeExt ret, OutNodeExt out) {
+      exists(DataFlowCall call, ReturnKindExt rk |
         result = summaryArgParam0(call, arg) and
-        out = rk.getAnOutNode(call)
+        pragma[only_bind_out](ret).getKind() = pragma[only_bind_into](rk) and
+        out = pragma[only_bind_into](rk).getAnOutNode(call)
       )
     }
 
@@ -797,9 +798,8 @@ module Private {
      * be useful to include in the exposed local data-flow/taint-tracking relations.
      */
     predicate summaryThroughStep(ArgNode arg, Node out, boolean preservesValue) {
-      exists(ReturnKindExt rk, ReturnNodeExt ret |
-        summaryLocalStep(summaryArgParam(arg, rk, out), ret, preservesValue) and
-        ret.getKind() = rk
+      exists(ReturnNodeExt ret |
+        summaryLocalStep(summaryArgParam(arg, ret, out), ret, preservesValue)
       )
     }
 
@@ -811,10 +811,9 @@ module Private {
      * be useful to include in the exposed local data-flow/taint-tracking relations.
      */
     predicate summaryGetterStep(ArgNode arg, ContentSet c, Node out) {
-      exists(ReturnKindExt rk, Node mid, ReturnNodeExt ret |
-        summaryReadStep(summaryArgParam(arg, rk, out), c, mid) and
-        summaryLocalStep(mid, ret, _) and
-        ret.getKind() = rk
+      exists(Node mid, ReturnNodeExt ret |
+        summaryReadStep(summaryArgParam(arg, ret, out), c, mid) and
+        summaryLocalStep(mid, ret, _)
       )
     }
 
@@ -826,10 +825,9 @@ module Private {
      * be useful to include in the exposed local data-flow/taint-tracking relations.
      */
     predicate summarySetterStep(ArgNode arg, ContentSet c, Node out) {
-      exists(ReturnKindExt rk, Node mid, ReturnNodeExt ret |
-        summaryLocalStep(summaryArgParam(arg, rk, out), mid, _) and
-        summaryStoreStep(mid, c, ret) and
-        ret.getKind() = rk
+      exists(Node mid, ReturnNodeExt ret |
+        summaryLocalStep(summaryArgParam(arg, ret, out), mid, _) and
+        summaryStoreStep(mid, c, ret)
       )
     }
   }
diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll
@@ -781,11 +781,12 @@ module Private {
       )
     }
 
-    pragma[nomagic]
-    private ParamNode summaryArgParam(ArgNode arg, ReturnKindExt rk, OutNodeExt out) {
-      exists(DataFlowCall call |
+    bindingset[ret]
+    private ParamNode summaryArgParam(ArgNode arg, ReturnNodeExt ret, OutNodeExt out) {
+      exists(DataFlowCall call, ReturnKindExt rk |
         result = summaryArgParam0(call, arg) and
-        out = rk.getAnOutNode(call)
+        pragma[only_bind_out](ret).getKind() = pragma[only_bind_into](rk) and
+        out = pragma[only_bind_into](rk).getAnOutNode(call)
       )
     }
 
@@ -797,9 +798,8 @@ module Private {
      * be useful to include in the exposed local data-flow/taint-tracking relations.
      */
     predicate summaryThroughStep(ArgNode arg, Node out, boolean preservesValue) {
-      exists(ReturnKindExt rk, ReturnNodeExt ret |
-        summaryLocalStep(summaryArgParam(arg, rk, out), ret, preservesValue) and
-        ret.getKind() = rk
+      exists(ReturnNodeExt ret |
+        summaryLocalStep(summaryArgParam(arg, ret, out), ret, preservesValue)
       )
     }
 
@@ -811,10 +811,9 @@ module Private {
      * be useful to include in the exposed local data-flow/taint-tracking relations.
      */
     predicate summaryGetterStep(ArgNode arg, ContentSet c, Node out) {
-      exists(ReturnKindExt rk, Node mid, ReturnNodeExt ret |
-        summaryReadStep(summaryArgParam(arg, rk, out), c, mid) and
-        summaryLocalStep(mid, ret, _) and
-        ret.getKind() = rk
+      exists(Node mid, ReturnNodeExt ret |
+        summaryReadStep(summaryArgParam(arg, ret, out), c, mid) and
+        summaryLocalStep(mid, ret, _)
       )
     }
 
@@ -826,10 +825,9 @@ module Private {
      * be useful to include in the exposed local data-flow/taint-tracking relations.
      */
     predicate summarySetterStep(ArgNode arg, ContentSet c, Node out) {
-      exists(ReturnKindExt rk, Node mid, ReturnNodeExt ret |
-        summaryLocalStep(summaryArgParam(arg, rk, out), mid, _) and
-        summaryStoreStep(mid, c, ret) and
-        ret.getKind() = rk
+      exists(Node mid, ReturnNodeExt ret |
+        summaryLocalStep(summaryArgParam(arg, ret, out), mid, _) and
+        summaryStoreStep(mid, c, ret)
       )
     }
   }

Original file line number	Diff line number	Diff line change
`@@ -781,11 +781,12 @@ module Private {`
`781`	`781`	`)`
`782`	`782`	`}`
`783`	`783`
`784`		`- pragma[nomagic]`
`785`		`- private ParamNode summaryArgParam(ArgNode arg, ReturnKindExt rk, OutNodeExt out) {`
`786`		`- exists(DataFlowCall call \|`
	`784`	`+ bindingset[ret]`
	`785`	`+ private ParamNode summaryArgParam(ArgNode arg, ReturnNodeExt ret, OutNodeExt out) {`
	`786`	`+ exists(DataFlowCall call, ReturnKindExt rk \|`
`787`	`787`	`result = summaryArgParam0(call, arg) and`
`788`		`- out = rk.getAnOutNode(call)`
	`788`	`+ pragma[only_bind_out](ret).getKind() = pragma[only_bind_into](rk) and`
	`789`	`+ out = pragma[only_bind_into](rk).getAnOutNode(call)`
`789`	`790`	`)`
`790`	`791`	`}`
`791`	`792`
`@@ -797,9 +798,8 @@ module Private {`
`797`	`798`	`* be useful to include in the exposed local data-flow/taint-tracking relations.`
`798`	`799`	`*/`
`799`	`800`	`predicate summaryThroughStep(ArgNode arg, Node out, boolean preservesValue) {`
`800`		`- exists(ReturnKindExt rk, ReturnNodeExt ret \|`
`801`		`- summaryLocalStep(summaryArgParam(arg, rk, out), ret, preservesValue) and`
`802`		`- ret.getKind() = rk`
	`801`	`+ exists(ReturnNodeExt ret \|`
	`802`	`+ summaryLocalStep(summaryArgParam(arg, ret, out), ret, preservesValue)`
`803`	`803`	`)`
`804`	`804`	`}`
`805`	`805`
`@@ -811,10 +811,9 @@ module Private {`
`811`	`811`	`* be useful to include in the exposed local data-flow/taint-tracking relations.`
`812`	`812`	`*/`
`813`	`813`	`predicate summaryGetterStep(ArgNode arg, ContentSet c, Node out) {`
`814`		`- exists(ReturnKindExt rk, Node mid, ReturnNodeExt ret \|`
`815`		`- summaryReadStep(summaryArgParam(arg, rk, out), c, mid) and`
`816`		`- summaryLocalStep(mid, ret, _) and`
`817`		`- ret.getKind() = rk`
	`814`	`+ exists(Node mid, ReturnNodeExt ret \|`
	`815`	`+ summaryReadStep(summaryArgParam(arg, ret, out), c, mid) and`
	`816`	`+ summaryLocalStep(mid, ret, _)`
`818`	`817`	`)`
`819`	`818`	`}`
`820`	`819`
`@@ -826,10 +825,9 @@ module Private {`
`826`	`825`	`* be useful to include in the exposed local data-flow/taint-tracking relations.`
`827`	`826`	`*/`
`828`	`827`	`predicate summarySetterStep(ArgNode arg, ContentSet c, Node out) {`
`829`		`- exists(ReturnKindExt rk, Node mid, ReturnNodeExt ret \|`
`830`		`- summaryLocalStep(summaryArgParam(arg, rk, out), mid, _) and`
`831`		`- summaryStoreStep(mid, c, ret) and`
`832`		`- ret.getKind() = rk`
	`828`	`+ exists(Node mid, ReturnNodeExt ret \|`
	`829`	`+ summaryLocalStep(summaryArgParam(arg, ret, out), mid, _) and`
	`830`	`+ summaryStoreStep(mid, c, ret)`
`833`	`831`	`)`
`834`	`832`	`}`
`835`	`833`	`}`