diff --git a/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll b/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll index 1b162d11a9a..809ef64f872 100644 --- a/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll +++ b/powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll @@ -419,7 +419,10 @@ module ExprNodes { private class IndexExprWriteAccessChildMapping extends IndexExprChildMapping, IndexExprWriteAccess { - override predicate relevantChild(Ast child) { this.isExplicitWrite(child) } + override predicate relevantChild(Ast child) { + super.relevantChild(child) or + this.isExplicitWrite(child) + } } class IndexExprWriteAccessCfgNode extends IndexExprCfgNode { @@ -443,7 +446,7 @@ module ExprNodes { } private class IndexExprReadAccessChildMapping extends IndexExprChildMapping, IndexExprReadAccess { - override predicate relevantChild(Ast child) { none() } + override predicate relevantChild(Ast child) { super.relevantChild(child) } } class IndexExprReadAccessCfgNode extends IndexExprCfgNode { @@ -480,6 +483,8 @@ module ExprNodes { /** Gets the name that is used to select the callee. */ string getName() { result = e.getName() } + predicate hasName(string name) { this.getName() = name } + /** Gets the i'th positional argument to this call. */ ExprCfgNode getPositionalArgument(int i) { e.hasCfgChild(e.getPositionalArgument(i), this, result) @@ -558,7 +563,10 @@ module ExprNodes { private class MemberExprWriteAccessChildMapping extends MemberExprChildMapping, MemberExprWriteAccess { - override predicate relevantChild(Ast child) { this.isExplicitWrite(child) } + override predicate relevantChild(Ast child) { + super.relevantChild(child) or + this.isExplicitWrite(child) + } } class MemberExprWriteAccessCfgNode extends MemberExprCfgNode { @@ -584,7 +592,7 @@ module ExprNodes { private class MemberExprReadAccessChildMapping extends MemberExprChildMapping, MemberExprReadAccess { - override predicate relevantChild(Ast child) { none() } + override predicate relevantChild(Ast child) { super.relevantChild(child) } } class MemberExprReadAccessCfgNode extends MemberExprCfgNode { @@ -1322,9 +1330,7 @@ module StmtNodes { } class ConfigurationChildMapping extends NonExprChildMapping, Configuration { - override predicate relevantChild(Ast child) { - child = this.getName() or child = this.getBody() - } + override predicate relevantChild(Ast child) { child = this.getName() or child = this.getBody() } } class ConfigurationCfgNode extends StmtCfgNode { diff --git a/powershell/ql/lib/semmle/code/powershell/controlflow/ControlFlowGraph.qll b/powershell/ql/lib/semmle/code/powershell/controlflow/ControlFlowGraph.qll index b986bafc933..d8ec9bb8802 100644 --- a/powershell/ql/lib/semmle/code/powershell/controlflow/ControlFlowGraph.qll +++ b/powershell/ql/lib/semmle/code/powershell/controlflow/ControlFlowGraph.qll @@ -6,7 +6,6 @@ private import SuccessorTypes private import internal.ControlFlowGraphImpl as CfgImpl private import internal.Splitting as Splitting private import internal.Completion -private import internal.Scope /** * An AST node with an associated control-flow graph. @@ -16,7 +15,16 @@ private import internal.Scope * Note that module declarations are not themselves CFG scopes, as they are part of * the CFG of the enclosing top-level or callable. */ -class CfgScope extends Scope instanceof CfgImpl::CfgScope { } +class CfgScope extends Scope instanceof CfgImpl::CfgScope { + final CfgScope getOuterCfgScope() { + exists(Ast parent | + parent = this.getParent() and + result = CfgImpl::getCfgScope(parent) + ) + } + + Parameter getAParameter() { result = super.getAParameter() } +} /** * A control flow node. diff --git a/powershell/ql/lib/semmle/code/powershell/controlflow/internal/Completion.qll b/powershell/ql/lib/semmle/code/powershell/controlflow/internal/Completion.qll index 31669a955ab..68b602dfd93 100644 --- a/powershell/ql/lib/semmle/code/powershell/controlflow/internal/Completion.qll +++ b/powershell/ql/lib/semmle/code/powershell/controlflow/internal/Completion.qll @@ -22,9 +22,9 @@ private newtype TCompletion = TMatchingCompletion(Boolean b) or TEmptinessCompletion(Boolean isEmpty) -private predicate commandThrows(Cmd c, boolean unconditional) { - c.getNamedArgument("ErrorAction").(StringConstExpr).getValue().getValue() = "Stop" and - if c.getCommandName() = "Write-Error" then unconditional = true else unconditional = false +private predicate commandThrows(CallExpr c, boolean unconditional) { + c.getNamedArgument("ErrorAction").getValue().asString() = "Stop" and + if c.getName() = "Write-Error" then unconditional = true else unconditional = false } pragma[noinline] @@ -127,7 +127,7 @@ private predicate mustHaveMatchingCompletion(Ast n) { inMatchingContext(n) } * that `n` evaluates to determines a true/false branch successor. */ private predicate inBooleanContext(Ast n) { - n = any(IfStmt ifStmt).getACondition() + n = any(If ifStmt).getACondition() or n = any(WhileStmt whileStmt).getCondition() or @@ -165,10 +165,7 @@ private predicate inBooleanContext(Ast n) { n = pipeline.getComponent(pipeline.getNumberOfComponents() - 1) ) or - exists(CmdExpr cmdExpr | - inBooleanContext(cmdExpr) and - n = cmdExpr.getExpr() - ) + n = any(ParenExpr parent | inBooleanContext(parent)).getExpr() } /** diff --git a/powershell/ql/lib/semmle/code/powershell/controlflow/internal/ControlFlowGraphImpl.qll b/powershell/ql/lib/semmle/code/powershell/controlflow/internal/ControlFlowGraphImpl.qll index 909545088fa..25d0773b0c0 100644 --- a/powershell/ql/lib/semmle/code/powershell/controlflow/internal/ControlFlowGraphImpl.qll +++ b/powershell/ql/lib/semmle/code/powershell/controlflow/internal/ControlFlowGraphImpl.qll @@ -8,6 +8,8 @@ private import codeql.controlflow.Cfg as CfgShared private import codeql.util.Boolean private import semmle.code.powershell.controlflow.ControlFlowGraph private import Completion +private import semmle.code.powershell.ast.internal.Raw.Raw as Raw +private import semmle.code.powershell.ast.internal.TAst private module CfgInput implements CfgShared::InputSig { private import ControlFlowGraphImpl as Impl @@ -51,11 +53,11 @@ private module CfgInput implements CfgShared::InputSig { t instanceof Cfg::SuccessorTypes::ExitSuccessor } - private predicate id(Ast node1, Ast node2) { node1 = node2 } + private predicate id(Raw::Ast node1, Raw::Ast node2) { node1 = node2 } - private predicate idOf(Ast node, int id) = equivalenceRelation(id/2)(node, id) + private predicate idOf(Raw::Ast node, int id) = equivalenceRelation(id/2)(node, id) - int idOfAstNode(AstNode node) { idOf(node, result) } + int idOfAstNode(AstNode node) { idOf(toRawIncludingSynth(node), result) } int idOfCfgScope(CfgScope node) { result = idOfAstNode(node) } } @@ -78,7 +80,7 @@ private module ConditionalCompletionSplittingInput implements import CfgShared::MakeWithSplitting -class CfgScope extends Scope { +class CfgScope extends ScriptBlock { predicate entry(Ast first) { first(this, first) } predicate exit(Ast last, Completion c) { last(this, last, c) } @@ -94,9 +96,24 @@ predicate succExit(CfgScope scope, Ast last, Completion c) { scope.exit(last, c) /** Defines the CFG by dispatch on the various AST types. */ module Trees { - class ParameterBlockTree extends StandardPostOrderTree instanceof ParamBlock { - override AstNode getChildNode(int i) { - exists(Parameter p | p = super.getParameter(i) | result = p.getDefaultValue()) + class ParameterTree extends ControlFlowTree instanceof Parameter { + final override predicate propagatesAbnormal(AstNode child) { child = super.getDefaultValue() } + + final override predicate first(AstNode first) { first = this } + + final override predicate last(AstNode last, Completion c) { + last(super.getDefaultValue(), last, c) and + completionIsNormal(c) + or + not super.hasDefaultValue() and + last = this and + completionIsSimple(c) + } + + final override predicate succ(AstNode pred, AstNode succ, Completion c) { + pred = this and + first(super.getDefaultValue(), succ) and + completionIsSimple(c) } } @@ -128,12 +145,6 @@ module Trees { not exists(super.getEndBlock()) and not exists(super.getProcessBlock()) and not exists(super.getBeginBlock()) and - last(super.getParamBlock(), last, c) - or - not exists(super.getEndBlock()) and - not exists(super.getProcessBlock()) and - not exists(super.getBeginBlock()) and - not exists(super.getParamBlock()) and // No blocks at all. We end where we started this.succEntry(last, c) } @@ -141,22 +152,28 @@ module Trees { override predicate succ(AstNode pred, AstNode succ, Completion c) { this.succEntry(pred, c) and ( - first(super.getParamBlock(), succ) + first(super.getParameter(0), succ) or - not exists(super.getParamBlock()) and + not exists(super.getAParameter()) and first(super.getBeginBlock(), succ) or - not exists(super.getParamBlock()) and + not exists(super.getAParameter()) and not exists(super.getBeginBlock()) and first(super.getProcessBlock(), succ) or - not exists(super.getParamBlock()) and + not exists(super.getAParameter()) and not exists(super.getBeginBlock()) and not exists(super.getProcessBlock()) and first(super.getEndBlock(), succ) ) or - last(super.getParamBlock(), pred, c) and + exists(int i | + last(super.getParameter(i), pred, c) and + completionIsNormal(c) and + first(super.getParameter(i + 1), succ) + ) + or + last(super.getParameter(super.getNumberOfParameters() - 1), pred, c) and completionIsNormal(c) and ( first(super.getBeginBlock(), succ) @@ -190,7 +207,7 @@ module Trees { } final override predicate propagatesAbnormal(AstNode child) { - child = super.getParamBlock() or + child = super.getAParameter() or child = super.getBeginBlock() or child = super.getProcessBlock() or child = super.getEndBlock() @@ -198,7 +215,7 @@ module Trees { } class FunctionScriptBlockTree extends PreOrderTree, ScriptBlockTree { - Function func; + FunctionBase func; FunctionScriptBlockTree() { func.getBody() = this } @@ -206,7 +223,7 @@ module Trees { exists(Parameter p | p = rank[i + 1](Parameter cand, int j | - cand.hasDefaultValue() and func.getFunctionParameter(j) = cand + cand.hasDefaultValue() and func.getParameter(j) = cand | cand order by j ) and @@ -478,13 +495,11 @@ module Trees { override AstNode getChildNode(int i) { i = 0 and result = super.getQualifier() or - i = 1 and result = super.getMember() + i = 1 and result = super.getMemberExpr() } } - class CmdParameterTree extends LeafTree instanceof CmdParameter { } - - class IfStmtTree extends PreOrderTree instanceof IfStmt { + class IfTree extends PostOrderTree instanceof If { final override predicate propagatesAbnormal(AstNode child) { child = super.getACondition() or @@ -493,17 +508,9 @@ module Trees { child = super.getElse() } - final override predicate last(AstNode last, Completion c) { - last(super.getAThen(), last, c) - or - last(super.getElse(), last, c) - } + final override predicate first(AstNode first) { first(super.getCondition(0), first) } final override predicate succ(AstNode pred, AstNode succ, Completion c) { - this = pred and - first(super.getCondition(0), succ) and - completionIsSimple(c) - or exists(int i, boolean value | last(super.getCondition(i), pred, c) and value = c.(BooleanCompletion).getValue() | @@ -515,9 +522,21 @@ module Trees { first(super.getCondition(i + 1), succ) or i = super.getNumberOfConditions() - 1 and - first(super.getElse(), succ) + ( + first(super.getElse(), succ) + or + not exists(super.getElse()) and + succ = this + ) ) ) + or + ( + last(super.getAThen(), pred, c) or + last(super.getElse(), pred, c) + ) and + completionIsNormal(c) and + succ = this } } @@ -592,6 +611,12 @@ module Trees { class VarAccessTree extends LeafTree instanceof VarAccess { } + class VarTree extends LeafTree instanceof Variable { } + + class EnvVariableTree extends LeafTree instanceof EnvVariable { } + + class AutomaticVariableTree extends LeafTree instanceof AutomaticVariable { } + class BinaryExprTree extends StandardPostOrderTree instanceof BinaryExpr { override AstNode getChildNode(int i) { i = 0 and result = super.getLeft() @@ -607,7 +632,7 @@ module Trees { class ScriptBlockExprTree extends LeafTree instanceof ScriptBlockExpr { } class ConvertExprTree extends StandardPostOrderTree instanceof ConvertExpr { - override AstNode getChildNode(int i) { i = 0 and result = super.getBase() } + override AstNode getChildNode(int i) { i = 0 and result = super.getExpr() } } class IndexExprTree extends StandardPostOrderTree instanceof IndexExpr { @@ -619,13 +644,13 @@ module Trees { } class ParenExprTree extends StandardPostOrderTree instanceof ParenExpr { - override AstNode getChildNode(int i) { i = 0 and result = super.getBase() } + override AstNode getChildNode(int i) { i = 0 and result = super.getExpr() } } class TypeNameExprTree extends LeafTree instanceof TypeNameExpr { } class ArrayLiteralTree extends StandardPostOrderTree instanceof ArrayLiteral { - override AstNode getChildNode(int i) { result = super.getElement(i) } + override AstNode getChildNode(int i) { result = super.getExpr(i) } } class ArrayExprTree extends StandardPostOrderTree instanceof ArrayExpr { @@ -671,7 +696,7 @@ module Trees { class TypeConstraintTree extends LeafTree instanceof TypeConstraint { } - class TypeTree extends LeafTree instanceof Type { } + class TypeDefinitionTree extends LeafTree instanceof TypeDefinitionStmt { } class TryStmtBlock extends PreOrderTree instanceof TryStmt { final override predicate propagatesAbnormal(AstNode child) { child = super.getFinally() } @@ -745,34 +770,30 @@ module Trees { } } - class CmdExprTree extends StandardPostOrderTree instanceof CmdExpr { - override AstNode getChildNode(int i) { i = 0 and result = super.getExpr() } - } - - class CmdTree extends StandardPostOrderTree instanceof Cmd { + class CallExprTree extends StandardPostOrderTree instanceof CallExpr { override AstNode getChildNode(int i) { - i = -1 and result = super.getCommand() + i = -2 and result = super.getQualifier() + or + i = -1 and result = super.getCallee() or result = super.getArgument(i) } } + class ExprStmtTree extends StandardPreOrderTree instanceof ExprStmt { + override AstNode getChildNode(int i) { i = 0 and result = super.getExpr() } + } + class StringConstTree extends LeafTree instanceof StringConstExpr { } class PipelineTree extends StandardPreOrderTree instanceof Pipeline { override AstNode getChildNode(int i) { result = super.getComponent(i) } } - class InvokeMemberExprTree extends StandardPostOrderTree instanceof InvokeMemberExpr { - override AstNode getChildNode(int i) { - i = -1 and result = super.getQualifier() - or - result = super.getArgument(i) - } - } -} + class FunctionDefinitionStmtTree extends LeafTree instanceof FunctionDefinitionStmt { } -private import Scope + class LiteralTree extends LeafTree instanceof Literal { } +} cached private CfgScope getCfgScopeImpl(Ast n) { result = scopeOf(n) } diff --git a/powershell/ql/lib/semmle/code/powershell/controlflow/internal/Splitting.qll b/powershell/ql/lib/semmle/code/powershell/controlflow/internal/Splitting.qll index 012f284eb44..b2734c98194 100644 --- a/powershell/ql/lib/semmle/code/powershell/controlflow/internal/Splitting.qll +++ b/powershell/ql/lib/semmle/code/powershell/controlflow/internal/Splitting.qll @@ -69,6 +69,8 @@ module ConditionalCompletionSplitting { child = parent.(LogicalOrExpr).getAnOperand() or child = parent.(ConditionalExpr).getBranch(_) + or + child = parent.(ParenExpr).getExpr() ) }