From 355ccf42ee38d6855833e3042ebca3de0f596147 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alvaro=20Mu=C3=B1oz?= <pwntester@github.com>
Date: Mon, 5 Feb 2024 10:44:37 +0100
Subject: [PATCH] Do not compress local flow steps

Use `neverSkipPathGrap` to `any()` so no local flow steps get pruned
and thrown away in order to compress the presented dataflow path.
---
 .../internal/DataFlowImplSpecific.qll         |  5 +++-
 .../dataflow/internal/DataFlowPrivate.qll     | 28 +++++--------------
 .../Security/CWE-094/ExpressionInjection.ql   |  1 +
 3 files changed, 12 insertions(+), 22 deletions(-)

diff --git a/ql/lib/codeql/actions/dataflow/internal/DataFlowImplSpecific.qll b/ql/lib/codeql/actions/dataflow/internal/DataFlowImplSpecific.qll
index 4abb455b0dd..2d3b9696ef6 100644
--- a/ql/lib/codeql/actions/dataflow/internal/DataFlowImplSpecific.qll
+++ b/ql/lib/codeql/actions/dataflow/internal/DataFlowImplSpecific.qll
@@ -6,6 +6,9 @@
 private import codeql.dataflow.DataFlow
 
 module ActionsDataFlow implements InputSig {
-  import DataFlowPrivate
+  import DataFlowPrivate as Private
   import DataFlowPublic
+  import Private
+
+  predicate neverSkipInPathGraph = Private::neverSkipInPathGraph/1;
 }
diff --git a/ql/lib/codeql/actions/dataflow/internal/DataFlowPrivate.qll b/ql/lib/codeql/actions/dataflow/internal/DataFlowPrivate.qll
index b4abb3e8aa5..8b57ea2436e 100644
--- a/ql/lib/codeql/actions/dataflow/internal/DataFlowPrivate.qll
+++ b/ql/lib/codeql/actions/dataflow/internal/DataFlowPrivate.qll
@@ -207,27 +207,6 @@ predicate stepOutputDefToUse(Node nodeFrom, Node nodeTo) {
   )
 }
 
-predicate test1(UsesExpr u, string f, JobStmt j) {
-  u.getLocation().getFile().getBaseName() = "inter1.yml" and
-  f = u.getId() and
-  j = u.getJob()
-}
-
-predicate test2(StepOutputAccessExpr r, string f, JobStmt j) {
-  r.getLocation().getFile().getBaseName() = "inter1.yml" and
-  f = r.getStepId() and
-  j = r.getJob()
-}
-
-predicate test3(UsesExpr u, StepOutputAccessExpr r, Node n) {
-  r.getLocation().getFile().getBaseName() = "inter1.yml" and
-  u.getLocation().getFile().getBaseName() = "inter1.yml" and
-  u.getId() = r.getStepId() and
-  u.getJob() = r.getJob() and
-  // el SOAE has no mapping DF NODE
-  n.asExpr() = r
-}
-
 predicate jobOutputDefToUse(Node nodeFrom, Node nodeTo) {
   // nodeTo is a JobOutputAccessExpr and nodeFrom is the Job output expression
   exists(Expression astFrom, JobOutputAccessExpr astTo |
@@ -310,3 +289,10 @@ predicate lambdaCall(DataFlowCall call, LambdaCallKind kind, Node receiver) { no
 
 /** Extra data-flow steps needed for lambda flow analysis. */
 predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preservesValue) { none() }
+
+/**
+ * Since our model is so simple, we dont want to compress the local flow steps.
+ * This compression is normally done to not show SSA steps, casts, etc.
+ */
+predicate neverSkipInPathGraph(Node node) { any() }
+
diff --git a/ql/src/Security/CWE-094/ExpressionInjection.ql b/ql/src/Security/CWE-094/ExpressionInjection.ql
index f8d6e0c804b..4af1e2c286a 100644
--- a/ql/src/Security/CWE-094/ExpressionInjection.ql
+++ b/ql/src/Security/CWE-094/ExpressionInjection.ql
@@ -24,6 +24,7 @@ private module MyConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
 
   predicate isSink(DataFlow::Node sink) { sink instanceof ExpressionInjectionSink }
+  //predicate neverSkip(DataFlow::Node node) { any() }
 }
 
 module MyFlow = TaintTracking::Global<MyConfig>;