From 9acb58e8c270b17c6171e6ad8776e836d76eac80 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Fri, 21 Feb 2025 14:43:51 +0100 Subject: [PATCH 01/20] SSA: Add SsaNode predicates that don't mention DefinitionExt. --- shared/ssa/codeql/ssa/Ssa.qll | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/shared/ssa/codeql/ssa/Ssa.qll b/shared/ssa/codeql/ssa/Ssa.qll index 407169cc6be7..41219991c33e 100644 --- a/shared/ssa/codeql/ssa/Ssa.qll +++ b/shared/ssa/codeql/ssa/Ssa.qll @@ -1715,6 +1715,15 @@ module Make Input> { abstract private class SsaNodeImpl extends NodeImpl { /** Gets the underlying SSA definition. */ abstract DefinitionExt getDefinitionExt(); + + /** Gets the SSA definition this node corresponds to, if any. */ + Definition asDefinition() { this = TSsaDefinitionNode(result) } + + /** Gets the basic block to which this node belongs. */ + abstract BasicBlock getBasicBlock(); + + /** Gets the underlying source variable that this node tracks flow for. */ + abstract SourceVariable getSourceVariable(); } final class SsaNode = SsaNodeImpl; @@ -1727,6 +1736,10 @@ module Make Input> { override DefinitionExt getDefinitionExt() { result = def } + override BasicBlock getBasicBlock() { result = def.getBasicBlock() } + + override SourceVariable getSourceVariable() { result = def.getSourceVariable() } + override Location getLocation() { result = def.getLocation() } override string toString() { result = def.toString() } @@ -1783,6 +1796,10 @@ module Make Input> { override SsaInputDefinitionExt getDefinitionExt() { result = def_ } + override BasicBlock getBasicBlock() { result = input_ } + + override SourceVariable getSourceVariable() { result = def_.getSourceVariable() } + override Location getLocation() { result = input_.getNode(input_.length() - 1).getLocation() } override string toString() { result = "[input] " + def_.toString() } From 4e515bc2f553aff1724351d3ee0c6bd31fde0add Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Fri, 21 Feb 2025 14:48:24 +0100 Subject: [PATCH 02/20] JS: Remove reference to isInputInto --- .../javascript/dataflow/internal/TaintTrackingPrivate.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/TaintTrackingPrivate.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/TaintTrackingPrivate.qll index 5ba7cddf0971..7b92934e09cd 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/TaintTrackingPrivate.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/TaintTrackingPrivate.qll @@ -41,7 +41,7 @@ pragma[inline_late] private BasicBlock getBasicBlockFromSsa2(Ssa2::Node node) { result = node.(Ssa2::ExprNode).getExpr().getBasicBlock() or - node.(Ssa2::SsaInputNode).isInputInto(_, result) + result = node.(Ssa2::SsaInputNode).getBasicBlock() } /** From 09b2aeb53a2eb8815a0fa99ad2908301df77922c Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Mon, 24 Feb 2025 10:58:14 +0100 Subject: [PATCH 03/20] SSA: Replace use-use step implementation in data-flow integration. --- .../dataflow/internal/DataFlowPrivate.qll | 4 +- shared/ssa/codeql/ssa/Ssa.qll | 106 +++++++----------- 2 files changed, 42 insertions(+), 68 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll index d170b8a3c1e3..3511666c8a18 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll @@ -1270,8 +1270,8 @@ Node getNodeFromSsa2(Ssa2::Node node) { } private predicate useUseFlow(Node node1, Node node2) { - exists(Ssa2::DefinitionExt def, Ssa2::Node ssa1, Ssa2::Node ssa2 | - Ssa2::localFlowStep(def, ssa1, ssa2, _) and + exists(Ssa2::Node ssa1, Ssa2::Node ssa2 | + Ssa2::localFlowStep(_, ssa1, ssa2, _) and node1 = getNodeFromSsa2(ssa1) and node2 = getNodeFromSsa2(ssa2) and not node1.getTopLevel().isExterns() diff --git a/shared/ssa/codeql/ssa/Ssa.qll b/shared/ssa/codeql/ssa/Ssa.qll index 41219991c33e..8fd1599f7828 100644 --- a/shared/ssa/codeql/ssa/Ssa.qll +++ b/shared/ssa/codeql/ssa/Ssa.qll @@ -1555,24 +1555,6 @@ module Make Input> { ) } - /** Same as `adjacentDefReadExt`, but skips uncertain reads. */ - pragma[nomagic] - private predicate adjacentDefSkipUncertainReadsExt( - DefinitionExt def, SourceVariable v, BasicBlock bb1, int i1, BasicBlock bb2, int i2 - ) { - adjacentDefReachesReadExt(def, v, bb1, i1, bb2, i2) and - variableRead(bb2, i2, v, true) - } - - pragma[nomagic] - private predicate adjacentReadPairExt(DefinitionExt def, ReadNode read1, ReadNode read2) { - exists(SourceVariable v, BasicBlock bb1, int i1, BasicBlock bb2, int i2 | - read1.readsAt(bb1, i1, v) and - adjacentDefSkipUncertainReadsExt(def, v, bb1, i1, bb2, i2) and - read2.readsAt(bb2, i2, v) - ) - } - final private class DefinitionExtFinal = DefinitionExt; /** An SSA definition into which another SSA definition may flow. */ @@ -1808,41 +1790,22 @@ module Make Input> { final class SsaInputNode = SsaInputNodeImpl; /** - * Holds if `nodeFrom` is a node for SSA definition `def`, which can input - * node `nodeTo`. + * Holds if `nodeFrom` corresponds to the reference to `v` at index `i` in + * `bb`. The boolean `isUseStep` indicates whether `nodeFrom` is an actual + * read. If it is false then `nodeFrom` may be any of the following: an + * uncertain write, a certain write, a phi, or a phi read. `def` is the SSA + * definition that is read/defined at `nodeFrom`. */ - pragma[nomagic] - private predicate inputFromDef( - DefinitionExt def, SsaDefinitionExtNode nodeFrom, SsaInputNode nodeTo + private predicate flowOutOf( + DefinitionExt def, Node nodeFrom, SourceVariable v, BasicBlock bb, int i, boolean isUseStep ) { - exists(SourceVariable v, BasicBlock bb, int i, BasicBlock input, SsaInputDefinitionExt next | - next.hasInputFromBlock(def, v, bb, i, input) and - def = nodeFrom.getDefinitionExt() and - def.definesAt(v, bb, i, _) and - nodeTo = TSsaInputNode(next, input) - ) - } - - /** - * Holds if `nodeFrom` is a last read of SSA definition `def`, which - * can reach input node `nodeTo`. - */ - pragma[nomagic] - private predicate inputFromRead(DefinitionExt def, ReadNode nodeFrom, SsaInputNode nodeTo) { - exists(SourceVariable v, BasicBlock bb, int i, BasicBlock input, SsaInputDefinitionExt next | - next.hasInputFromBlock(def, v, bb, i, input) and - nodeFrom.readsAt(bb, i, v) and - nodeTo = TSsaInputNode(next, input) - ) - } - - pragma[nomagic] - private predicate firstReadExt(DefinitionExt def, ReadNode read) { - exists(SourceVariable v, BasicBlock bb1, int i1, BasicBlock bb2, int i2 | - def.definesAt(v, bb1, i1, _) and - adjacentDefSkipUncertainReadsExt(def, v, bb1, i1, bb2, i2) and - read.readsAt(bb2, i2, v) - ) + nodeFrom.(SsaDefinitionExtNode).getDefinitionExt() = def and + def.definesAt(v, bb, i, _) and + isUseStep = false + or + ssaDefReachesReadExt(v, def, bb, i) and + [nodeFrom, nodeFrom.(ExprPostUpdateNode).getPreUpdateNode()].(ReadNode).readsAt(bb, i, v) and + isUseStep = true } /** @@ -1862,23 +1825,34 @@ module Make Input> { nodeTo.(SsaDefinitionExtNode).getDefinitionExt() = def and isUseStep = false or - // Flow from SSA definition to first read - def = nodeFrom.(SsaDefinitionExtNode).getDefinitionExt() and - firstReadExt(def, nodeTo) and - isUseStep = false - or - // Flow from (post-update) read to next read - adjacentReadPairExt(def, [nodeFrom, nodeFrom.(ExprPostUpdateNode).getPreUpdateNode()], nodeTo) and - nodeFrom != nodeTo and - isUseStep = true + // Flow from definition/read to next read + exists(SourceVariable v, BasicBlock bb1, int i1, BasicBlock bb2, int i2 | + flowOutOf(def, nodeFrom, v, bb1, i1, isUseStep) and + AdjacentSsaRefs::adjacentRefRead(bb1, i1, bb2, i2, v) and + nodeTo.(ReadNode).readsAt(bb2, i2, v) + ) or - // Flow into phi (read) SSA definition node from def - inputFromDef(def, nodeFrom, nodeTo) and - isUseStep = false + // Flow from definition/read to next uncertain write + exists(SourceVariable v, BasicBlock bb1, int i1, BasicBlock bb2, int i2 | + flowOutOf(def, nodeFrom, v, bb1, i1, isUseStep) and + AdjacentSsaRefs::adjacentRefRead(bb1, i1, bb2, i2, v) and + exists(UncertainWriteDefinition def2 | + DfInput::allowFlowIntoUncertainDef(def2) and + nodeTo.(SsaDefinitionExtNode).getDefinitionExt() = def2 and + def2.definesAt(v, bb2, i2) + ) + ) or - // Flow into phi (read) SSA definition node from (post-update) read - inputFromRead(def, [nodeFrom, nodeFrom.(ExprPostUpdateNode).getPreUpdateNode()], nodeTo) and - isUseStep = true + // Flow from definition/read to phi input + exists( + SourceVariable v, BasicBlock bb, int i, BasicBlock input, BasicBlock bbPhi, + DefinitionExt phi + | + flowOutOf(def, nodeFrom, v, bb, i, isUseStep) and + AdjacentSsaRefs::adjacentRefPhi(bb, i, input, bbPhi, v) and + nodeTo = TSsaInputNode(phi, input) and + phi.definesAt(v, bbPhi, -1, _) + ) or // Flow from input node to def nodeTo.(SsaDefinitionExtNode).getDefinitionExt() = def and From 88fe4faf9df571eccf9701335539f9a879c0ab88 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Mon, 24 Feb 2025 11:00:48 +0100 Subject: [PATCH 04/20] SSA: Remove nodes that are no longer used. --- shared/ssa/codeql/ssa/Ssa.qll | 2 -- 1 file changed, 2 deletions(-) diff --git a/shared/ssa/codeql/ssa/Ssa.qll b/shared/ssa/codeql/ssa/Ssa.qll index 8fd1599f7828..dd7a11098f2b 100644 --- a/shared/ssa/codeql/ssa/Ssa.qll +++ b/shared/ssa/codeql/ssa/Ssa.qll @@ -1563,8 +1563,6 @@ module Make Input> { this instanceof PhiNode or this instanceof PhiReadNode - or - DfInput::allowFlowIntoUncertainDef(this) } /** Holds if `def` may flow into this definition via basic block `input`. */ From 782b6cfb9a2785d2f2b1b09a1a3b16c9b24e049b Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Mon, 24 Feb 2025 12:40:45 +0100 Subject: [PATCH 05/20] SSA: Fix bug in guards for ssa input nodes. --- shared/ssa/codeql/ssa/Ssa.qll | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/shared/ssa/codeql/ssa/Ssa.qll b/shared/ssa/codeql/ssa/Ssa.qll index dd7a11098f2b..9c971b6d6f49 100644 --- a/shared/ssa/codeql/ssa/Ssa.qll +++ b/shared/ssa/codeql/ssa/Ssa.qll @@ -1574,8 +1574,12 @@ module Make Input> { } cached - private DefinitionExt getAPhiInputDef(SsaInputDefinitionExt phi, BasicBlock bb) { - phi.hasInputFromBlock(result, _, _, _, bb) + private Definition getAPhiInputDef(SsaInputDefinitionExt phi, BasicBlock bb) { + exists(SourceVariable v, BasicBlock bbDef | + phi.definesAt(v, bbDef, _, _) and + getABasicBlockPredecessor(bbDef) = bb and + ssaDefReachesEndOfBlock(bb, result, v) + ) } private newtype TNode = From 1af753cd0ced8f8ec28ffe3e2004d3144a59b135 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Mon, 24 Feb 2025 12:43:35 +0100 Subject: [PATCH 06/20] JS: Use shared barrier guard for falsy check. --- .../internal/TaintTrackingPrivate.qll | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/TaintTrackingPrivate.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/TaintTrackingPrivate.qll index 7b92934e09cd..4bb07df9a875 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/TaintTrackingPrivate.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/TaintTrackingPrivate.qll @@ -36,12 +36,16 @@ predicate defaultAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2, defaultAdditionalTaintStep(node1, node2) and model = "" // TODO: set model } -bindingset[node] -pragma[inline_late] -private BasicBlock getBasicBlockFromSsa2(Ssa2::Node node) { - result = node.(Ssa2::ExprNode).getExpr().getBasicBlock() - or - result = node.(Ssa2::SsaInputNode).getBasicBlock() +private predicate guardChecksFalsy( + Ssa2::SsaDataflowInput::Guard g, Ssa2::SsaDataflowInput::Expr e, boolean outcome +) { + exists(ConditionGuardNode guard | + guard.getTest() = g and + guard.getOutcome() = outcome and + e = g and + e instanceof VarAccess and + outcome = false + ) } /** @@ -64,13 +68,7 @@ private BasicBlock getBasicBlockFromSsa2(Ssa2::Node node) { * ``` */ private predicate varAccessBarrier(DataFlow::Node node) { - exists(ConditionGuardNode guard, Ssa2::ExprNode nodeFrom, Ssa2::Node nodeTo | - guard.getOutcome() = false and - guard.getTest().(VarAccess) = nodeFrom.getExpr() and - Ssa2::localFlowStep(_, nodeFrom, nodeTo, true) and - guard.dominates(getBasicBlockFromSsa2(nodeTo)) and - node = getNodeFromSsa2(nodeTo) - ) + getNodeFromSsa2(Ssa2::BarrierGuard::getABarrierNode()) = node } /** From 09454f9f14b16793bba8f43678d33a6d7d61603b Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Mon, 24 Feb 2025 13:02:03 +0100 Subject: [PATCH 07/20] SSA: Remove unused. --- shared/ssa/codeql/ssa/Ssa.qll | 54 ----------------------------------- 1 file changed, 54 deletions(-) diff --git a/shared/ssa/codeql/ssa/Ssa.qll b/shared/ssa/codeql/ssa/Ssa.qll index 9c971b6d6f49..9eda86e1dcab 100644 --- a/shared/ssa/codeql/ssa/Ssa.qll +++ b/shared/ssa/codeql/ssa/Ssa.qll @@ -1508,53 +1508,6 @@ module Make Input> { module DataFlowIntegration { private import codeql.util.Boolean - pragma[nomagic] - private predicate adjacentDefReachesReadExt( - DefinitionExt def, SourceVariable v, BasicBlock bb1, int i1, BasicBlock bb2, int i2 - ) { - adjacentDefReadExt(def, v, bb1, i1, bb2, i2) and - ( - def.definesAt(v, bb1, i1, _) - or - variableRead(bb1, i1, v, true) - ) - or - exists(BasicBlock bb3, int i3 | - adjacentDefReachesReadExt(def, v, bb1, i1, bb3, i3) and - variableRead(bb3, i3, v, false) and - adjacentDefReadExt(def, v, bb3, i3, bb2, i2) - ) - } - - pragma[nomagic] - private predicate adjacentDefReachesUncertainReadExt( - DefinitionExt def, SourceVariable v, BasicBlock bb1, int i1, BasicBlock bb2, int i2 - ) { - adjacentDefReachesReadExt(def, v, bb1, i1, bb2, i2) and - variableRead(bb2, i2, v, false) - } - - /** - * Holds if the reference to `def` at index `i` in basic block `bb` can reach - * another definition `next` of the same underlying source variable, without - * passing through another write or non-pseudo read. - * - * The reference is either a read of `def` or `def` itself. - */ - pragma[nomagic] - private predicate lastRefBeforeRedefExt( - DefinitionExt def, SourceVariable v, BasicBlock bb, int i, BasicBlock input, - DefinitionExt next - ) { - lastRefRedefExt(def, v, bb, i, input, next) and - not variableRead(bb, i, v, false) - or - exists(BasicBlock bb0, int i0 | - lastRefRedefExt(def, v, bb0, i0, input, next) and - adjacentDefReachesUncertainReadExt(def, v, bb, i, bb0, i0) - ) - } - final private class DefinitionExtFinal = DefinitionExt; /** An SSA definition into which another SSA definition may flow. */ @@ -1564,13 +1517,6 @@ module Make Input> { or this instanceof PhiReadNode } - - /** Holds if `def` may flow into this definition via basic block `input`. */ - predicate hasInputFromBlock( - DefinitionExt def, SourceVariable v, BasicBlock bb, int i, BasicBlock input - ) { - lastRefBeforeRedefExt(def, v, bb, i, input, this) - } } cached From db7ec4a781cb505c9055d8cf3875da0e763c5655 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Mon, 24 Feb 2025 13:50:08 +0100 Subject: [PATCH 08/20] Java: Remove getDefinitionExt reference --- .../semmle/code/java/dataflow/internal/DataFlowNodes.qll | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll index df9a487a2c1f..fd14bcfd100c 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll @@ -167,7 +167,7 @@ module Public { or result instanceof TypeObject and this instanceof AdditionalNode or - result = this.(SsaNode).getDefinitionExt().getSourceVariable().getType() + result = this.(SsaNode).getTypeImpl() } /** Gets the callable in which this node occurs. */ @@ -394,7 +394,9 @@ class SsaNode extends Node, TSsaNode { SsaNode() { this = TSsaNode(node) } - SsaImpl::Impl::DefinitionExt getDefinitionExt() { result = node.getDefinitionExt() } + BasicBlock getBasicBlock() { result = node.getBasicBlock() } + + Type getTypeImpl() { result = node.getSourceVariable().getType() } override Location getLocation() { result = node.getLocation() } @@ -442,7 +444,7 @@ module Private { result.asCallable() = n.(CaptureNode).getSynthesizedCaptureNode().getEnclosingCallable() or result.asFieldScope() = n.(FieldValueNode).getField() or result.asCallable() = any(Expr e | n.(AdditionalNode).nodeAt(e, _)).getEnclosingCallable() or - result.asCallable() = n.(SsaNode).getDefinitionExt().getBasicBlock().getEnclosingCallable() + result.asCallable() = n.(SsaNode).getBasicBlock().getEnclosingCallable() } /** Holds if `p` is a `ParameterNode` of `c` with position `pos`. */ From 0583d85f201320362d5ec3e8af5b757ef88f57ee Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Mon, 24 Feb 2025 14:18:34 +0100 Subject: [PATCH 09/20] C#: Remove getDefinitionExt references. --- .../dataflow/internal/DataFlowPrivate.qll | 79 ++++--------------- shared/ssa/codeql/ssa/Ssa.qll | 11 +++ 2 files changed, 28 insertions(+), 62 deletions(-) diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll index d8c57d4a3c16..6c20ec294a68 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll @@ -664,7 +664,7 @@ module LocalFlow { ssaDef.getADefinition() = def and ssaDef.getControlFlowNode() = cfn and nodeFrom = TAssignableDefinitionNode(def, cfn) and - nodeTo.(SsaDefinitionExtNode).getDefinitionExt() = ssaDef + nodeTo.(SsaDefinitionNode).getDefinition() = ssaDef ) } @@ -1269,78 +1269,33 @@ predicate nodeIsHidden(Node n) { } /** An SSA node. */ -abstract class SsaNode extends NodeImpl, TSsaNode { +class SsaNode extends NodeImpl, TSsaNode { SsaImpl::DataFlowIntegration::SsaNode node; - SsaImpl::DefinitionExt def; - SsaNode() { - this = TSsaNode(node) and - def = node.getDefinitionExt() - } - - SsaImpl::DefinitionExt getDefinitionExt() { result = def } + SsaNode() { this = TSsaNode(node) } override DataFlowCallable getEnclosingCallableImpl() { - result.getAControlFlowNode().getBasicBlock() = def.getBasicBlock() + result.getAControlFlowNode().getBasicBlock() = node.getBasicBlock() } - override Type getTypeImpl() { result = def.getSourceVariable().getType() } + override Type getTypeImpl() { result = node.getSourceVariable().getType() } - override ControlFlow::Node getControlFlowNodeImpl() { - result = def.(Ssa::Definition).getControlFlowNode() - } + override ControlFlow::Node getControlFlowNodeImpl() { none() } override Location getLocationImpl() { result = node.getLocation() } override string toStringImpl() { result = node.toString() } } -/** An (extended) SSA definition, viewed as a node in a data flow graph. */ -class SsaDefinitionExtNode extends SsaNode { - override SsaImpl::DataFlowIntegration::SsaDefinitionExtNode node; -} +/** An SSA definition, viewed as a node in a data flow graph. */ +class SsaDefinitionNode extends SsaNode { + override SsaImpl::DataFlowIntegration::SsaDefinitionNode node; -/** - * A node that represents an input to an SSA phi (read) definition. - * - * This allows for barrier guards to filter input to phi nodes. For example, in - * - * ```csharp - * var x = taint; - * if (x != "safe") - * { - * x = "safe"; - * } - * sink(x); - * ``` - * - * the `false` edge out of `x != "safe"` guards the input from `x = taint` into the - * `phi` node after the condition. - * - * It is also relevant to filter input into phi read nodes: - * - * ```csharp - * var x = taint; - * if (b) - * { - * if (x != "safe1") - * { - * return; - * } - * } else { - * if (x != "safe2") - * { - * return; - * } - * } - * - * sink(x); - * ``` - * - * both inputs into the phi read node after the outer condition are guarded. - */ -class SsaInputNode extends SsaNode { - override SsaImpl::DataFlowIntegration::SsaInputNode node; + SsaImpl::Definition getDefinition() { result = node.getDefinition() } + + override ControlFlow::Node getControlFlowNodeImpl() { + result = this.getDefinition().(Ssa::Definition).getControlFlowNode() + } } /** A definition, viewed as a node in a data flow graph. */ @@ -1728,12 +1683,12 @@ private module ReturnNodes { * A data-flow node that represents an assignment to an `out` or a `ref` * parameter. */ - class OutRefReturnNode extends ReturnNode, SsaDefinitionExtNode { + class OutRefReturnNode extends ReturnNode, SsaDefinitionNode { OutRefReturnKind kind; OutRefReturnNode() { exists(Parameter p | - this.getDefinitionExt().(Ssa::Definition).isLiveOutRefParameterDefinition(p) and + this.getDefinition().(Ssa::Definition).isLiveOutRefParameterDefinition(p) and kind.getPosition() = p.getPosition() | p.isOut() and kind instanceof OutReturnKind @@ -2464,7 +2419,7 @@ private predicate readContentStep(Node node1, Content c, Node node2) { exists(ForeachStmt fs, Ssa::ExplicitDefinition def | x.hasDefPath(fs.getIterableExpr(), node1.getControlFlowNode(), def.getADefinition(), def.getControlFlowNode()) and - node2.(SsaDefinitionExtNode).getDefinitionExt() = def and + node2.(SsaDefinitionNode).getDefinition() = def and c instanceof ElementContent ) or diff --git a/shared/ssa/codeql/ssa/Ssa.qll b/shared/ssa/codeql/ssa/Ssa.qll index 9eda86e1dcab..e77946a90a63 100644 --- a/shared/ssa/codeql/ssa/Ssa.qll +++ b/shared/ssa/codeql/ssa/Ssa.qll @@ -1677,6 +1677,17 @@ module Make Input> { final class SsaDefinitionExtNode = SsaDefinitionExtNodeImpl; + /** An SSA definition, viewed as a node in a data flow graph. */ + private class SsaDefinitionNodeImpl extends SsaDefinitionExtNodeImpl { + private Definition def; + + SsaDefinitionNodeImpl() { this = TSsaDefinitionNode(def) } + + Definition getDefinition() { result = def } + } + + final class SsaDefinitionNode = SsaDefinitionNodeImpl; + /** * A node that represents an input to an SSA phi (read) definition. * From 7499df43d0f61d95626e251f0c90686f442eff50 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Mon, 24 Feb 2025 14:27:17 +0100 Subject: [PATCH 10/20] Rust: Remove getDefinitionExt reference. --- .../rust/dataflow/internal/DataFlowImpl.qll | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll index 11c24bba17ce..6d237d786beb 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll +++ b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll @@ -405,16 +405,15 @@ module Node { /** An SSA node. */ class SsaNode extends Node, TSsaNode { SsaImpl::DataFlowIntegration::SsaNode node; - SsaImpl::DefinitionExt def; - SsaNode() { - this = TSsaNode(node) and - def = node.getDefinitionExt() - } + SsaNode() { this = TSsaNode(node) } - override CfgScope getCfgScope() { result = def.getBasicBlock().getScope() } + override CfgScope getCfgScope() { result = node.getBasicBlock().getScope() } - SsaImpl::DefinitionExt getDefinitionExt() { result = def } + /** Gets the definition this node corresponds to, if any. */ + SsaImpl::Definition asDefinition() { + result = node.(SsaImpl::DataFlowIntegration::SsaDefinitionNode).getDefinition() + } override Location getLocation() { result = node.getLocation() } @@ -634,7 +633,7 @@ module LocalFlow { or // An edge from a pattern/expression to its corresponding SSA definition. nodeFrom.(Node::AstCfgFlowNode).getCfgNode() = - nodeTo.(Node::SsaNode).getDefinitionExt().(Ssa::WriteDefinition).getControlFlowNode() + nodeTo.(Node::SsaNode).asDefinition().(Ssa::WriteDefinition).getControlFlowNode() or nodeFrom.(Node::SourceParameterNode).getParameter().(ParamCfgNode).getPat() = nodeTo.asPat() or From 22b3dc8f430631e1c0fb07eda78ab4b68f6363f3 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Mon, 24 Feb 2025 15:25:53 +0100 Subject: [PATCH 11/20] Ruby: Remove getDefinitionExt references. --- .../dataflow/internal/DataFlowDispatch.qll | 2 +- .../dataflow/internal/DataFlowPrivate.qll | 89 ++++++++----------- .../ruby/dataflow/internal/DataFlowPublic.qll | 6 +- .../internal/TaintTrackingPrivate.qll | 2 +- 4 files changed, 41 insertions(+), 58 deletions(-) diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll index 8049416a6ab2..62253587e7ad 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll @@ -641,7 +641,7 @@ private module TrackInstanceInput implements CallGraphConstruction::InputSig { // type being checked against localFlowStep(nodeFrom, nodeTo, summary) and not hasAdjacentTypeCheckedRead(nodeTo) and - not TypeInference::asModulePattern(nodeTo.(SsaDefinitionExtNode).getDefinitionExt(), _) + not TypeInference::asModulePattern(nodeTo.(SsaDefinitionNodeImpl).getDefinition(), _) } predicate stepCall(DataFlow::Node nodeFrom, DataFlow::Node nodeTo, StepSummary summary) { diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll index ca5b661181bb..e28860dc5157 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll @@ -93,9 +93,9 @@ module SsaFlow { result = TSelfToplevelParameterNode(p.asToplevelSelf()) } - ParameterNodeImpl toParameterNodeImpl(SsaDefinitionExtNode node) { + ParameterNodeImpl toParameterNodeImpl(SsaDefinitionNodeImpl node) { exists(SsaImpl::WriteDefinition def, SsaImpl::ParameterExt p | - def = node.getDefinitionExt() and + def = node.getDefinition() and result = toParameterNode(p) and p.isInitializedBy(def) ) @@ -392,10 +392,10 @@ module VariableCapture { // From an assignment or implicit initialization of a captured variable to its flow-insensitive node private predicate flowInsensitiveWriteStep( - SsaDefinitionExtNode node1, CapturedVariableNode node2, CapturedVariable v + SsaDefinitionNodeImpl node1, CapturedVariableNode node2, CapturedVariable v ) { exists(CapturedSsaDefinitionExt def | - def = node1.getDefinitionExt() and + def = node1.getDefinition() and def.getSourceVariable() = v and ( def instanceof Ssa::WriteDefinition @@ -408,11 +408,11 @@ module VariableCapture { // From a captured variable node to its flow-sensitive capture nodes private predicate flowInsensitiveReadStep( - CapturedVariableNode node1, SsaDefinitionExtNode node2, CapturedVariable v + CapturedVariableNode node1, SsaDefinitionNodeImpl node2, CapturedVariable v ) { exists(CapturedSsaDefinitionExt def | node1.getVariable() = v and - def = node2.getDefinitionExt() and + def = node2.getDefinition() and def.getSourceVariable() = v and ( def instanceof Ssa::CapturedCallDefinition @@ -571,8 +571,8 @@ private module Cached { } /** Holds if `n` wraps an SSA definition without ingoing flow. */ - private predicate entrySsaDefinition(SsaDefinitionExtNode n) { - n.getDefinitionExt() = + private predicate entrySsaDefinition(SsaDefinitionNodeImpl n) { + n.getDefinition() = any(SsaImpl::WriteDefinition def | not def.(Ssa::WriteDefinition).assigns(_)) } @@ -614,7 +614,7 @@ private module Cached { // to parameters (which are themselves local sources) entrySsaDefinition(n) and not exists(SsaImpl::ParameterExt p | - p.isInitializedBy(n.(SsaDefinitionExtNode).getDefinitionExt()) + p.isInitializedBy(n.(SsaDefinitionNodeImpl).getDefinition()) ) or isStoreTargetNode(n) @@ -749,51 +749,38 @@ predicate nodeIsHidden(Node n) { } /** An SSA node. */ -abstract class SsaNode extends NodeImpl, TSsaNode { +class SsaNode extends NodeImpl, TSsaNode { SsaImpl::DataFlowIntegration::SsaNode node; - SsaImpl::DefinitionExt def; - SsaNode() { - this = TSsaNode(node) and - def = node.getDefinitionExt() - } + SsaNode() { this = TSsaNode(node) } - SsaImpl::DefinitionExt getDefinitionExt() { result = def } + /** Gets the underlying variable. */ + Variable getVariable() { result = node.getSourceVariable() } /** Holds if this node should be hidden from path explanations. */ - abstract predicate isHidden(); + predicate isHidden() { any() } + + override CfgScope getCfgScope() { result = node.getBasicBlock().getScope() } override Location getLocationImpl() { result = node.getLocation() } override string toStringImpl() { result = node.toString() } } -/** An (extended) SSA definition, viewed as a node in a data flow graph. */ -class SsaDefinitionExtNode extends SsaNode { - override SsaImpl::DataFlowIntegration::SsaDefinitionExtNode node; +class SsaDefinitionNodeImpl extends SsaNode { + override SsaImpl::DataFlowIntegration::SsaDefinitionNode node; - /** Gets the underlying variable. */ - Variable getVariable() { result = def.getSourceVariable() } + SsaImpl::Definition getDefinition() { result = node.getDefinition() } override predicate isHidden() { - not def instanceof Ssa::WriteDefinition - or - isDesugarNode(def.(Ssa::WriteDefinition).getWriteAccess().getExpr()) - or - def = getParameterDef(_) + exists(SsaImpl::Definition def | def = this.getDefinition() | + not def instanceof Ssa::WriteDefinition + or + isDesugarNode(def.(Ssa::WriteDefinition).getWriteAccess().getExpr()) + or + def = getParameterDef(_) + ) } - - override CfgScope getCfgScope() { result = def.getBasicBlock().getScope() } -} - -class SsaDefinitionNodeImpl extends SsaDefinitionExtNode { - Ssa::Definition ssaDef; - - SsaDefinitionNodeImpl() { ssaDef = def } - - override Location getLocationImpl() { result = ssaDef.getLocation() } - - override string toStringImpl() { result = ssaDef.toString() } } /** @@ -833,17 +820,13 @@ class SsaDefinitionNodeImpl extends SsaDefinitionExtNode { */ class SsaInputNode extends SsaNode { override SsaImpl::DataFlowIntegration::SsaInputNode node; - - override predicate isHidden() { any() } - - override CfgScope getCfgScope() { result = node.getDefinitionExt().getBasicBlock().getScope() } } /** An SSA definition for a `self` variable. */ -class SsaSelfDefinitionNode extends SsaDefinitionExtNode { +class SsaSelfDefinitionNode extends SsaDefinitionNodeImpl { private SelfVariable self; - SsaSelfDefinitionNode() { self = def.getSourceVariable() } + SsaSelfDefinitionNode() { self = super.getVariable() } /** Gets the scope in which the `self` variable is declared. */ Scope getSelfScope() { result = self.getDeclaringScope() } @@ -1976,9 +1959,9 @@ predicate localMustFlowStep(Node node1, Node node2) { or exists(SsaImpl::Definition def | def.(Ssa::WriteDefinition).assigns(node1.asExpr()) and - node2.(SsaDefinitionExtNode).getDefinitionExt() = def + node2.(SsaDefinitionNodeImpl).getDefinition() = def or - def = node1.(SsaDefinitionExtNode).getDefinitionExt() and + def = node1.(SsaDefinitionNodeImpl).getDefinition() and node2.asExpr() = SsaImpl::getARead(def) ) or @@ -2122,8 +2105,8 @@ class CastNode extends Node { predicate neverSkipInPathGraph(Node n) { // ensure that all variable assignments are included in the path graph n = - any(SsaDefinitionExtNode def | - def.getDefinitionExt() instanceof Ssa::WriteDefinition and + any(SsaDefinitionNodeImpl def | + def.getDefinition() instanceof Ssa::WriteDefinition and not def.isHidden() ) } @@ -2446,7 +2429,7 @@ module TypeInference { } pragma[nomagic] - private predicate ssaDefHasType(SsaDefinitionExtNode def, Module tp, boolean exact) { + private predicate ssaDefHasType(SsaDefinitionNodeImpl def, Module tp, boolean exact) { exists(ParameterNodeImpl p | parameterNodeHasType(p, tp, exact) and p = SsaFlow::toParameterNodeImpl(def) @@ -2454,7 +2437,7 @@ module TypeInference { or selfInMethodOrToplevelHasType(def.getVariable(), tp, exact) or - asModulePattern(def.getDefinitionExt(), tp) and + asModulePattern(def.getDefinition(), tp) and exact = false } @@ -2523,11 +2506,11 @@ module TypeInference { or parameterNodeHasType(n, tp, exact) or - exists(SsaDefinitionExtNode def | ssaDefHasType(def, tp, exact) | + exists(SsaDefinitionNodeImpl def | ssaDefHasType(def, tp, exact) | n = def or n.asExpr() = any(CfgNodes::ExprCfgNode read | - read = def.getDefinitionExt().getARead() and + read = def.getDefinition().(SsaImpl::DefinitionExt).getARead() and not isTypeCheckedRead(read, _) // could in principle be checked against a new type ) ) diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll index 93e579c585d5..3be4fdbcfe89 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll @@ -363,7 +363,7 @@ class PostUpdateNode extends Node { /** An SSA definition, viewed as a node in a data flow graph. */ class SsaDefinitionNode extends Node instanceof SsaDefinitionNodeImpl { /** Gets the underlying SSA definition. */ - Ssa::Definition getDefinition() { result = super.getDefinitionExt() } + Ssa::Definition getDefinition() { result = super.getDefinition() } /** Gets the underlying variable. */ Variable getVariable() { result = this.getDefinition().getSourceVariable() } @@ -434,7 +434,7 @@ private module Cached { LocalSourceNode getConstantAccessNode(ConstantAccess access) { // Namespaces don't evaluate to the constant being accessed, they return the value of their last statement. // Use the definition of 'self' in the namespace as the representative in this case. - result.(SsaDefinitionExtNode).getDefinitionExt().(Ssa::SelfDefinition).getSourceVariable() = + result.(SsaDefinitionNode).getDefinition().(Ssa::SelfDefinition).getSourceVariable() = access.(Namespace).getModuleSelfVariable() or not access instanceof Namespace and @@ -1002,7 +1002,7 @@ class ModuleNode instanceof Module { * This only gets `self` at the module level, not inside any (singleton) method. */ LocalSourceNode getModuleLevelSelf() { - result.(SsaDefinitionExtNode).getVariable() = super.getADeclaration().getModuleSelfVariable() + result.(SsaDefinitionNode).getVariable() = super.getADeclaration().getModuleSelfVariable() } /** diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/TaintTrackingPrivate.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/TaintTrackingPrivate.qll index 2da85f79b5ad..104ef68c2677 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/TaintTrackingPrivate.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/TaintTrackingPrivate.qll @@ -88,7 +88,7 @@ private module Cached { nodeFrom.asExpr() = value and value = case.getValue() and clause = case.getBranch(_) and - def = nodeTo.(SsaDefinitionExtNode).getDefinitionExt() and + def = nodeTo.(SsaDefinitionNodeImpl).getDefinition() and def.getControlFlowNode() = variablesInPattern(clause.getPattern()) and not def.(Ssa::WriteDefinition).assigns(value) ) From 57c4fd6f2518a799018ba62e7697cc1e203cc01d Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Tue, 25 Feb 2025 09:23:53 +0100 Subject: [PATCH 12/20] JS: Combine phi reads and ssa input nodes into SynthReadNode class. --- .../dataflow/internal/DataFlowNode.qll | 6 +-- .../dataflow/internal/DataFlowPrivate.qll | 39 +++++-------------- shared/ssa/codeql/ssa/Ssa.qll | 7 ++++ 3 files changed, 18 insertions(+), 34 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowNode.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowNode.qll index 26bff4fdf806..8d54f639cb0a 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowNode.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowNode.qll @@ -34,10 +34,8 @@ private module Cached { TSsaDefNode(SsaDefinition d) or /** Use of a variable or 'this', with flow from a post-update node (from an earlier use) */ TSsaUseNode(ControlFlowNode use) { use = any(Ssa2::SsaConfig::SourceVariable v).getAUse() } or - /** Phi-read node (new SSA library). Ordinary phi nodes are represented by TSsaDefNode. */ - TSsaPhiReadNode(Ssa2::PhiReadNode phi) or - /** Input to a phi node (new SSA library) */ - TSsaInputNode(Ssa2::SsaInputNode input) or + /** A synthesized variable read (new SSA library). Definitions are represented by TSsaDefNode. */ + TSsaSynthReadNode(Ssa2::SsaSynthReadNode read) or TCapturedVariableNode(LocalVariable v) { v.isCaptured() } or TPropNode(@property p) or TRestPatternNode(DestructuringPattern dp, Expr rest) { rest = dp.getRest() } or diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll index 3511666c8a18..4b82d895f642 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll @@ -35,38 +35,21 @@ class SsaUseNode extends DataFlow::Node, TSsaUseNode { override Location getLocation() { result = expr.getLocation() } } -class SsaPhiReadNode extends DataFlow::Node, TSsaPhiReadNode { - private Ssa2::PhiReadNode phi; +class SsaSynthReadNode extends DataFlow::Node, TSsaSynthReadNode { + private Ssa2::SsaSynthReadNode read; - SsaPhiReadNode() { this = TSsaPhiReadNode(phi) } + SsaSynthReadNode() { this = TSsaSynthReadNode(read) } cached - override string toString() { result = "[ssa-phi-read] " + phi.getSourceVariable().getName() } - - cached - override StmtContainer getContainer() { result = phi.getSourceVariable().getDeclaringContainer() } - - cached - override Location getLocation() { result = phi.getLocation() } -} - -class SsaInputNode extends DataFlow::Node, TSsaInputNode { - private Ssa2::SsaInputNode input; - - SsaInputNode() { this = TSsaInputNode(input) } - - cached - override string toString() { - result = "[ssa-input] " + input.getDefinitionExt().getSourceVariable().getName() - } + override string toString() { result = "[ssa-synth-read] " + read.getSourceVariable().getName() } cached override StmtContainer getContainer() { - result = input.getDefinitionExt().getSourceVariable().getDeclaringContainer() + result = read.getSourceVariable().getDeclaringContainer() } cached - override Location getLocation() { result = input.getLocation() } + override Location getLocation() { result = read.getLocation() } } class FlowSummaryNode extends DataFlow::Node, TFlowSummaryNode { @@ -675,9 +658,7 @@ predicate nodeIsHidden(Node node) { or node instanceof SsaUseNode or - node instanceof SsaPhiReadNode - or - node instanceof SsaInputNode + node instanceof SsaSynthReadNode } predicate neverSkipInPathGraph(Node node) { @@ -1258,12 +1239,10 @@ Node getNodeFromSsa2(Ssa2::Node node) { result = TImplicitThisUse(use, true) ) or - result = TSsaPhiReadNode(node.(Ssa2::SsaDefinitionExtNode).getDefinitionExt()) - or - result = TSsaInputNode(node.(Ssa2::SsaInputNode)) + result = TSsaSynthReadNode(node) or exists(SsaPhiNode legacyPhi, Ssa2::PhiNode ssaPhi | - node.(Ssa2::SsaDefinitionExtNode).getDefinitionExt() = ssaPhi and + node.(Ssa2::SsaDefinitionNode).getDefinition() = ssaPhi and samePhi(legacyPhi, ssaPhi) and result = TSsaDefNode(legacyPhi) ) diff --git a/shared/ssa/codeql/ssa/Ssa.qll b/shared/ssa/codeql/ssa/Ssa.qll index e77946a90a63..dc80246f2881 100644 --- a/shared/ssa/codeql/ssa/Ssa.qll +++ b/shared/ssa/codeql/ssa/Ssa.qll @@ -1688,6 +1688,13 @@ module Make Input> { final class SsaDefinitionNode = SsaDefinitionNodeImpl; + final class SsaSynthReadNode extends SsaNode { + SsaSynthReadNode() { + this.(SsaDefinitionExtNodeImpl).getDefinitionExt() instanceof PhiReadNode or + this instanceof SsaInputNodeImpl + } + } + /** * A node that represents an input to an SSA phi (read) definition. * From 95cbd21a62a7856770071a77fec1824b3fca3f37 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Tue, 25 Feb 2025 09:59:35 +0100 Subject: [PATCH 13/20] Ruby: Accept test change following SSA bugfix. This is a result of the commit "SSA: Fix bug in guards for ssa input nodes." --- .../barrier-guards/barrier-guards.expected | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.expected b/ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.expected index 919dcb71c2fc..3e838551f81d 100644 --- a/ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.expected +++ b/ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.expected @@ -2,40 +2,87 @@ testFailures newStyleBarrierGuards | barrier-guards.rb:3:16:4:19 | [input] SSA phi read(foo) | | barrier-guards.rb:4:5:4:7 | foo | +| barrier-guards.rb:9:25:10:19 | [input] SSA phi read(foo) | | barrier-guards.rb:10:5:10:7 | foo | +| barrier-guards.rb:17:1:18:19 | [input] SSA phi read(foo) | | barrier-guards.rb:18:5:18:7 | foo | +| barrier-guards.rb:23:1:24:19 | [input] SSA phi read(foo) | | barrier-guards.rb:24:5:24:7 | foo | +| barrier-guards.rb:27:20:28:19 | [input] SSA phi read(foo) | | barrier-guards.rb:28:5:28:7 | foo | +| barrier-guards.rb:37:21:38:19 | [input] SSA phi read(foo) | | barrier-guards.rb:38:5:38:7 | foo | +| barrier-guards.rb:43:16:46:5 | [input] SSA phi read(foo) | | barrier-guards.rb:45:9:45:11 | foo | | barrier-guards.rb:70:22:71:19 | [input] SSA phi read(foo) | | barrier-guards.rb:71:5:71:7 | foo | +| barrier-guards.rb:82:26:83:19 | [input] SSA phi read(foo) | | barrier-guards.rb:83:5:83:7 | foo | +| barrier-guards.rb:90:1:91:19 | [input] SSA phi read(foo) | | barrier-guards.rb:91:5:91:7 | foo | +| barrier-guards.rb:125:11:126:19 | [input] SSA phi read(foo) | | barrier-guards.rb:126:5:126:7 | foo | +| barrier-guards.rb:132:11:133:19 | [input] SSA phi read(foo) | | barrier-guards.rb:133:5:133:7 | foo | +| barrier-guards.rb:134:11:135:19 | [input] SSA phi read(foo) | | barrier-guards.rb:135:5:135:7 | foo | +| barrier-guards.rb:139:18:140:19 | [input] SSA phi read(foo) | | barrier-guards.rb:140:5:140:7 | foo | +| barrier-guards.rb:141:19:142:19 | [input] SSA phi read(foo) | | barrier-guards.rb:142:5:142:7 | foo | +| barrier-guards.rb:148:21:149:19 | [input] SSA phi read(foo) | | barrier-guards.rb:149:5:149:7 | foo | +| barrier-guards.rb:153:18:154:19 | [input] SSA phi read(foo) | | barrier-guards.rb:154:5:154:7 | foo | +| barrier-guards.rb:158:10:159:19 | [input] SSA phi read(foo) | | barrier-guards.rb:159:5:159:7 | foo | +| barrier-guards.rb:163:11:164:19 | [input] SSA phi read(foo) | | barrier-guards.rb:164:5:164:7 | foo | +| barrier-guards.rb:191:4:191:15 | [input] SSA phi read(foo) | +| barrier-guards.rb:191:20:191:31 | [input] SSA phi read(foo) | +| barrier-guards.rb:191:32:192:19 | [input] SSA phi read(foo) | | barrier-guards.rb:192:5:192:7 | foo | +| barrier-guards.rb:195:4:195:15 | [input] SSA phi read(foo) | +| barrier-guards.rb:195:4:195:31 | [input] SSA phi read(foo) | +| barrier-guards.rb:195:20:195:31 | [input] SSA phi read(foo) | +| barrier-guards.rb:195:36:195:47 | [input] SSA phi read(foo) | +| barrier-guards.rb:195:48:196:19 | [input] SSA phi read(foo) | | barrier-guards.rb:196:5:196:7 | foo | +| barrier-guards.rb:199:4:199:15 | [input] SSA phi read(foo) | +| barrier-guards.rb:199:4:199:31 | [input] SSA phi read(foo) | +| barrier-guards.rb:199:20:199:31 | [input] SSA phi read(foo) | +| barrier-guards.rb:203:4:203:15 | [input] SSA phi read(foo) | +| barrier-guards.rb:203:36:203:47 | [input] SSA phi read(foo) | +| barrier-guards.rb:207:21:207:21 | [input] SSA phi read(foo) | +| barrier-guards.rb:207:22:208:19 | [input] SSA phi read(foo) | | barrier-guards.rb:208:5:208:7 | foo | +| barrier-guards.rb:211:22:212:19 | [input] SSA phi read(foo) | | barrier-guards.rb:212:5:212:7 | foo | +| barrier-guards.rb:215:28:216:19 | [input] SSA phi read(foo) | | barrier-guards.rb:216:5:216:7 | foo | | barrier-guards.rb:219:21:219:23 | foo | +| barrier-guards.rb:219:21:219:32 | [input] SSA phi read(foo) | +| barrier-guards.rb:219:95:220:19 | [input] SSA phi read(foo) | | barrier-guards.rb:220:5:220:7 | foo | +| barrier-guards.rb:227:21:227:21 | [input] SSA phi read(foo) | +| barrier-guards.rb:227:22:228:7 | [input] SSA phi read(foo) | +| barrier-guards.rb:232:18:233:19 | [input] SSA phi read(foo) | | barrier-guards.rb:233:5:233:7 | foo | +| barrier-guards.rb:237:19:237:38 | [input] SSA phi read(foo) | | barrier-guards.rb:237:24:237:26 | foo | +| barrier-guards.rb:243:9:244:19 | [input] SSA phi read(foo) | | barrier-guards.rb:244:5:244:7 | foo | +| barrier-guards.rb:259:17:260:19 | [input] SSA phi read(foo) | | barrier-guards.rb:260:5:260:7 | foo | +| barrier-guards.rb:264:17:265:19 | [input] SSA phi read(foo) | | barrier-guards.rb:265:5:265:7 | foo | +| barrier-guards.rb:272:17:272:19 | [input] SSA phi read(foo) | | barrier-guards.rb:272:17:272:19 | foo | +| barrier-guards.rb:275:20:276:19 | [input] SSA phi read(foo) | | barrier-guards.rb:276:5:276:7 | foo | +| barrier-guards.rb:281:21:282:19 | [input] SSA phi read(foo) | | barrier-guards.rb:282:5:282:7 | foo | +| barrier-guards.rb:291:7:292:19 | [input] SSA phi read(foo) | | barrier-guards.rb:292:5:292:7 | foo | | barrier_flow.rb:19:14:19:14 | x | | barrier_flow.rb:32:10:32:10 | x | From 1f628d0f86204086ecf3ccc14256d15bbb847ce9 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Tue, 25 Feb 2025 10:01:57 +0100 Subject: [PATCH 14/20] Ruby: Remove reference to SsaInputNode. --- .../dataflow/internal/DataFlowPrivate.qll | 40 ++----------------- .../dataflow/barrier-guards/barrier-guards.ql | 2 +- 2 files changed, 4 insertions(+), 38 deletions(-) diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll index e28860dc5157..004944c83e4a 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll @@ -783,43 +783,9 @@ class SsaDefinitionNodeImpl extends SsaNode { } } -/** - * A node that represents an input to an SSA phi (read) definition. - * - * This allows for barrier guards to filter input to phi nodes. For example, in - * - * ```rb - * x = taint - * if x != "safe" then - * x = "safe" - * end - * sink x - * ``` - * - * the `false` edge out of `x != "safe"` guards the input from `x = taint` into the - * `phi` node after the condition. - * - * It is also relevant to filter input into phi read nodes: - * - * ```rb - * x = taint - * if b then - * if x != "safe1" then - * return - * end - * else - * if x != "safe2" then - * return - * end - * end - * - * sink x - * ``` - * - * both inputs into the phi read node after the outer condition are guarded. - */ -class SsaInputNode extends SsaNode { - override SsaImpl::DataFlowIntegration::SsaInputNode node; +/** A synthesized SSA read. */ +class SsaSynthReadNode extends SsaNode { + override SsaImpl::DataFlowIntegration::SsaSynthReadNode node; } /** An SSA definition for a `self` variable. */ diff --git a/ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.ql b/ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.ql index 1d0d6388e4f0..1bf7461e7fac 100644 --- a/ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.ql +++ b/ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.ql @@ -26,7 +26,7 @@ module BarrierGuardTest implements TestSig { tag = "guarded" and exists(DataFlow::Node n | newStyleBarrierGuards(n) and - not n instanceof SsaInputNode and + not n instanceof SsaSynthReadNode and location = n.getLocation() and element = n.toString() and value = "" From f00f2c6f479ae2dc7d2e78d9c233604f7d04e4d4 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Tue, 25 Feb 2025 10:03:43 +0100 Subject: [PATCH 15/20] SSA: Deprecate public SsaDefinitionExtNode and SsaInputNode. --- shared/ssa/codeql/ssa/Ssa.qll | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/shared/ssa/codeql/ssa/Ssa.qll b/shared/ssa/codeql/ssa/Ssa.qll index dc80246f2881..bfb2893d4835 100644 --- a/shared/ssa/codeql/ssa/Ssa.qll +++ b/shared/ssa/codeql/ssa/Ssa.qll @@ -1675,7 +1675,7 @@ module Make Input> { override string toString() { result = def.toString() } } - final class SsaDefinitionExtNode = SsaDefinitionExtNodeImpl; + deprecated final class SsaDefinitionExtNode = SsaDefinitionExtNodeImpl; /** An SSA definition, viewed as a node in a data flow graph. */ private class SsaDefinitionNodeImpl extends SsaDefinitionExtNodeImpl { @@ -1753,7 +1753,7 @@ module Make Input> { override string toString() { result = "[input] " + def_.toString() } } - final class SsaInputNode = SsaInputNodeImpl; + deprecated final class SsaInputNode = SsaInputNodeImpl; /** * Holds if `nodeFrom` corresponds to the reference to `v` at index `i` in @@ -1765,7 +1765,7 @@ module Make Input> { private predicate flowOutOf( DefinitionExt def, Node nodeFrom, SourceVariable v, BasicBlock bb, int i, boolean isUseStep ) { - nodeFrom.(SsaDefinitionExtNode).getDefinitionExt() = def and + nodeFrom.(SsaDefinitionExtNodeImpl).getDefinitionExt() = def and def.definesAt(v, bb, i, _) and isUseStep = false or @@ -1788,7 +1788,7 @@ module Make Input> { // Flow from parameter into entry definition DfInput::ssaDefInitializesParam(def, nodeFrom.(ParameterNode).getParameter()) ) and - nodeTo.(SsaDefinitionExtNode).getDefinitionExt() = def and + nodeTo.(SsaDefinitionExtNodeImpl).getDefinitionExt() = def and isUseStep = false or // Flow from definition/read to next read @@ -1804,7 +1804,7 @@ module Make Input> { AdjacentSsaRefs::adjacentRefRead(bb1, i1, bb2, i2, v) and exists(UncertainWriteDefinition def2 | DfInput::allowFlowIntoUncertainDef(def2) and - nodeTo.(SsaDefinitionExtNode).getDefinitionExt() = def2 and + nodeTo.(SsaDefinitionExtNodeImpl).getDefinitionExt() = def2 and def2.definesAt(v, bb2, i2) ) ) @@ -1821,8 +1821,8 @@ module Make Input> { ) or // Flow from input node to def - nodeTo.(SsaDefinitionExtNode).getDefinitionExt() = def and - def = nodeFrom.(SsaInputNode).getDefinitionExt() and + nodeTo.(SsaDefinitionExtNodeImpl).getDefinitionExt() = def and + def = nodeFrom.(SsaInputNodeImpl).getDefinitionExt() and isUseStep = false } @@ -1835,10 +1835,10 @@ module Make Input> { // Flow from parameter into entry definition DfInput::ssaDefInitializesParam(def, nodeFrom.(ParameterNode).getParameter()) ) and - nodeTo.(SsaDefinitionExtNode).getDefinitionExt() = def + nodeTo.(SsaDefinitionExtNodeImpl).getDefinitionExt() = def or // Flow from SSA definition to read - nodeFrom.(SsaDefinitionExtNode).getDefinitionExt() = def and + nodeFrom.(SsaDefinitionExtNodeImpl).getDefinitionExt() = def and nodeTo.(ExprNode).getExpr() = DfInput::getARead(def) } @@ -1852,7 +1852,7 @@ module Make Input> { signature predicate guardChecksSig(DfInput::Guard g, DfInput::Expr e, boolean branch); pragma[nomagic] - private Definition getAPhiInputDef(SsaInputNode n) { + private Definition getAPhiInputDef(SsaInputNodeImpl n) { exists(SsaInputDefinitionExt phi, BasicBlock bb | result = getAPhiInputDef(phi, bb) and n.isInputInto(phi, bb) @@ -1927,7 +1927,7 @@ module Make Input> { // guard controls input block to a phi node exists(SsaInputDefinitionExt phi | def = getAPhiInputDef(result) and - result.(SsaInputNode).isInputInto(phi, bb) + result.(SsaInputNodeImpl).isInputInto(phi, bb) | DfInput::guardControlsBlock(g, bb, branch) or From b1b72b73edc6ec0e194e78b997317a2001bbad81 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Tue, 25 Feb 2025 10:20:15 +0100 Subject: [PATCH 16/20] SSA: Add qldoc. --- shared/ssa/codeql/ssa/Ssa.qll | 2 ++ 1 file changed, 2 insertions(+) diff --git a/shared/ssa/codeql/ssa/Ssa.qll b/shared/ssa/codeql/ssa/Ssa.qll index bfb2893d4835..bdf571923737 100644 --- a/shared/ssa/codeql/ssa/Ssa.qll +++ b/shared/ssa/codeql/ssa/Ssa.qll @@ -1683,11 +1683,13 @@ module Make Input> { SsaDefinitionNodeImpl() { this = TSsaDefinitionNode(def) } + /** Gets the underlying SSA definition. */ Definition getDefinition() { result = def } } final class SsaDefinitionNode = SsaDefinitionNodeImpl; + /** A node that represents a synthetic read of a source variable. */ final class SsaSynthReadNode extends SsaNode { SsaSynthReadNode() { this.(SsaDefinitionExtNodeImpl).getDefinitionExt() instanceof PhiReadNode or From ae3736bc250395ec211a3eea21de6c55110b7f27 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Tue, 25 Feb 2025 10:37:29 +0100 Subject: [PATCH 17/20] C#: Accept test changes showing that we skip over useless input nodes. --- .../dataflow/local/DataFlowStep.expected | 52 +++++++------------ .../dataflow/local/TaintTrackingStep.expected | 52 +++++++------------ 2 files changed, 40 insertions(+), 64 deletions(-) diff --git a/csharp/ql/test/library-tests/dataflow/local/DataFlowStep.expected b/csharp/ql/test/library-tests/dataflow/local/DataFlowStep.expected index a8b0a4d0cd55..9e488197e7c1 100644 --- a/csharp/ql/test/library-tests/dataflow/local/DataFlowStep.expected +++ b/csharp/ql/test/library-tests/dataflow/local/DataFlowStep.expected @@ -533,8 +533,6 @@ | LocalDataFlow.cs:381:17:381:29 | "not tainted" | LocalDataFlow.cs:381:13:381:13 | access to local variable x | | LocalDataFlow.cs:382:9:382:17 | SSA phi(x) | LocalDataFlow.cs:382:15:382:15 | access to local variable x | | SSA.cs:5:17:5:17 | SSA entry def(this.S) | SSA.cs:67:9:67:14 | access to field S | -| SSA.cs:5:17:5:17 | [input] SSA def(this.S) | SSA.cs:136:23:136:28 | SSA def(this.S) | -| SSA.cs:5:17:5:17 | [input] SSA qualifier def(this.S.SsaFieldNonSink0) | SSA.cs:136:23:136:28 | SSA qualifier def(this.S.SsaFieldNonSink0) | | SSA.cs:5:17:5:17 | this | SSA.cs:67:9:67:12 | this access | | SSA.cs:5:26:5:32 | SSA param(tainted) | SSA.cs:8:24:8:30 | access to parameter tainted | | SSA.cs:5:26:5:32 | tainted | SSA.cs:5:26:5:32 | SSA param(tainted) | @@ -664,28 +662,28 @@ | SSA.cs:58:27:58:33 | access to parameter tainted | SSA.cs:58:16:58:23 | access to local variable ssaSink3 | | SSA.cs:58:27:58:33 | access to parameter tainted | SSA.cs:67:32:67:38 | access to parameter tainted | | SSA.cs:59:23:59:30 | SSA def(ssaSink3) | SSA.cs:60:15:60:22 | access to local variable ssaSink3 | -| SSA.cs:59:23:59:30 | [post] access to local variable ssaSink3 | SSA.cs:89:13:89:33 | [input] SSA def(ssaSink3) | +| SSA.cs:59:23:59:30 | [post] access to local variable ssaSink3 | SSA.cs:59:23:59:30 | SSA def(ssaSink3) | +| SSA.cs:59:23:59:30 | access to local variable ssaSink3 | SSA.cs:59:23:59:30 | SSA def(ssaSink3) | | SSA.cs:59:23:59:30 | access to local variable ssaSink3 | SSA.cs:59:23:59:30 | SSA def(ssaSink3) | -| SSA.cs:59:23:59:30 | access to local variable ssaSink3 | SSA.cs:89:13:89:33 | [input] SSA def(ssaSink3) | | SSA.cs:63:23:63:30 | SSA def(nonSink0) | SSA.cs:64:15:64:22 | access to local variable nonSink0 | -| SSA.cs:63:23:63:30 | [post] access to local variable nonSink0 | SSA.cs:89:13:89:33 | [input] SSA def(nonSink0) | +| SSA.cs:63:23:63:30 | [post] access to local variable nonSink0 | SSA.cs:63:23:63:30 | SSA def(nonSink0) | +| SSA.cs:63:23:63:30 | access to local variable nonSink0 | SSA.cs:63:23:63:30 | SSA def(nonSink0) | | SSA.cs:63:23:63:30 | access to local variable nonSink0 | SSA.cs:63:23:63:30 | SSA def(nonSink0) | -| SSA.cs:63:23:63:30 | access to local variable nonSink0 | SSA.cs:89:13:89:33 | [input] SSA def(nonSink0) | | SSA.cs:67:9:67:12 | [post] this access | SSA.cs:68:23:68:26 | this access | | SSA.cs:67:9:67:12 | this access | SSA.cs:68:23:68:26 | this access | | SSA.cs:67:9:67:14 | [post] access to field S | SSA.cs:68:23:68:28 | access to field S | | SSA.cs:67:9:67:14 | access to field S | SSA.cs:68:23:68:28 | access to field S | | SSA.cs:67:9:67:28 | access to field SsaFieldSink0 | SSA.cs:67:9:67:38 | SSA def(this.S.SsaFieldSink0) | -| SSA.cs:67:9:67:38 | SSA def(this.S.SsaFieldSink0) | SSA.cs:89:13:89:33 | [input] SSA qualifier def(this.S.SsaFieldSink0) | +| SSA.cs:67:9:67:38 | SSA def(this.S.SsaFieldSink0) | SSA.cs:68:23:68:28 | SSA qualifier def(this.S.SsaFieldSink0) | | SSA.cs:67:32:67:38 | access to parameter tainted | SSA.cs:67:9:67:28 | access to field SsaFieldSink0 | | SSA.cs:67:32:67:38 | access to parameter tainted | SSA.cs:77:20:77:26 | access to parameter tainted | | SSA.cs:68:23:68:26 | [post] this access | SSA.cs:69:15:69:18 | this access | | SSA.cs:68:23:68:26 | this access | SSA.cs:69:15:69:18 | this access | | SSA.cs:68:23:68:28 | SSA def(this.S) | SSA.cs:69:15:69:20 | access to field S | | SSA.cs:68:23:68:28 | SSA qualifier def(this.S.SsaFieldSink0) | SSA.cs:69:15:69:34 | access to field SsaFieldSink0 | -| SSA.cs:68:23:68:28 | [post] access to field S | SSA.cs:89:13:89:33 | [input] SSA def(this.S) | +| SSA.cs:68:23:68:28 | [post] access to field S | SSA.cs:68:23:68:28 | SSA def(this.S) | +| SSA.cs:68:23:68:28 | access to field S | SSA.cs:68:23:68:28 | SSA def(this.S) | | SSA.cs:68:23:68:28 | access to field S | SSA.cs:68:23:68:28 | SSA def(this.S) | -| SSA.cs:68:23:68:28 | access to field S | SSA.cs:89:13:89:33 | [input] SSA def(this.S) | | SSA.cs:69:15:69:18 | [post] this access | SSA.cs:72:9:72:12 | this access | | SSA.cs:69:15:69:18 | this access | SSA.cs:72:9:72:12 | this access | | SSA.cs:69:15:69:20 | [post] access to field S | SSA.cs:72:9:72:14 | access to field S | @@ -695,15 +693,15 @@ | SSA.cs:72:9:72:14 | [post] access to field S | SSA.cs:73:23:73:28 | access to field S | | SSA.cs:72:9:72:14 | access to field S | SSA.cs:73:23:73:28 | access to field S | | SSA.cs:72:9:72:31 | access to field SsaFieldNonSink0 | SSA.cs:72:9:72:36 | SSA def(this.S.SsaFieldNonSink0) | -| SSA.cs:72:9:72:36 | SSA def(this.S.SsaFieldNonSink0) | SSA.cs:89:13:89:33 | [input] SSA qualifier def(this.S.SsaFieldNonSink0) | +| SSA.cs:72:9:72:36 | SSA def(this.S.SsaFieldNonSink0) | SSA.cs:73:23:73:28 | SSA qualifier def(this.S.SsaFieldNonSink0) | | SSA.cs:72:35:72:36 | "" | SSA.cs:72:9:72:31 | access to field SsaFieldNonSink0 | | SSA.cs:73:23:73:26 | [post] this access | SSA.cs:74:15:74:18 | this access | | SSA.cs:73:23:73:26 | this access | SSA.cs:74:15:74:18 | this access | | SSA.cs:73:23:73:28 | SSA def(this.S) | SSA.cs:74:15:74:20 | access to field S | | SSA.cs:73:23:73:28 | SSA qualifier def(this.S.SsaFieldNonSink0) | SSA.cs:74:15:74:37 | access to field SsaFieldNonSink0 | -| SSA.cs:73:23:73:28 | [post] access to field S | SSA.cs:89:13:89:33 | [input] SSA def(this.S) | +| SSA.cs:73:23:73:28 | [post] access to field S | SSA.cs:73:23:73:28 | SSA def(this.S) | +| SSA.cs:73:23:73:28 | access to field S | SSA.cs:73:23:73:28 | SSA def(this.S) | | SSA.cs:73:23:73:28 | access to field S | SSA.cs:73:23:73:28 | SSA def(this.S) | -| SSA.cs:73:23:73:28 | access to field S | SSA.cs:89:13:89:33 | [input] SSA def(this.S) | | SSA.cs:74:15:74:18 | [post] this access | SSA.cs:80:9:80:12 | this access | | SSA.cs:74:15:74:18 | this access | SSA.cs:80:9:80:12 | this access | | SSA.cs:74:15:74:20 | [post] access to field S | SSA.cs:80:9:80:14 | access to field S | @@ -752,15 +750,9 @@ | SSA.cs:89:13:89:22 | [post] access to parameter nonTainted | SSA.cs:92:17:92:26 | access to parameter nonTainted | | SSA.cs:89:13:89:22 | access to parameter nonTainted | SSA.cs:89:13:89:33 | [input] SSA phi read(nonTainted) | | SSA.cs:89:13:89:22 | access to parameter nonTainted | SSA.cs:92:17:92:26 | access to parameter nonTainted | -| SSA.cs:89:13:89:33 | [input] SSA def(nonSink0) | SSA.cs:63:23:63:30 | SSA def(nonSink0) | -| SSA.cs:89:13:89:33 | [input] SSA def(ssaSink3) | SSA.cs:59:23:59:30 | SSA def(ssaSink3) | -| SSA.cs:89:13:89:33 | [input] SSA def(this.S) | SSA.cs:68:23:68:28 | SSA def(this.S) | -| SSA.cs:89:13:89:33 | [input] SSA def(this.S) | SSA.cs:73:23:73:28 | SSA def(this.S) | | SSA.cs:89:13:89:33 | [input] SSA phi read(nonTainted) | SSA.cs:97:9:97:32 | SSA phi read(nonTainted) | | SSA.cs:89:13:89:33 | [input] SSA phi read(ssaSink0) | SSA.cs:97:9:97:32 | SSA phi read(ssaSink0) | | SSA.cs:89:13:89:33 | [input] SSA phi(ssaSink4) | SSA.cs:97:9:97:32 | SSA phi(ssaSink4) | -| SSA.cs:89:13:89:33 | [input] SSA qualifier def(this.S.SsaFieldNonSink0) | SSA.cs:73:23:73:28 | SSA qualifier def(this.S.SsaFieldNonSink0) | -| SSA.cs:89:13:89:33 | [input] SSA qualifier def(this.S.SsaFieldSink0) | SSA.cs:68:23:68:28 | SSA qualifier def(this.S.SsaFieldSink0) | | SSA.cs:91:13:91:20 | access to local variable ssaSink4 | SSA.cs:91:13:91:31 | SSA def(ssaSink4) | | SSA.cs:91:13:91:31 | SSA def(ssaSink4) | SSA.cs:93:21:93:28 | access to local variable ssaSink4 | | SSA.cs:91:13:91:31 | SSA def(ssaSink4) | SSA.cs:95:21:95:28 | access to local variable ssaSink4 | @@ -785,9 +777,9 @@ | SSA.cs:97:9:97:32 | SSA phi read(ssaSink0) | SSA.cs:117:36:117:43 | access to local variable ssaSink0 | | SSA.cs:97:9:97:32 | SSA phi(ssaSink4) | SSA.cs:97:23:97:30 | access to local variable ssaSink4 | | SSA.cs:97:23:97:30 | SSA def(ssaSink4) | SSA.cs:98:15:98:22 | access to local variable ssaSink4 | -| SSA.cs:97:23:97:30 | [post] access to local variable ssaSink4 | SSA.cs:102:13:102:33 | [input] SSA def(ssaSink4) | +| SSA.cs:97:23:97:30 | [post] access to local variable ssaSink4 | SSA.cs:97:23:97:30 | SSA def(ssaSink4) | +| SSA.cs:97:23:97:30 | access to local variable ssaSink4 | SSA.cs:97:23:97:30 | SSA def(ssaSink4) | | SSA.cs:97:23:97:30 | access to local variable ssaSink4 | SSA.cs:97:23:97:30 | SSA def(ssaSink4) | -| SSA.cs:97:23:97:30 | access to local variable ssaSink4 | SSA.cs:102:13:102:33 | [input] SSA def(ssaSink4) | | SSA.cs:101:16:101:23 | access to local variable nonSink3 | SSA.cs:101:16:101:28 | SSA def(nonSink3) | | SSA.cs:101:16:101:28 | SSA def(nonSink3) | SSA.cs:102:13:102:33 | [input] SSA phi(nonSink3) | | SSA.cs:101:27:101:28 | "" | SSA.cs:101:16:101:23 | access to local variable nonSink3 | @@ -795,7 +787,6 @@ | SSA.cs:102:13:102:22 | [post] access to parameter nonTainted | SSA.cs:105:17:105:26 | access to parameter nonTainted | | SSA.cs:102:13:102:22 | access to parameter nonTainted | SSA.cs:102:13:102:33 | [input] SSA phi read(nonTainted) | | SSA.cs:102:13:102:22 | access to parameter nonTainted | SSA.cs:105:17:105:26 | access to parameter nonTainted | -| SSA.cs:102:13:102:33 | [input] SSA def(ssaSink4) | SSA.cs:97:23:97:30 | SSA def(ssaSink4) | | SSA.cs:102:13:102:33 | [input] SSA phi read(nonSink0) | SSA.cs:110:9:110:32 | SSA phi read(nonSink0) | | SSA.cs:102:13:102:33 | [input] SSA phi read(nonTainted) | SSA.cs:110:9:110:32 | SSA phi read(nonTainted) | | SSA.cs:102:13:102:33 | [input] SSA phi(nonSink3) | SSA.cs:110:9:110:32 | SSA phi(nonSink3) | @@ -823,9 +814,9 @@ | SSA.cs:110:9:110:32 | SSA phi read(nonTainted) | SSA.cs:115:13:115:22 | access to parameter nonTainted | | SSA.cs:110:9:110:32 | SSA phi(nonSink3) | SSA.cs:110:23:110:30 | access to local variable nonSink3 | | SSA.cs:110:23:110:30 | SSA def(nonSink3) | SSA.cs:111:15:111:22 | access to local variable nonSink3 | -| SSA.cs:110:23:110:30 | [post] access to local variable nonSink3 | SSA.cs:115:13:115:33 | [input] SSA def(nonSink3) | +| SSA.cs:110:23:110:30 | [post] access to local variable nonSink3 | SSA.cs:110:23:110:30 | SSA def(nonSink3) | +| SSA.cs:110:23:110:30 | access to local variable nonSink3 | SSA.cs:110:23:110:30 | SSA def(nonSink3) | | SSA.cs:110:23:110:30 | access to local variable nonSink3 | SSA.cs:110:23:110:30 | SSA def(nonSink3) | -| SSA.cs:110:23:110:30 | access to local variable nonSink3 | SSA.cs:115:13:115:33 | [input] SSA def(nonSink3) | | SSA.cs:114:9:114:12 | [post] this access | SSA.cs:117:13:117:16 | this access | | SSA.cs:114:9:114:12 | [post] this access | SSA.cs:123:23:123:26 | this access | | SSA.cs:114:9:114:12 | this access | SSA.cs:117:13:117:16 | this access | @@ -841,7 +832,6 @@ | SSA.cs:115:13:115:22 | [post] access to parameter nonTainted | SSA.cs:118:17:118:26 | access to parameter nonTainted | | SSA.cs:115:13:115:22 | access to parameter nonTainted | SSA.cs:115:13:115:33 | [input] SSA phi read(nonTainted) | | SSA.cs:115:13:115:22 | access to parameter nonTainted | SSA.cs:118:17:118:26 | access to parameter nonTainted | -| SSA.cs:115:13:115:33 | [input] SSA def(nonSink3) | SSA.cs:110:23:110:30 | SSA def(nonSink3) | | SSA.cs:115:13:115:33 | [input] SSA phi read(nonTainted) | SSA.cs:123:9:123:30 | SSA phi read(nonTainted) | | SSA.cs:115:13:115:33 | [input] SSA phi read(this.S) | SSA.cs:123:9:123:30 | SSA phi read(this.S) | | SSA.cs:115:13:115:33 | [input] SSA phi(this.S.SsaFieldSink1) | SSA.cs:123:9:123:30 | SSA phi(this.S.SsaFieldSink1) | @@ -881,14 +871,14 @@ | SSA.cs:121:21:121:40 | access to field SsaFieldSink1 | SSA.cs:121:17:121:41 | [input] SSA phi(this.S.SsaFieldSink1) | | SSA.cs:123:9:123:30 | SSA phi read(nonTainted) | SSA.cs:128:13:128:22 | access to parameter nonTainted | | SSA.cs:123:9:123:30 | SSA phi read(this.S) | SSA.cs:123:23:123:28 | access to field S | -| SSA.cs:123:9:123:30 | SSA phi(this.S.SsaFieldSink1) | SSA.cs:128:13:128:33 | [input] SSA qualifier def(this.S.SsaFieldSink1) | +| SSA.cs:123:9:123:30 | SSA phi(this.S.SsaFieldSink1) | SSA.cs:123:23:123:28 | SSA qualifier def(this.S.SsaFieldSink1) | | SSA.cs:123:23:123:26 | [post] this access | SSA.cs:124:15:124:18 | this access | | SSA.cs:123:23:123:26 | this access | SSA.cs:124:15:124:18 | this access | | SSA.cs:123:23:123:28 | SSA def(this.S) | SSA.cs:124:15:124:20 | access to field S | | SSA.cs:123:23:123:28 | SSA qualifier def(this.S.SsaFieldSink1) | SSA.cs:124:15:124:34 | access to field SsaFieldSink1 | -| SSA.cs:123:23:123:28 | [post] access to field S | SSA.cs:128:13:128:33 | [input] SSA def(this.S) | +| SSA.cs:123:23:123:28 | [post] access to field S | SSA.cs:123:23:123:28 | SSA def(this.S) | +| SSA.cs:123:23:123:28 | access to field S | SSA.cs:123:23:123:28 | SSA def(this.S) | | SSA.cs:123:23:123:28 | access to field S | SSA.cs:123:23:123:28 | SSA def(this.S) | -| SSA.cs:123:23:123:28 | access to field S | SSA.cs:128:13:128:33 | [input] SSA def(this.S) | | SSA.cs:124:15:124:18 | [post] this access | SSA.cs:127:9:127:12 | this access | | SSA.cs:124:15:124:18 | this access | SSA.cs:127:9:127:12 | this access | | SSA.cs:124:15:124:20 | [post] access to field S | SSA.cs:127:9:127:14 | access to field S | @@ -906,10 +896,8 @@ | SSA.cs:127:35:127:36 | "" | SSA.cs:127:9:127:31 | access to field SsaFieldNonSink0 | | SSA.cs:128:13:128:22 | [post] access to parameter nonTainted | SSA.cs:131:17:131:26 | access to parameter nonTainted | | SSA.cs:128:13:128:22 | access to parameter nonTainted | SSA.cs:131:17:131:26 | access to parameter nonTainted | -| SSA.cs:128:13:128:33 | [input] SSA def(this.S) | SSA.cs:123:23:123:28 | SSA def(this.S) | | SSA.cs:128:13:128:33 | [input] SSA phi read(this.S) | SSA.cs:136:9:136:30 | SSA phi read(this.S) | | SSA.cs:128:13:128:33 | [input] SSA phi(this.S.SsaFieldNonSink0) | SSA.cs:136:9:136:30 | SSA phi(this.S.SsaFieldNonSink0) | -| SSA.cs:128:13:128:33 | [input] SSA qualifier def(this.S.SsaFieldSink1) | SSA.cs:123:23:123:28 | SSA qualifier def(this.S.SsaFieldSink1) | | SSA.cs:130:13:130:16 | [post] this access | SSA.cs:132:21:132:24 | this access | | SSA.cs:130:13:130:16 | [post] this access | SSA.cs:134:21:134:24 | this access | | SSA.cs:130:13:130:16 | this access | SSA.cs:132:21:132:24 | this access | @@ -939,13 +927,13 @@ | SSA.cs:134:21:134:43 | [post] access to field SsaFieldNonSink0 | SSA.cs:134:17:134:44 | [input] SSA phi(this.S.SsaFieldNonSink0) | | SSA.cs:134:21:134:43 | access to field SsaFieldNonSink0 | SSA.cs:134:17:134:44 | [input] SSA phi(this.S.SsaFieldNonSink0) | | SSA.cs:136:9:136:30 | SSA phi read(this.S) | SSA.cs:136:23:136:28 | access to field S | -| SSA.cs:136:9:136:30 | SSA phi(this.S.SsaFieldNonSink0) | SSA.cs:5:17:5:17 | [input] SSA qualifier def(this.S.SsaFieldNonSink0) | +| SSA.cs:136:9:136:30 | SSA phi(this.S.SsaFieldNonSink0) | SSA.cs:136:23:136:28 | SSA qualifier def(this.S.SsaFieldNonSink0) | | SSA.cs:136:23:136:26 | [post] this access | SSA.cs:137:15:137:18 | this access | | SSA.cs:136:23:136:26 | this access | SSA.cs:137:15:137:18 | this access | | SSA.cs:136:23:136:28 | SSA def(this.S) | SSA.cs:137:15:137:20 | access to field S | | SSA.cs:136:23:136:28 | SSA qualifier def(this.S.SsaFieldNonSink0) | SSA.cs:137:15:137:37 | access to field SsaFieldNonSink0 | -| SSA.cs:136:23:136:28 | [post] access to field S | SSA.cs:5:17:5:17 | [input] SSA def(this.S) | -| SSA.cs:136:23:136:28 | access to field S | SSA.cs:5:17:5:17 | [input] SSA def(this.S) | +| SSA.cs:136:23:136:28 | [post] access to field S | SSA.cs:136:23:136:28 | SSA def(this.S) | +| SSA.cs:136:23:136:28 | access to field S | SSA.cs:136:23:136:28 | SSA def(this.S) | | SSA.cs:136:23:136:28 | access to field S | SSA.cs:136:23:136:28 | SSA def(this.S) | | SSA.cs:144:34:144:34 | SSA param(t) | SSA.cs:146:13:146:13 | access to parameter t | | SSA.cs:144:34:144:34 | t | SSA.cs:144:34:144:34 | SSA param(t) | diff --git a/csharp/ql/test/library-tests/dataflow/local/TaintTrackingStep.expected b/csharp/ql/test/library-tests/dataflow/local/TaintTrackingStep.expected index ff7d8b0f6ba2..ea0ae7f9da73 100644 --- a/csharp/ql/test/library-tests/dataflow/local/TaintTrackingStep.expected +++ b/csharp/ql/test/library-tests/dataflow/local/TaintTrackingStep.expected @@ -643,8 +643,6 @@ | LocalDataFlow.cs:381:17:381:29 | "not tainted" | LocalDataFlow.cs:381:13:381:13 | access to local variable x | | LocalDataFlow.cs:382:9:382:17 | SSA phi(x) | LocalDataFlow.cs:382:15:382:15 | access to local variable x | | SSA.cs:5:17:5:17 | SSA entry def(this.S) | SSA.cs:67:9:67:14 | access to field S | -| SSA.cs:5:17:5:17 | [input] SSA def(this.S) | SSA.cs:136:23:136:28 | SSA def(this.S) | -| SSA.cs:5:17:5:17 | [input] SSA qualifier def(this.S.SsaFieldNonSink0) | SSA.cs:136:23:136:28 | SSA qualifier def(this.S.SsaFieldNonSink0) | | SSA.cs:5:17:5:17 | this | SSA.cs:67:9:67:12 | this access | | SSA.cs:5:26:5:32 | SSA param(tainted) | SSA.cs:8:24:8:30 | access to parameter tainted | | SSA.cs:5:26:5:32 | tainted | SSA.cs:5:26:5:32 | SSA param(tainted) | @@ -780,28 +778,28 @@ | SSA.cs:58:27:58:33 | access to parameter tainted | SSA.cs:58:16:58:23 | access to local variable ssaSink3 | | SSA.cs:58:27:58:33 | access to parameter tainted | SSA.cs:67:32:67:38 | access to parameter tainted | | SSA.cs:59:23:59:30 | SSA def(ssaSink3) | SSA.cs:60:15:60:22 | access to local variable ssaSink3 | -| SSA.cs:59:23:59:30 | [post] access to local variable ssaSink3 | SSA.cs:89:13:89:33 | [input] SSA def(ssaSink3) | +| SSA.cs:59:23:59:30 | [post] access to local variable ssaSink3 | SSA.cs:59:23:59:30 | SSA def(ssaSink3) | +| SSA.cs:59:23:59:30 | access to local variable ssaSink3 | SSA.cs:59:23:59:30 | SSA def(ssaSink3) | | SSA.cs:59:23:59:30 | access to local variable ssaSink3 | SSA.cs:59:23:59:30 | SSA def(ssaSink3) | -| SSA.cs:59:23:59:30 | access to local variable ssaSink3 | SSA.cs:89:13:89:33 | [input] SSA def(ssaSink3) | | SSA.cs:63:23:63:30 | SSA def(nonSink0) | SSA.cs:64:15:64:22 | access to local variable nonSink0 | -| SSA.cs:63:23:63:30 | [post] access to local variable nonSink0 | SSA.cs:89:13:89:33 | [input] SSA def(nonSink0) | +| SSA.cs:63:23:63:30 | [post] access to local variable nonSink0 | SSA.cs:63:23:63:30 | SSA def(nonSink0) | +| SSA.cs:63:23:63:30 | access to local variable nonSink0 | SSA.cs:63:23:63:30 | SSA def(nonSink0) | | SSA.cs:63:23:63:30 | access to local variable nonSink0 | SSA.cs:63:23:63:30 | SSA def(nonSink0) | -| SSA.cs:63:23:63:30 | access to local variable nonSink0 | SSA.cs:89:13:89:33 | [input] SSA def(nonSink0) | | SSA.cs:67:9:67:12 | [post] this access | SSA.cs:68:23:68:26 | this access | | SSA.cs:67:9:67:12 | this access | SSA.cs:68:23:68:26 | this access | | SSA.cs:67:9:67:14 | [post] access to field S | SSA.cs:68:23:68:28 | access to field S | | SSA.cs:67:9:67:14 | access to field S | SSA.cs:68:23:68:28 | access to field S | | SSA.cs:67:9:67:28 | access to field SsaFieldSink0 | SSA.cs:67:9:67:38 | SSA def(this.S.SsaFieldSink0) | -| SSA.cs:67:9:67:38 | SSA def(this.S.SsaFieldSink0) | SSA.cs:89:13:89:33 | [input] SSA qualifier def(this.S.SsaFieldSink0) | +| SSA.cs:67:9:67:38 | SSA def(this.S.SsaFieldSink0) | SSA.cs:68:23:68:28 | SSA qualifier def(this.S.SsaFieldSink0) | | SSA.cs:67:32:67:38 | access to parameter tainted | SSA.cs:67:9:67:28 | access to field SsaFieldSink0 | | SSA.cs:67:32:67:38 | access to parameter tainted | SSA.cs:77:20:77:26 | access to parameter tainted | | SSA.cs:68:23:68:26 | [post] this access | SSA.cs:69:15:69:18 | this access | | SSA.cs:68:23:68:26 | this access | SSA.cs:69:15:69:18 | this access | | SSA.cs:68:23:68:28 | SSA def(this.S) | SSA.cs:69:15:69:20 | access to field S | | SSA.cs:68:23:68:28 | SSA qualifier def(this.S.SsaFieldSink0) | SSA.cs:69:15:69:34 | access to field SsaFieldSink0 | -| SSA.cs:68:23:68:28 | [post] access to field S | SSA.cs:89:13:89:33 | [input] SSA def(this.S) | +| SSA.cs:68:23:68:28 | [post] access to field S | SSA.cs:68:23:68:28 | SSA def(this.S) | +| SSA.cs:68:23:68:28 | access to field S | SSA.cs:68:23:68:28 | SSA def(this.S) | | SSA.cs:68:23:68:28 | access to field S | SSA.cs:68:23:68:28 | SSA def(this.S) | -| SSA.cs:68:23:68:28 | access to field S | SSA.cs:89:13:89:33 | [input] SSA def(this.S) | | SSA.cs:69:15:69:18 | [post] this access | SSA.cs:72:9:72:12 | this access | | SSA.cs:69:15:69:18 | this access | SSA.cs:72:9:72:12 | this access | | SSA.cs:69:15:69:20 | [post] access to field S | SSA.cs:72:9:72:14 | access to field S | @@ -811,15 +809,15 @@ | SSA.cs:72:9:72:14 | [post] access to field S | SSA.cs:73:23:73:28 | access to field S | | SSA.cs:72:9:72:14 | access to field S | SSA.cs:73:23:73:28 | access to field S | | SSA.cs:72:9:72:31 | access to field SsaFieldNonSink0 | SSA.cs:72:9:72:36 | SSA def(this.S.SsaFieldNonSink0) | -| SSA.cs:72:9:72:36 | SSA def(this.S.SsaFieldNonSink0) | SSA.cs:89:13:89:33 | [input] SSA qualifier def(this.S.SsaFieldNonSink0) | +| SSA.cs:72:9:72:36 | SSA def(this.S.SsaFieldNonSink0) | SSA.cs:73:23:73:28 | SSA qualifier def(this.S.SsaFieldNonSink0) | | SSA.cs:72:35:72:36 | "" | SSA.cs:72:9:72:31 | access to field SsaFieldNonSink0 | | SSA.cs:73:23:73:26 | [post] this access | SSA.cs:74:15:74:18 | this access | | SSA.cs:73:23:73:26 | this access | SSA.cs:74:15:74:18 | this access | | SSA.cs:73:23:73:28 | SSA def(this.S) | SSA.cs:74:15:74:20 | access to field S | | SSA.cs:73:23:73:28 | SSA qualifier def(this.S.SsaFieldNonSink0) | SSA.cs:74:15:74:37 | access to field SsaFieldNonSink0 | -| SSA.cs:73:23:73:28 | [post] access to field S | SSA.cs:89:13:89:33 | [input] SSA def(this.S) | +| SSA.cs:73:23:73:28 | [post] access to field S | SSA.cs:73:23:73:28 | SSA def(this.S) | +| SSA.cs:73:23:73:28 | access to field S | SSA.cs:73:23:73:28 | SSA def(this.S) | | SSA.cs:73:23:73:28 | access to field S | SSA.cs:73:23:73:28 | SSA def(this.S) | -| SSA.cs:73:23:73:28 | access to field S | SSA.cs:89:13:89:33 | [input] SSA def(this.S) | | SSA.cs:74:15:74:18 | [post] this access | SSA.cs:80:9:80:12 | this access | | SSA.cs:74:15:74:18 | this access | SSA.cs:80:9:80:12 | this access | | SSA.cs:74:15:74:20 | [post] access to field S | SSA.cs:80:9:80:14 | access to field S | @@ -869,15 +867,9 @@ | SSA.cs:89:13:89:22 | access to parameter nonTainted | SSA.cs:89:13:89:33 | [input] SSA phi read(nonTainted) | | SSA.cs:89:13:89:22 | access to parameter nonTainted | SSA.cs:92:17:92:26 | access to parameter nonTainted | | SSA.cs:89:13:89:29 | access to property Length | SSA.cs:89:13:89:33 | ... > ... | -| SSA.cs:89:13:89:33 | [input] SSA def(nonSink0) | SSA.cs:63:23:63:30 | SSA def(nonSink0) | -| SSA.cs:89:13:89:33 | [input] SSA def(ssaSink3) | SSA.cs:59:23:59:30 | SSA def(ssaSink3) | -| SSA.cs:89:13:89:33 | [input] SSA def(this.S) | SSA.cs:68:23:68:28 | SSA def(this.S) | -| SSA.cs:89:13:89:33 | [input] SSA def(this.S) | SSA.cs:73:23:73:28 | SSA def(this.S) | | SSA.cs:89:13:89:33 | [input] SSA phi read(nonTainted) | SSA.cs:97:9:97:32 | SSA phi read(nonTainted) | | SSA.cs:89:13:89:33 | [input] SSA phi read(ssaSink0) | SSA.cs:97:9:97:32 | SSA phi read(ssaSink0) | | SSA.cs:89:13:89:33 | [input] SSA phi(ssaSink4) | SSA.cs:97:9:97:32 | SSA phi(ssaSink4) | -| SSA.cs:89:13:89:33 | [input] SSA qualifier def(this.S.SsaFieldNonSink0) | SSA.cs:73:23:73:28 | SSA qualifier def(this.S.SsaFieldNonSink0) | -| SSA.cs:89:13:89:33 | [input] SSA qualifier def(this.S.SsaFieldSink0) | SSA.cs:68:23:68:28 | SSA qualifier def(this.S.SsaFieldSink0) | | SSA.cs:91:13:91:20 | access to local variable ssaSink4 | SSA.cs:91:13:91:31 | SSA def(ssaSink4) | | SSA.cs:91:13:91:31 | SSA def(ssaSink4) | SSA.cs:93:21:93:28 | access to local variable ssaSink4 | | SSA.cs:91:13:91:31 | SSA def(ssaSink4) | SSA.cs:95:21:95:28 | access to local variable ssaSink4 | @@ -903,9 +895,9 @@ | SSA.cs:97:9:97:32 | SSA phi read(ssaSink0) | SSA.cs:117:36:117:43 | access to local variable ssaSink0 | | SSA.cs:97:9:97:32 | SSA phi(ssaSink4) | SSA.cs:97:23:97:30 | access to local variable ssaSink4 | | SSA.cs:97:23:97:30 | SSA def(ssaSink4) | SSA.cs:98:15:98:22 | access to local variable ssaSink4 | -| SSA.cs:97:23:97:30 | [post] access to local variable ssaSink4 | SSA.cs:102:13:102:33 | [input] SSA def(ssaSink4) | +| SSA.cs:97:23:97:30 | [post] access to local variable ssaSink4 | SSA.cs:97:23:97:30 | SSA def(ssaSink4) | +| SSA.cs:97:23:97:30 | access to local variable ssaSink4 | SSA.cs:97:23:97:30 | SSA def(ssaSink4) | | SSA.cs:97:23:97:30 | access to local variable ssaSink4 | SSA.cs:97:23:97:30 | SSA def(ssaSink4) | -| SSA.cs:97:23:97:30 | access to local variable ssaSink4 | SSA.cs:102:13:102:33 | [input] SSA def(ssaSink4) | | SSA.cs:101:16:101:23 | access to local variable nonSink3 | SSA.cs:101:16:101:28 | SSA def(nonSink3) | | SSA.cs:101:16:101:28 | SSA def(nonSink3) | SSA.cs:102:13:102:33 | [input] SSA phi(nonSink3) | | SSA.cs:101:27:101:28 | "" | SSA.cs:101:16:101:23 | access to local variable nonSink3 | @@ -914,7 +906,6 @@ | SSA.cs:102:13:102:22 | access to parameter nonTainted | SSA.cs:102:13:102:33 | [input] SSA phi read(nonTainted) | | SSA.cs:102:13:102:22 | access to parameter nonTainted | SSA.cs:105:17:105:26 | access to parameter nonTainted | | SSA.cs:102:13:102:29 | access to property Length | SSA.cs:102:13:102:33 | ... > ... | -| SSA.cs:102:13:102:33 | [input] SSA def(ssaSink4) | SSA.cs:97:23:97:30 | SSA def(ssaSink4) | | SSA.cs:102:13:102:33 | [input] SSA phi read(nonSink0) | SSA.cs:110:9:110:32 | SSA phi read(nonSink0) | | SSA.cs:102:13:102:33 | [input] SSA phi read(nonTainted) | SSA.cs:110:9:110:32 | SSA phi read(nonTainted) | | SSA.cs:102:13:102:33 | [input] SSA phi(nonSink3) | SSA.cs:110:9:110:32 | SSA phi(nonSink3) | @@ -943,9 +934,9 @@ | SSA.cs:110:9:110:32 | SSA phi read(nonTainted) | SSA.cs:115:13:115:22 | access to parameter nonTainted | | SSA.cs:110:9:110:32 | SSA phi(nonSink3) | SSA.cs:110:23:110:30 | access to local variable nonSink3 | | SSA.cs:110:23:110:30 | SSA def(nonSink3) | SSA.cs:111:15:111:22 | access to local variable nonSink3 | -| SSA.cs:110:23:110:30 | [post] access to local variable nonSink3 | SSA.cs:115:13:115:33 | [input] SSA def(nonSink3) | +| SSA.cs:110:23:110:30 | [post] access to local variable nonSink3 | SSA.cs:110:23:110:30 | SSA def(nonSink3) | +| SSA.cs:110:23:110:30 | access to local variable nonSink3 | SSA.cs:110:23:110:30 | SSA def(nonSink3) | | SSA.cs:110:23:110:30 | access to local variable nonSink3 | SSA.cs:110:23:110:30 | SSA def(nonSink3) | -| SSA.cs:110:23:110:30 | access to local variable nonSink3 | SSA.cs:115:13:115:33 | [input] SSA def(nonSink3) | | SSA.cs:114:9:114:12 | [post] this access | SSA.cs:117:13:117:16 | this access | | SSA.cs:114:9:114:12 | [post] this access | SSA.cs:123:23:123:26 | this access | | SSA.cs:114:9:114:12 | this access | SSA.cs:117:13:117:16 | this access | @@ -962,7 +953,6 @@ | SSA.cs:115:13:115:22 | access to parameter nonTainted | SSA.cs:115:13:115:33 | [input] SSA phi read(nonTainted) | | SSA.cs:115:13:115:22 | access to parameter nonTainted | SSA.cs:118:17:118:26 | access to parameter nonTainted | | SSA.cs:115:13:115:29 | access to property Length | SSA.cs:115:13:115:33 | ... > ... | -| SSA.cs:115:13:115:33 | [input] SSA def(nonSink3) | SSA.cs:110:23:110:30 | SSA def(nonSink3) | | SSA.cs:115:13:115:33 | [input] SSA phi read(nonTainted) | SSA.cs:123:9:123:30 | SSA phi read(nonTainted) | | SSA.cs:115:13:115:33 | [input] SSA phi read(this.S) | SSA.cs:123:9:123:30 | SSA phi read(this.S) | | SSA.cs:115:13:115:33 | [input] SSA phi(this.S.SsaFieldSink1) | SSA.cs:123:9:123:30 | SSA phi(this.S.SsaFieldSink1) | @@ -1003,14 +993,14 @@ | SSA.cs:121:21:121:40 | access to field SsaFieldSink1 | SSA.cs:121:17:121:41 | [input] SSA phi(this.S.SsaFieldSink1) | | SSA.cs:123:9:123:30 | SSA phi read(nonTainted) | SSA.cs:128:13:128:22 | access to parameter nonTainted | | SSA.cs:123:9:123:30 | SSA phi read(this.S) | SSA.cs:123:23:123:28 | access to field S | -| SSA.cs:123:9:123:30 | SSA phi(this.S.SsaFieldSink1) | SSA.cs:128:13:128:33 | [input] SSA qualifier def(this.S.SsaFieldSink1) | +| SSA.cs:123:9:123:30 | SSA phi(this.S.SsaFieldSink1) | SSA.cs:123:23:123:28 | SSA qualifier def(this.S.SsaFieldSink1) | | SSA.cs:123:23:123:26 | [post] this access | SSA.cs:124:15:124:18 | this access | | SSA.cs:123:23:123:26 | this access | SSA.cs:124:15:124:18 | this access | | SSA.cs:123:23:123:28 | SSA def(this.S) | SSA.cs:124:15:124:20 | access to field S | | SSA.cs:123:23:123:28 | SSA qualifier def(this.S.SsaFieldSink1) | SSA.cs:124:15:124:34 | access to field SsaFieldSink1 | -| SSA.cs:123:23:123:28 | [post] access to field S | SSA.cs:128:13:128:33 | [input] SSA def(this.S) | +| SSA.cs:123:23:123:28 | [post] access to field S | SSA.cs:123:23:123:28 | SSA def(this.S) | +| SSA.cs:123:23:123:28 | access to field S | SSA.cs:123:23:123:28 | SSA def(this.S) | | SSA.cs:123:23:123:28 | access to field S | SSA.cs:123:23:123:28 | SSA def(this.S) | -| SSA.cs:123:23:123:28 | access to field S | SSA.cs:128:13:128:33 | [input] SSA def(this.S) | | SSA.cs:124:15:124:18 | [post] this access | SSA.cs:127:9:127:12 | this access | | SSA.cs:124:15:124:18 | this access | SSA.cs:127:9:127:12 | this access | | SSA.cs:124:15:124:20 | [post] access to field S | SSA.cs:127:9:127:14 | access to field S | @@ -1029,10 +1019,8 @@ | SSA.cs:128:13:128:22 | [post] access to parameter nonTainted | SSA.cs:131:17:131:26 | access to parameter nonTainted | | SSA.cs:128:13:128:22 | access to parameter nonTainted | SSA.cs:131:17:131:26 | access to parameter nonTainted | | SSA.cs:128:13:128:29 | access to property Length | SSA.cs:128:13:128:33 | ... > ... | -| SSA.cs:128:13:128:33 | [input] SSA def(this.S) | SSA.cs:123:23:123:28 | SSA def(this.S) | | SSA.cs:128:13:128:33 | [input] SSA phi read(this.S) | SSA.cs:136:9:136:30 | SSA phi read(this.S) | | SSA.cs:128:13:128:33 | [input] SSA phi(this.S.SsaFieldNonSink0) | SSA.cs:136:9:136:30 | SSA phi(this.S.SsaFieldNonSink0) | -| SSA.cs:128:13:128:33 | [input] SSA qualifier def(this.S.SsaFieldSink1) | SSA.cs:123:23:123:28 | SSA qualifier def(this.S.SsaFieldSink1) | | SSA.cs:130:13:130:16 | [post] this access | SSA.cs:132:21:132:24 | this access | | SSA.cs:130:13:130:16 | [post] this access | SSA.cs:134:21:134:24 | this access | | SSA.cs:130:13:130:16 | this access | SSA.cs:132:21:132:24 | this access | @@ -1063,13 +1051,13 @@ | SSA.cs:134:21:134:43 | [post] access to field SsaFieldNonSink0 | SSA.cs:134:17:134:44 | [input] SSA phi(this.S.SsaFieldNonSink0) | | SSA.cs:134:21:134:43 | access to field SsaFieldNonSink0 | SSA.cs:134:17:134:44 | [input] SSA phi(this.S.SsaFieldNonSink0) | | SSA.cs:136:9:136:30 | SSA phi read(this.S) | SSA.cs:136:23:136:28 | access to field S | -| SSA.cs:136:9:136:30 | SSA phi(this.S.SsaFieldNonSink0) | SSA.cs:5:17:5:17 | [input] SSA qualifier def(this.S.SsaFieldNonSink0) | +| SSA.cs:136:9:136:30 | SSA phi(this.S.SsaFieldNonSink0) | SSA.cs:136:23:136:28 | SSA qualifier def(this.S.SsaFieldNonSink0) | | SSA.cs:136:23:136:26 | [post] this access | SSA.cs:137:15:137:18 | this access | | SSA.cs:136:23:136:26 | this access | SSA.cs:137:15:137:18 | this access | | SSA.cs:136:23:136:28 | SSA def(this.S) | SSA.cs:137:15:137:20 | access to field S | | SSA.cs:136:23:136:28 | SSA qualifier def(this.S.SsaFieldNonSink0) | SSA.cs:137:15:137:37 | access to field SsaFieldNonSink0 | -| SSA.cs:136:23:136:28 | [post] access to field S | SSA.cs:5:17:5:17 | [input] SSA def(this.S) | -| SSA.cs:136:23:136:28 | access to field S | SSA.cs:5:17:5:17 | [input] SSA def(this.S) | +| SSA.cs:136:23:136:28 | [post] access to field S | SSA.cs:136:23:136:28 | SSA def(this.S) | +| SSA.cs:136:23:136:28 | access to field S | SSA.cs:136:23:136:28 | SSA def(this.S) | | SSA.cs:136:23:136:28 | access to field S | SSA.cs:136:23:136:28 | SSA def(this.S) | | SSA.cs:144:34:144:34 | SSA param(t) | SSA.cs:146:13:146:13 | access to parameter t | | SSA.cs:144:34:144:34 | t | SSA.cs:144:34:144:34 | SSA param(t) | From 449150e6b56f4f3095a73443e4630de329c39469 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Tue, 25 Feb 2025 10:42:21 +0100 Subject: [PATCH 18/20] JS: Accept fixed FP flow. --- javascript/ql/test/library-tests/TripleDot/useuse.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/ql/test/library-tests/TripleDot/useuse.js b/javascript/ql/test/library-tests/TripleDot/useuse.js index eb862d0f7b70..2a312d9aae9c 100644 --- a/javascript/ql/test/library-tests/TripleDot/useuse.js +++ b/javascript/ql/test/library-tests/TripleDot/useuse.js @@ -165,7 +165,7 @@ function t9() { // same as t8 but with a SanitizerGuard that isn't just a variab if (typeof obj === "undefined" || typeof obj === "undefined") { // The shared SSA library expects short-circuiting operators be pre-order in the CFG, // but in JS they are post-order (as per evaluation order). - sink(obj.field); // $ SPURIOUS: hasTaintFlow=t9.1 + sink(obj.field); } else { sink(obj.field); // $ hasTaintFlow=t9.1 } From b2a595596b66ec05732e7e0e00d9772ed8a5b193 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Tue, 25 Feb 2025 11:33:16 +0100 Subject: [PATCH 19/20] JS: Remove irrelevant comment. --- javascript/ql/test/library-tests/TripleDot/useuse.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/javascript/ql/test/library-tests/TripleDot/useuse.js b/javascript/ql/test/library-tests/TripleDot/useuse.js index 2a312d9aae9c..59a3f85e6721 100644 --- a/javascript/ql/test/library-tests/TripleDot/useuse.js +++ b/javascript/ql/test/library-tests/TripleDot/useuse.js @@ -163,8 +163,6 @@ function t9() { // same as t8 but with a SanitizerGuard that isn't just a variab } if (typeof obj === "undefined" || typeof obj === "undefined") { - // The shared SSA library expects short-circuiting operators be pre-order in the CFG, - // but in JS they are post-order (as per evaluation order). sink(obj.field); } else { sink(obj.field); // $ hasTaintFlow=t9.1 From 28e96449e7d860d69ee1678d0901aa43b279aeb3 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Tue, 25 Feb 2025 13:58:11 +0100 Subject: [PATCH 20/20] C#: Address review comment. --- .../code/csharp/dataflow/internal/DataFlowPrivate.qll | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll index 6c20ec294a68..11f59d59752f 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll @@ -1291,10 +1291,10 @@ class SsaNode extends NodeImpl, TSsaNode { class SsaDefinitionNode extends SsaNode { override SsaImpl::DataFlowIntegration::SsaDefinitionNode node; - SsaImpl::Definition getDefinition() { result = node.getDefinition() } + Ssa::Definition getDefinition() { result = node.getDefinition() } override ControlFlow::Node getControlFlowNodeImpl() { - result = this.getDefinition().(Ssa::Definition).getControlFlowNode() + result = this.getDefinition().getControlFlowNode() } } @@ -1688,7 +1688,7 @@ private module ReturnNodes { OutRefReturnNode() { exists(Parameter p | - this.getDefinition().(Ssa::Definition).isLiveOutRefParameterDefinition(p) and + this.getDefinition().isLiveOutRefParameterDefinition(p) and kind.getPosition() = p.getPosition() | p.isOut() and kind instanceof OutReturnKind