diff --git a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPrivate.qll b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPrivate.qll index a2437f884010..2197b71317c8 100644 --- a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPrivate.qll +++ b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPrivate.qll @@ -97,15 +97,22 @@ module SsaFlow { or result.(Impl::ExprPostUpdateNode).getExpr() = n.(PostUpdateNode).getPreUpdateNode().asExpr() or - n = toParameterNode(result.(Impl::ParameterNode).getParameter()) + exists(SsaImpl::ParameterExt p | + n = toParameterNode(p) and + p.isInitializedBy(result.(Impl::WriteDefSourceNode).getDefinition()) + ) + or + result.(Impl::WriteDefSourceNode).getDefinition().(Ssa::WriteDefinition).assigns(n.asExpr()) } - predicate localFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo, boolean isUseStep) { - Impl::localFlowStep(def, asNode(nodeFrom), asNode(nodeTo), isUseStep) + predicate localFlowStep( + SsaImpl::SsaInput::SourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep + ) { + Impl::localFlowStep(v, asNode(nodeFrom), asNode(nodeTo), isUseStep) } - predicate localMustFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo) { - Impl::localMustFlowStep(def, asNode(nodeFrom), asNode(nodeTo)) + predicate localMustFlowStep(Node nodeFrom, Node nodeTo) { + Impl::localMustFlowStep(_, asNode(nodeFrom), asNode(nodeTo)) } } @@ -179,7 +186,7 @@ module LocalFlow { } predicate localMustFlowStep(Node nodeFrom, Node nodeTo) { - SsaFlow::localMustFlowStep(_, nodeFrom, nodeTo) + SsaFlow::localMustFlowStep(nodeFrom, nodeTo) or nodeFrom = unique(FlowSummaryNode n1 | @@ -258,9 +265,7 @@ private module Cached { ( LocalFlow::localFlowStepCommon(nodeFrom, nodeTo) or - exists(SsaImpl::DefinitionExt def, boolean isUseStep | - SsaFlow::localFlowStep(def, nodeFrom, nodeTo, isUseStep) - | + exists(boolean isUseStep | SsaFlow::localFlowStep(_, nodeFrom, nodeTo, isUseStep) | isUseStep = false or isUseStep = true and @@ -293,8 +298,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(_)) } @@ -334,7 +339,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) @@ -419,57 +424,36 @@ predicate nodeIsHidden(Node n) { n.(NodeImpl).nodeIsHidden() } predicate neverSkipInPathGraph(Node n) { isReturned(n.(AstNode).getCfgNode()) } /** 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() } + Ssa::Definition getDefinition() { result = node.getDefinition() } override predicate isHidden() { - not def instanceof Ssa::WriteDefinition - or - def = getParameterDef(_) + exists(SsaImpl::Definition def | def = this.getDefinition() | + not def instanceof Ssa::WriteDefinition + 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() } -} - -class SsaInputNode extends SsaNode { - override SsaImpl::DataFlowIntegration::SsaInputNode node; - - override predicate isHidden() { any() } - - override CfgScope getCfgScope() { result = node.getDefinitionExt().getBasicBlock().getScope() } } private string getANamedArgument(CfgNodes::ExprNodes::CallExprCfgNode c) { diff --git a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/SsaImpl.qll b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/SsaImpl.qll index 44130ae12661..7e4b811e0911 100644 --- a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/SsaImpl.qll +++ b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/SsaImpl.qll @@ -207,13 +207,15 @@ private module Cached { import DataFlowIntegrationImpl cached - predicate localFlowStep(DefinitionExt def, Node nodeFrom, Node nodeTo, boolean isUseStep) { - DataFlowIntegrationImpl::localFlowStep(def, nodeFrom, nodeTo, isUseStep) + predicate localFlowStep( + SsaInput::SourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep + ) { + DataFlowIntegrationImpl::localFlowStep(v, nodeFrom, nodeTo, isUseStep) } cached - predicate localMustFlowStep(DefinitionExt def, Node nodeFrom, Node nodeTo) { - DataFlowIntegrationImpl::localMustFlowStep(def, nodeFrom, nodeTo) + predicate localMustFlowStep(SsaInput::SourceVariable v, Node nodeFrom, Node nodeTo) { + DataFlowIntegrationImpl::localMustFlowStep(v, nodeFrom, nodeTo) } signature predicate guardChecksSig(Cfg::CfgNodes::AstCfgNode g, Cfg::CfgNode e, boolean branch); @@ -346,33 +348,34 @@ class ParameterExt extends TParameterExt { } private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInputSig { - class Parameter = ParameterExt; - class Expr extends Cfg::CfgNodes::ExprCfgNode { predicate hasCfgNode(SsaInput::BasicBlock bb, int i) { this = bb.getNode(i) } } Expr getARead(Definition def) { result = Cached::getARead(def) } - predicate ssaDefAssigns(WriteDefinition def, Expr value) { - def.(Ssa::WriteDefinition).assigns(value) + predicate ssaDefHasSource(WriteDefinition def) { + any(ParameterExt p).isInitializedBy(def) or def.(Ssa::WriteDefinition).assigns(_) } - predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) { p.isInitializedBy(def) } - class Guard extends Cfg::CfgNodes::AstCfgNode { - predicate hasCfgNode(SsaInput::BasicBlock bb, int i) { this = bb.getNode(i) } + /** + * Holds if the control flow branching from `bb1` is dependent on this guard, + * and that the edge from `bb1` to `bb2` corresponds to the evaluation of this + * guard to `branch`. + */ + predicate controlsBranchEdge(SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, boolean branch) { + exists(Cfg::SuccessorTypes::ConditionalSuccessor s | + this.getBasicBlock() = bb1 and + bb2 = bb1.getASuccessor(s) and + s.getValue() = branch + ) + } } /** Holds if the guard `guard` controls block `bb` upon evaluating to `branch`. */ - predicate guardControlsBlock(Guard guard, SsaInput::BasicBlock bb, boolean branch) { none() } - - /** Gets an immediate conditional successor of basic block `bb`, if any. */ - SsaInput::BasicBlock getAConditionalBasicBlockSuccessor(SsaInput::BasicBlock bb, boolean branch) { - exists(Cfg::SuccessorTypes::ConditionalSuccessor s | - result = bb.getASuccessor(s) and - s.getValue() = branch - ) + predicate guardDirectlyControlsBlock(Guard guard, SsaInput::BasicBlock bb, boolean branch) { + none() } } diff --git a/powershell/ql/test/library-tests/dataflow/local/flow.expected b/powershell/ql/test/library-tests/dataflow/local/flow.expected index b5b5ec1a1263..dd5d6f834b31 100644 --- a/powershell/ql/test/library-tests/dataflow/local/flow.expected +++ b/powershell/ql/test/library-tests/dataflow/local/flow.expected @@ -6,11 +6,9 @@ | test.ps1:2:1:2:8 | Call to Sink | test.ps1:1:1:24:22 | pre-return value for {...} | | test.ps1:4:1:4:2 | b | test.ps1:5:4:5:5 | b | | test.ps1:4:6:4:12 | Call to GetBool | test.ps1:4:1:4:2 | b | -| test.ps1:5:1:7:1 | phi (a2) | test.ps1:8:6:8:8 | a2 | | test.ps1:5:4:5:5 | b | test.ps1:10:14:10:15 | b | -| test.ps1:6:5:6:7 | a2 | test.ps1:6:11:6:16 | [input] phi (a2) | +| test.ps1:6:5:6:7 | a2 | test.ps1:8:6:8:8 | a2 | | test.ps1:6:11:6:16 | Call to Source | test.ps1:6:5:6:7 | a2 | -| test.ps1:6:11:6:16 | [input] phi (a2) | test.ps1:5:1:7:1 | phi (a2) | | test.ps1:8:1:8:8 | Call to Sink | test.ps1:1:1:24:22 | pre-return value for {...} | | test.ps1:8:1:8:8 | Call to Sink | test.ps1:1:1:24:22 | pre-return value for {...} | | test.ps1:10:1:10:2 | c | test.ps1:11:6:11:7 | c | diff --git a/powershell/ql/test/library-tests/dataflow/local/taint.expected b/powershell/ql/test/library-tests/dataflow/local/taint.expected index cc15a9b83d17..1d2bd8ef34b3 100644 --- a/powershell/ql/test/library-tests/dataflow/local/taint.expected +++ b/powershell/ql/test/library-tests/dataflow/local/taint.expected @@ -7,11 +7,9 @@ | test.ps1:2:1:2:8 | Call to Sink | test.ps1:1:1:24:22 | pre-return value for {...} | | test.ps1:4:1:4:2 | b | test.ps1:5:4:5:5 | b | | test.ps1:4:6:4:12 | Call to GetBool | test.ps1:4:1:4:2 | b | -| test.ps1:5:1:7:1 | phi (a2) | test.ps1:8:6:8:8 | a2 | | test.ps1:5:4:5:5 | b | test.ps1:10:14:10:15 | b | -| test.ps1:6:5:6:7 | a2 | test.ps1:6:11:6:16 | [input] phi (a2) | +| test.ps1:6:5:6:7 | a2 | test.ps1:8:6:8:8 | a2 | | test.ps1:6:11:6:16 | Call to Source | test.ps1:6:5:6:7 | a2 | -| test.ps1:6:11:6:16 | [input] phi (a2) | test.ps1:5:1:7:1 | phi (a2) | | test.ps1:8:1:8:8 | Call to Sink | test.ps1:1:1:24:22 | pre-return value for {...} | | test.ps1:8:1:8:8 | Call to Sink | test.ps1:1:1:24:22 | pre-return value for {...} | | test.ps1:10:1:10:2 | c | test.ps1:11:6:11:7 | c |