From 88e32ba3e15b38945c5f328737446c1767832a63 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Mon, 16 Sep 2024 18:57:03 +0100 Subject: [PATCH 1/4] PS: Add local-flow test. --- .../ql/test/library-tests/dataflow/local/test.expected | 0 powershell/ql/test/library-tests/dataflow/local/test.ps1 | 8 ++++++++ powershell/ql/test/library-tests/dataflow/local/test.ql | 6 ++++++ 3 files changed, 14 insertions(+) create mode 100644 powershell/ql/test/library-tests/dataflow/local/test.expected create mode 100644 powershell/ql/test/library-tests/dataflow/local/test.ps1 create mode 100644 powershell/ql/test/library-tests/dataflow/local/test.ql diff --git a/powershell/ql/test/library-tests/dataflow/local/test.expected b/powershell/ql/test/library-tests/dataflow/local/test.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/powershell/ql/test/library-tests/dataflow/local/test.ps1 b/powershell/ql/test/library-tests/dataflow/local/test.ps1 new file mode 100644 index 00000000000..6530b256fdc --- /dev/null +++ b/powershell/ql/test/library-tests/dataflow/local/test.ps1 @@ -0,0 +1,8 @@ +$a1 = Source() +Sink($a1) + +$b = GetBool() +if($b) { + $a2 = Source() +} +Sink($a2) \ No newline at end of file diff --git a/powershell/ql/test/library-tests/dataflow/local/test.ql b/powershell/ql/test/library-tests/dataflow/local/test.ql new file mode 100644 index 00000000000..1bea991f468 --- /dev/null +++ b/powershell/ql/test/library-tests/dataflow/local/test.ql @@ -0,0 +1,6 @@ +import powershell +import semmle.code.powershell.dataflow.DataFlow + +from DataFlow::Node pred, DataFlow::Node succ +where DataFlow::localFlowStep(pred, succ) +select pred, succ \ No newline at end of file From c87873bd26703d174c709926eca4fe34fd17acfb Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Mon, 16 Sep 2024 18:57:13 +0100 Subject: [PATCH 2/4] PS: Add more cfg classes and helper predicats. --- .../ql/lib/semmle/code/powershell/Command.qll | 17 ++++++++++ .../powershell/InvokeMemberExpression.qll | 2 +- .../code/powershell/controlflow/CfgNodes.qll | 31 +++++++++++++++++++ .../powershell/controlflow/internal/Scope.qll | 19 ++++++++++++ .../ast/Expressions/expressions.ql | 2 +- .../library-tests/dataflow/local/test.ps1 | 10 +++--- 6 files changed, 74 insertions(+), 7 deletions(-) diff --git a/powershell/ql/lib/semmle/code/powershell/Command.qll b/powershell/ql/lib/semmle/code/powershell/Command.qll index 73c2df41a7a..908a812c3ac 100644 --- a/powershell/ql/lib/semmle/code/powershell/Command.qll +++ b/powershell/ql/lib/semmle/code/powershell/Command.qll @@ -40,3 +40,20 @@ class Cmd extends @command, CmdBase { Redirection getARedirection() { result = this.getRedirection(_) } } + +/** + * An argument to a command. + * + * The argument may be named or positional. + */ +class Argument extends Expr { + Cmd cmd; + + Argument() { cmd.getArgument(_) = this or cmd.getNamedArgument(_) = this } + + Cmd getCmd() { result = cmd } + + int getIndex() { cmd.getArgument(result) = this } + + string getName() { cmd.getNamedArgument(result) = this } +} diff --git a/powershell/ql/lib/semmle/code/powershell/InvokeMemberExpression.qll b/powershell/ql/lib/semmle/code/powershell/InvokeMemberExpression.qll index 1ff04f78e3a..c337a4f08fa 100644 --- a/powershell/ql/lib/semmle/code/powershell/InvokeMemberExpression.qll +++ b/powershell/ql/lib/semmle/code/powershell/InvokeMemberExpression.qll @@ -3,7 +3,7 @@ import powershell class InvokeMemberExpr extends @invoke_member_expression, MemberExprBase { override SourceLocation getLocation() { invoke_member_expression_location(this, result) } - Expr getBase() { invoke_member_expression(this, result, _) } + Expr getQualifier() { invoke_member_expression(this, result, _) } CmdElement getMember() { invoke_member_expression(this, _, result) } diff --git a/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll b/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll index a4f83847963..a94c43ff057 100644 --- a/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll +++ b/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll @@ -166,6 +166,37 @@ module ExprNodes { predicate isImplicitWrite() { e.isImplicit() } } + + /** A control-flow node that wraps an argument expression. */ + class ArgumentCfgNode extends ExprCfgNode { + override string getAPrimaryQlClass() { result = "ArgumentCfgNode" } + + override Argument e; + + final override Argument getExpr() { result = super.getExpr() } + } + + private class InvokeMemberChildMapping extends ExprChildMapping, InvokeMemberExpr { + override predicate relevantChild(Ast n) { n = this.getQualifier() or n = this.getAnArgument() } + } + + /** A control-flow node that wraps an `InvokeMemberExpr` expression. */ + class InvokeMemberCfgNode extends ExprCfgNode { + override string getAPrimaryQlClass() { result = "InvokeMemberCfgNode" } + + override InvokeMemberChildMapping e; + + final override InvokeMemberExpr getExpr() { result = super.getExpr() } + + final ExprCfgNode getQualifier() { e.hasCfgChild(e.getQualifier(), this, result) } + } + + /** A control-flow node that wraps a qualifier expression. */ + class QualifierCfgNode extends ExprCfgNode { + QualifierCfgNode() { this = any(InvokeMemberCfgNode invoke).getQualifier() } + + InvokeMemberCfgNode getInvokeMember() { this = result.getQualifier() } + } } module StmtNodes { diff --git a/powershell/ql/lib/semmle/code/powershell/controlflow/internal/Scope.qll b/powershell/ql/lib/semmle/code/powershell/controlflow/internal/Scope.qll index 1a23cacec35..84b794dbee8 100644 --- a/powershell/ql/lib/semmle/code/powershell/controlflow/internal/Scope.qll +++ b/powershell/ql/lib/semmle/code/powershell/controlflow/internal/Scope.qll @@ -17,4 +17,23 @@ Scope scopeOf(Ast n) { class Scope extends Ast, @script_block { /** Gets the outer scope, if any. */ Scope getOuterScope() { result = scopeOf(this) } + + /** + * Gets the `i`'th paramter in this scope. + * + * This may be both function paramters and parameter block parameters. + */ + Parameter getParameter(int i) { + exists(Function func | + func.getBody() = this and + result = func.getParameter(i) + ) + } + + /** + * Gets a paramter in this scope. + * + * This may be both function paramters and parameter block parameters. + */ + Parameter getAParameter() { result = this.getParameter(_) } } diff --git a/powershell/ql/test/library-tests/ast/Expressions/expressions.ql b/powershell/ql/test/library-tests/ast/Expressions/expressions.ql index 22d68304d1f..6f3064f5c40 100644 --- a/powershell/ql/test/library-tests/ast/Expressions/expressions.ql +++ b/powershell/ql/test/library-tests/ast/Expressions/expressions.ql @@ -10,7 +10,7 @@ query predicate cmdExpr(CmdExpr cmd, Expr e) { } query predicate invokeMemoryExpression(InvokeMemberExpr invoke, Expr e, int i, Expr arg) { - e = invoke.getBase() and + e = invoke.getQualifier() and arg = invoke.getArgument(i) } diff --git a/powershell/ql/test/library-tests/dataflow/local/test.ps1 b/powershell/ql/test/library-tests/dataflow/local/test.ps1 index 6530b256fdc..86bbd7cadaf 100644 --- a/powershell/ql/test/library-tests/dataflow/local/test.ps1 +++ b/powershell/ql/test/library-tests/dataflow/local/test.ps1 @@ -1,8 +1,8 @@ -$a1 = Source() -Sink($a1) +$a1 = Source +Sink $a1 -$b = GetBool() +$b = GetBool if($b) { - $a2 = Source() + $a2 = Source } -Sink($a2) \ No newline at end of file +Sink $a2 \ No newline at end of file From d616506f2391eb8b11b286160002b2ecad2c09d7 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Mon, 16 Sep 2024 18:40:21 +0100 Subject: [PATCH 3/4] PS: Integrate SSA computations into dataflow. --- .../dataflow/internal/DataFlowPrivate.qll | 148 +++++++++++++++--- 1 file changed, 130 insertions(+), 18 deletions(-) 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 77a0eb804ca..9bc578a6c7a 100644 --- a/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPrivate.qll +++ b/powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPrivate.qll @@ -2,8 +2,10 @@ private import codeql.util.Boolean private import codeql.util.Unit private import powershell private import semmle.code.powershell.Cfg +private import semmle.code.powershell.dataflow.Ssa private import DataFlowPublic private import DataFlowDispatch +private import SsaImpl as SsaImpl /** Gets the callable in which this node occurs. */ DataFlowCallable nodeGetEnclosingCallable(Node n) { result = n.(NodeImpl).getEnclosingCallable() } @@ -39,9 +41,40 @@ private class ExprNodeImpl extends ExprNode, NodeImpl { override string toStringImpl() { result = this.getExprNode().toString() } } +/** Gets the SSA definition node corresponding to parameter `p`. */ +pragma[nomagic] +SsaImpl::DefinitionExt getParameterDef(Parameter p) { + exists(EntryBasicBlock bb, int i | + SsaImpl::parameterWrite(bb, i, p) and + result.definesAt(p, bb, i, _) + ) +} + /** Provides logic related to SSA. */ module SsaFlow { - // TODO + private module Impl = SsaImpl::DataFlowIntegration; + + private ParameterNodeImpl toParameterNode(SsaImpl::ParameterExt p) { + result = TNormalParameterNode(p.asParameter()) + } + + Impl::Node asNode(Node n) { + n = TSsaNode(result) + or + result.(Impl::ExprNode).getExpr() = n.asExpr() + or + result.(Impl::ExprPostUpdateNode).getExpr() = n.(PostUpdateNode).getPreUpdateNode().asExpr() + or + n = toParameterNode(result.(Impl::ParameterNode).getParameter()) + } + + predicate localFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo, boolean isUseStep) { + Impl::localFlowStep(def, asNode(nodeFrom), asNode(nodeTo), isUseStep) + } + + predicate localMustFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo) { + Impl::localMustFlowStep(def, asNode(nodeFrom), asNode(nodeTo)) + } } /** Provides predicates related to local data flow. */ @@ -54,19 +87,6 @@ module LocalFlow { predicate localMustFlowStep(Node node1, Node node2) { none() } } -/** An argument of a call (including qualifier arguments and block arguments). */ -private class Argument extends CfgNodes::ExprCfgNode { - private CfgNodes::StmtNodes::CmdCfgNode call; - private ArgumentPosition arg; - - Argument() { none() } - - /** Holds if this expression is the `i`th argument of `c`. */ - predicate isArgumentOf(CfgNodes::StmtNodes::CmdCfgNode c, ArgumentPosition pos) { - c = call and pos = arg - } -} - /** Provides logic related to captured variables. */ module VariableCapture { // TODO @@ -78,8 +98,11 @@ private module Cached { cached newtype TNode = TExprNode(CfgNodes::ExprCfgNode n) or + TSsaNode(SsaImpl::DataFlowIntegration::SsaNode node) or + TNormalParameterNode(Parameter p) or TExprPostUpdateNode(CfgNodes::ExprCfgNode n) { - none() // TODO + n instanceof CfgNodes::ExprNodes::ArgumentCfgNode or + n instanceof CfgNodes::ExprNodes::QualifierCfgNode } cached @@ -117,6 +140,60 @@ import Cached /** Holds if `n` should be hidden from path explanations. */ predicate nodeIsHidden(Node n) { none() } +/** An SSA node. */ +abstract 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 } + + /** Holds if this node should be hidden from path explanations. */ + abstract predicate isHidden(); + + 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; + + /** Gets the underlying variable. */ + Variable getVariable() { result = def.getSourceVariable() } + + override predicate isHidden() { + 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 module ParameterNodes { abstract class ParameterNodeImpl extends NodeImpl { abstract Parameter getParameter(); @@ -130,7 +207,30 @@ private module ParameterNodes { ) } } - // TODO + + /** + * The value of a normal parameter at function entry, viewed as a node in a data + * flow graph. + */ + class NormalParameterNode extends ParameterNodeImpl, TNormalParameterNode { + Parameter parameter; + + NormalParameterNode() { this = TNormalParameterNode(parameter) } + + override Parameter getParameter() { result = parameter } + + override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { + exists(CfgScope callable, int i | + callable = c.asCfgScope() and pos.isPositional(i) and callable.getParameter(i) = parameter + ) + } + + override CfgScope getCfgScope() { result.getAParameter() = parameter } + + override Location getLocationImpl() { result = parameter.getLocation() } + + override string toStringImpl() { result = parameter.toString() } + } } import ParameterNodes @@ -252,7 +352,19 @@ abstract class PostUpdateNodeImpl extends Node { } private module PostUpdateNodes { - // TODO + class ExprPostUpdateNode extends PostUpdateNodeImpl, NodeImpl, TExprPostUpdateNode { + private CfgNodes::ExprCfgNode e; + + ExprPostUpdateNode() { this = TExprPostUpdateNode(e) } + + override ExprNode getPreUpdateNode() { e = result.getExprNode() } + + override CfgScope getCfgScope() { result = e.getExpr().getEnclosingScope() } + + override Location getLocationImpl() { result = e.getLocation() } + + override string toStringImpl() { result = "[post] " + e.toString() } + } } private import PostUpdateNodes @@ -276,7 +388,7 @@ class NodeRegion instanceof Unit { predicate contains(Node n) { none() } /** Gets a best-effort total ordering. */ - int totalOrder() { none() } + int totalOrder() { result = 1 } } /** From f14e1cc782ebd575b709e039968be89b59e87f04 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Mon, 16 Sep 2024 20:32:25 +0100 Subject: [PATCH 4/4] PS: Add more expression classes and a helper class for calls. --- powershell/ql/lib/powershell.qll | 2 ++ .../ql/lib/semmle/code/powershell/Call.qll | 29 +++++++++++++++++++ .../ql/lib/semmle/code/powershell/Command.qll | 4 ++- .../lib/semmle/code/powershell/ErrorExpr.qll | 7 +++++ 4 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 powershell/ql/lib/semmle/code/powershell/Call.qll create mode 100644 powershell/ql/lib/semmle/code/powershell/ErrorExpr.qll diff --git a/powershell/ql/lib/powershell.qll b/powershell/ql/lib/powershell.qll index f62af84902a..e900e5fd3f0 100644 --- a/powershell/ql/lib/powershell.qll +++ b/powershell/ql/lib/powershell.qll @@ -70,7 +70,9 @@ import semmle.code.powershell.Pipeline import semmle.code.powershell.StringConstantExpression import semmle.code.powershell.MemberExpr import semmle.code.powershell.InvokeMemberExpression +import semmle.code.powershell.Call import semmle.code.powershell.SubExpression +import semmle.code.powershell.ErrorExpr import semmle.code.powershell.ConvertExpr import semmle.code.powershell.IndexExpr import semmle.code.powershell.HashTable diff --git a/powershell/ql/lib/semmle/code/powershell/Call.qll b/powershell/ql/lib/semmle/code/powershell/Call.qll new file mode 100644 index 00000000000..53200c1ce0a --- /dev/null +++ b/powershell/ql/lib/semmle/code/powershell/Call.qll @@ -0,0 +1,29 @@ +import powershell + +abstract private class AbstractCall extends Ast { + abstract Expr getCommand(); + + abstract Expr getArgument(int i); + + Expr getNamedArgument(string name) { none() } + + Expr getQualifier() { none() } +} + +private class CmdCall extends AbstractCall instanceof Cmd { + final override Expr getCommand() { result = Cmd.super.getCommand() } + + final override Expr getArgument(int i) { result = Cmd.super.getArgument(i) } + + final override Expr getNamedArgument(string name) { result = Cmd.super.getNamedArgument(name) } +} + +private class InvokeMemberCall extends AbstractCall instanceof InvokeMemberExpr { + final override Expr getCommand() { result = super.getMember() } + + final override Expr getArgument(int i) { result = InvokeMemberExpr.super.getArgument(i) } + + final override Expr getQualifier() { result = InvokeMemberExpr.super.getQualifier() } +} + +final class Call = AbstractCall; diff --git a/powershell/ql/lib/semmle/code/powershell/Command.qll b/powershell/ql/lib/semmle/code/powershell/Command.qll index 908a812c3ac..694162f2808 100644 --- a/powershell/ql/lib/semmle/code/powershell/Command.qll +++ b/powershell/ql/lib/semmle/code/powershell/Command.qll @@ -15,6 +15,8 @@ class Cmd extends @command, CmdBase { CmdElement getElement(int i) { command_command_element(this, i, result) } + Expr getCommand() { result = this.getElement(0) } + StringConstExpr getCmdName() { result = this.getElement(0) } Expr getArgument(int i) { @@ -43,7 +45,7 @@ class Cmd extends @command, CmdBase { /** * An argument to a command. - * + * * The argument may be named or positional. */ class Argument extends Expr { diff --git a/powershell/ql/lib/semmle/code/powershell/ErrorExpr.qll b/powershell/ql/lib/semmle/code/powershell/ErrorExpr.qll new file mode 100644 index 00000000000..2816f4390b3 --- /dev/null +++ b/powershell/ql/lib/semmle/code/powershell/ErrorExpr.qll @@ -0,0 +1,7 @@ +import powershell + +class ErrorExpr extends @error_expression, Expr { + final override SourceLocation getLocation() { error_expression_location(this, result) } + + final override string toString() { result = "error" } +}