Skip to content

Commit 8aa119b

Browse files
MathiasVPMathiasVP
authored
Merge pull request #120 from microsoft/flow-through-array-expr
PS: Flow through arrays
2 parents eb0f094 + bc7c893 commit 8aa119b

File tree

7 files changed

+215
-76
lines changed

7 files changed

+215
-76
lines changed

‎powershell/ql/lib/semmle/code/powershell/ArrayExpression.qll

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,22 @@ class ArrayExpr extends @array_expression, Expr {
55

66
StmtBlock getStmtBlock() { array_expression(this, result) }
77

8+
/**
9+
* Gets the i'th element of this `ArrayExpr`, if this can be determined statically.
10+
*
11+
* See `getStmtBlock` when the array elements are not known statically.
12+
*/
13+
Expr getElement(int i) {
14+
result =
15+
unique( | | this.getStmtBlock().getAStmt()).(CmdExpr).getExpr().(ArrayLiteral).getElement(i)
16+
}
17+
18+
/**
19+
* Gets an element of this `ArrayExpr`, if this can be determined statically.
20+
*
21+
* See `getStmtBlock` when the array elements are not known statically.
22+
*/
23+
Expr getAnElement() { result = this.getElement(_) }
24+
825
override string toString() { result = "@(...)" }
926
}

‎powershell/ql/lib/semmle/code/powershell/StatementBlock.qll

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
11
import powershell
2+
private import semmle.code.powershell.internal.AstEscape::Private
3+
4+
private module ReturnContainerInterpreter implements InterpretAstInputSig {
5+
class T = Ast;
6+
7+
pragma[inline]
8+
T interpret(Ast a) { result = a }
9+
}
210

311
class StmtBlock extends @statement_block, Ast {
412
override SourceLocation getLocation() { statement_block_location(this, result) }
@@ -16,4 +24,9 @@ class StmtBlock extends @statement_block, Ast {
1624
TrapStmt getATrapStmt() { result = this.getTrapStmt(_) }
1725

1826
override string toString() { result = "{...}" }
27+
28+
/** Gets an element that may escape this `StmtBlock`. */
29+
Ast getAnElement() {
30+
result = this.(AstEscape<ReturnContainerInterpreter>::Element).getAnEscapingElement()
31+
}
1932
}

‎powershell/ql/lib/semmle/code/powershell/controlflow/CfgNodes.qll

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,25 @@ class ProcessBlockCfgNode extends NamedBlockCfgNode {
201201
PipelineParameter getPipelineParameter() { result = block.getEnclosingFunction().getAParameter() }
202202
}
203203

204+
private class StmtBlockChildMapping extends NonExprChildMapping, StmtBlock {
205+
override predicate relevantChild(Ast n) { n = this.getAStmt() or n = this.getAnElement() }
206+
}
207+
208+
class StmtBlockCfgNode extends AstCfgNode {
209+
StmtBlockChildMapping block;
210+
211+
StmtBlockCfgNode() { this.getAstNode() = block }
212+
213+
StmtBlock getBlock() { result = block }
214+
215+
StmtCfgNode getStmt(int i) { block.hasCfgChild(block.getStmt(i), this, result) }
216+
217+
StmtCfgNode getAStmt() { block.hasCfgChild(block.getAStmt(), this, result) }
218+
219+
/** Gets an AST element that may be returned from this `StmtBlockCfgNode`. */
220+
AstCfgNode getAnElement() { block.hasCfgChild(block.getAnElement(), this, result) }
221+
}
222+
204223
/** Provides classes for control-flow nodes that wrap AST expressions. */
205224
module ExprNodes {
206225
private class VarAccessChildMapping extends ExprChildMapping, VarAccess {
@@ -418,6 +437,22 @@ module ExprNodes {
418437
class IndexCfgReadNode extends IndexCfgNode {
419438
IndexCfgReadNode() { this.getExpr() instanceof IndexExprRead }
420439
}
440+
441+
class ArrayExprChildMapping extends ExprChildMapping, ArrayExpr {
442+
override predicate relevantChild(Ast n) { n = this.getStmtBlock() or n = this.getAnElement() }
443+
}
444+
445+
class ArrayExprCfgNode extends ExprCfgNode {
446+
override string getAPrimaryQlClass() { result = "ArrayExprCfgNode" }
447+
448+
override ArrayExprChildMapping e;
449+
450+
ExprCfgNode getElement(int i) { e.hasCfgChild(e.getElement(i), this, result) }
451+
452+
ExprCfgNode getAnElement() { result = this.getElement(_) }
453+
454+
StmtBlockCfgNode getStmtBlock() { e.hasCfgChild(e.getStmtBlock(), this, result) }
455+
}
421456
}
422457

423458
module StmtNodes {

‎powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPrivate.qll

Lines changed: 37 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,15 @@ module LocalFlow {
102102
or
103103
nodeFrom.asExpr() = nodeTo.asStmt().(CfgNodes::StmtNodes::CmdExprCfgNode).getExpr()
104104
or
105+
exists(
106+
CfgNodes::ExprNodes::ArrayExprCfgNode arrayExpr, EscapeContainer::EscapeContainer container
107+
|
108+
nodeTo.asExpr() = arrayExpr and
109+
container = arrayExpr.getStmtBlock().getAstNode() and
110+
nodeFrom.(AstNode).getCfgNode() = container.getAnEscapingElement() and
111+
not container.mayBeMultiReturned(_)
112+
)
113+
or
105114
nodeFrom.(AstNode).getCfgNode() = nodeTo.(PreReturNodeImpl).getReturnedNode()
106115
or
107116
exists(CfgNode cfgNode |
@@ -550,100 +559,45 @@ abstract class ReturnNode extends Node {
550559
abstract ReturnKind getKind();
551560
}
552561

553-
private module ReturnNodes {
554-
/** An AST element that may produce return values when evaluated. */
555-
abstract private class ReturnContainer extends Ast {
556-
/**
557-
* Gets a direct node that will may be returned when evaluating this node.
558-
*/
559-
CfgNode getANode() { none() }
560-
561-
/** Gets a child that may produce more nodes that may be returned. */
562-
abstract ReturnContainer getAChild();
563-
564-
/**
565-
* Gets a (possibly transitive) node that may be returned when evaluating
566-
* this node.
567-
*/
568-
final CfgNode getAReturnedNode() {
569-
result = this.getANode()
562+
private module EscapeContainer {
563+
private import semmle.code.powershell.internal.AstEscape::Private
564+
565+
private module ReturnContainerInterpreter implements InterpretAstInputSig {
566+
class T = CfgNodes::AstCfgNode;
567+
568+
T interpret(Ast a) {
569+
result.(CfgNodes::ExprCfgNode).getExpr() = a
570570
or
571-
result = this.getAChild().getAReturnedNode()
571+
result.(CfgNodes::StmtCfgNode).getStmt() = a.(Cmd)
572572
}
573+
}
573574

575+
class EscapeContainer extends AstEscape<ReturnContainerInterpreter>::Element {
574576
/** Holds if `n` may be returned multiples times. */
575577
predicate mayBeMultiReturned(CfgNode n) {
576578
n = this.getANode() and
577579
n.getASuccessor+() = n
578580
or
579-
this.getAChild().mayBeMultiReturned(n)
580-
}
581-
}
582-
583-
class ScriptBlockReturnContainer extends ReturnContainer, ScriptBlock {
584-
final override ReturnContainer getAChild() { result = this.getEndBlock() }
585-
}
586-
587-
class NamedBlockReturnContainer extends ReturnContainer, NamedBlock {
588-
final override ReturnContainer getAChild() { result = this.getAStmt() }
589-
}
590-
591-
class CmdExprReturnContainer extends ReturnContainer, CmdExpr {
592-
final override CfgNodes::ExprCfgNode getANode() { result.getExpr() = this.getExpr() }
593-
594-
final override ReturnContainer getAChild() { none() }
595-
}
596-
597-
class LoopStmtReturnContainer extends ReturnContainer, LoopStmt {
598-
final override ReturnContainer getAChild() { result = this.getBody() }
599-
}
600-
601-
class StmtBlockReturnConainer extends ReturnContainer, StmtBlock {
602-
final override ReturnContainer getAChild() { result = this.getAStmt() }
603-
}
604-
605-
class TryStmtReturnContainer extends ReturnContainer, TryStmt {
606-
final override ReturnContainer getAChild() {
607-
result = this.getBody() or result = this.getACatchClause() or result = this.getFinally()
581+
this.getAChild().(EscapeContainer).mayBeMultiReturned(n)
608582
}
609583
}
584+
}
610585

611-
class ReturnStmtReturnContainer extends ReturnContainer, ReturnStmt {
612-
final override ReturnContainer getAChild() { result = this.getPipeline() }
613-
}
614-
615-
class CatchClausReturnContainer extends ReturnContainer, CatchClause {
616-
final override ReturnContainer getAChild() { result = this.getBody() }
617-
}
618-
619-
class SwitchStmtReturnContainer extends ReturnContainer, SwitchStmt {
620-
final override ReturnContainer getAChild() { result = this.getACase() }
621-
}
622-
623-
class CmdBaseReturnContainer extends ReturnContainer, CmdExpr {
624-
final override CfgNodes::ExprCfgNode getANode() { result.getExpr() = this.getExpr() }
625-
626-
final override ReturnContainer getAChild() { none() }
627-
}
628-
629-
class CmdReturnContainer extends ReturnContainer, Cmd {
630-
final override CfgNodes::StmtCfgNode getANode() { result.getStmt() = this }
631-
632-
final override ReturnContainer getAChild() { none() }
633-
}
586+
private module ReturnNodes {
587+
private import EscapeContainer
634588

635-
private predicate isReturnedImpl(CfgNodes::AstCfgNode n, ReturnContainer container) {
589+
private predicate isReturnedImpl(CfgNodes::AstCfgNode n, EscapeContainer container) {
636590
container = n.getScope() and
637-
n = container.getAReturnedNode()
591+
n = container.getAnEscapingElement()
638592
}
639593

640594
/**
641595
* Holds if `n` may be returned, and there are possibly
642596
* more than one return value from the function.
643597
*/
644598
predicate isMultiReturned(CfgNodes::AstCfgNode n) {
645-
exists(ReturnContainer container | isReturnedImpl(n, container) |
646-
strictcount(container.getAReturnedNode()) > 1
599+
exists(EscapeContainer container | isReturnedImpl(n, container) |
600+
strictcount(container.getAnEscapingElement()) > 1
647601
or
648602
container.mayBeMultiReturned(n)
649603
)
@@ -722,6 +676,15 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
722676
index = ec.getIndex().asInt()
723677
)
724678
or
679+
exists(
680+
CfgNodes::ExprNodes::ArrayExprCfgNode arrayExpr, EscapeContainer::EscapeContainer container
681+
|
682+
node2.asExpr() = arrayExpr and
683+
container = arrayExpr.getStmtBlock().getAstNode() and
684+
node1.(AstNode).getCfgNode() = container.getAnEscapingElement() and
685+
container.mayBeMultiReturned(_)
686+
)
687+
or
725688
c.isAnyElement() and
726689
exists(CfgNode cfgNode |
727690
node1 = TPreReturnNodeImpl(cfgNode, false) and

‎powershell/ql/lib/semmle/code/powershell/internal/AstEscape.qll

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
private import powershell as PS
2+
3+
/**
4+
* TODO: This whole computation cab be sped up by providing a set of "root"s and doing
5+
* a forward/backwards traversal first.
6+
*/
7+
module Private {
8+
signature module InterpretAstInputSig {
9+
/** The type on which to translate `Ast` elements during escape calculations */
10+
class T;
11+
12+
/** Interpret `a` into a `T` */
13+
T interpret(PS::Ast a);
14+
}
15+
16+
module AstEscape<InterpretAstInputSig Interpret> {
17+
private import Interpret
18+
19+
/** An AST element that may produce a value which can escape from this `Ast` when evaluated. */
20+
abstract private class ElementImpl instanceof PS::Ast {
21+
string toString() { result = super.toString() }
22+
23+
/** Gets a direct node that will may escape when evaluating this element. */
24+
T getANode() { none() }
25+
26+
/** Gets a child that may produce more elements that may escape. */
27+
abstract Element getAChild();
28+
29+
/**
30+
* Gets a (possibly transitive) element that may escape when evaluating
31+
* this element.
32+
*/
33+
final T getAnEscapingElement() {
34+
result = this.getANode()
35+
or
36+
result = this.getAChild().getAnEscapingElement()
37+
}
38+
}
39+
40+
final class Element = ElementImpl;
41+
42+
private class ScriptBlockElement extends ElementImpl instanceof PS::ScriptBlock {
43+
final override Element getAChild() { result = super.getEndBlock() }
44+
}
45+
46+
private class NamedBlockElement extends ElementImpl instanceof PS::NamedBlock {
47+
final override Element getAChild() { result = super.getAStmt() }
48+
}
49+
50+
private class CmdExprElement extends ElementImpl instanceof PS::CmdExpr {
51+
final override T getANode() { result = interpret(super.getExpr()) }
52+
53+
final override Element getAChild() { none() }
54+
}
55+
56+
private class LoopStmtElement extends ElementImpl instanceof PS::LoopStmt {
57+
final override Element getAChild() { result = super.getBody() }
58+
}
59+
60+
private class StmtBlockElement extends ElementImpl instanceof PS::StmtBlock {
61+
final override Element getAChild() { result = super.getAStmt() }
62+
}
63+
64+
private class TryStmtElement extends ElementImpl instanceof PS::TryStmt {
65+
final override Element getAChild() {
66+
result = super.getBody() or result = super.getACatchClause() or result = super.getFinally()
67+
}
68+
}
69+
70+
private class ReturnStmtElement extends ElementImpl instanceof PS::ReturnStmt {
71+
final override Element getAChild() { result = super.getPipeline() }
72+
}
73+
74+
private class CatchClausElement extends ElementImpl instanceof PS::CatchClause {
75+
final override Element getAChild() { result = super.getBody() }
76+
}
77+
78+
private class SwitchStmtElement extends ElementImpl instanceof PS::SwitchStmt {
79+
final override Element getAChild() { result = super.getACase() }
80+
}
81+
82+
private class CmdBaseElement extends ElementImpl instanceof PS::CmdExpr {
83+
final override T getANode() { result = interpret(super.getExpr()) }
84+
85+
final override Element getAChild() { none() }
86+
}
87+
88+
private class CmdElement extends ElementImpl instanceof PS::Cmd {
89+
final override T getANode() { result = interpret(this) }
90+
91+
final override Element getAChild() { none() }
92+
}
93+
}
94+
}
95+
96+
module Public { }

‎powershell/ql/test/library-tests/dataflow/fields/test.expected

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,12 @@ edges
3939
| test.ps1:35:15:35:17 | x | test.ps1:35:9:35:17 | ...,... [element 2] | provenance | |
4040
| test.ps1:38:6:38:11 | arr8 [element 2] | test.ps1:38:6:38:14 | ...[...] | provenance | |
4141
| test.ps1:39:6:39:11 | arr8 [element 2] | test.ps1:39:6:39:21 | ...[...] | provenance | |
42+
| test.ps1:41:6:41:17 | Source | test.ps1:43:17:43:19 | y | provenance | |
43+
| test.ps1:43:11:43:19 | ...,... [element 2] | test.ps1:46:6:46:11 | arr9 [element 2] | provenance | |
44+
| test.ps1:43:11:43:19 | ...,... [element 2] | test.ps1:47:6:47:11 | arr9 [element 2] | provenance | |
45+
| test.ps1:43:17:43:19 | y | test.ps1:43:11:43:19 | ...,... [element 2] | provenance | |
46+
| test.ps1:46:6:46:11 | arr9 [element 2] | test.ps1:46:6:46:14 | ...[...] | provenance | |
47+
| test.ps1:47:6:47:11 | arr9 [element 2] | test.ps1:47:6:47:21 | ...[...] | provenance | |
4248
| test.ps1:52:22:54:6 | this [field] | test.ps1:53:14:53:19 | this [field] | provenance | |
4349
| test.ps1:53:14:53:19 | this [field] | test.ps1:53:14:53:25 | field | provenance | |
4450
| test.ps1:59:1:59:9 | [post] myClass [field] | test.ps1:61:1:61:9 | myClass [field] | provenance | |
@@ -107,6 +113,13 @@ nodes
107113
| test.ps1:38:6:38:14 | ...[...] | semmle.label | ...[...] |
108114
| test.ps1:39:6:39:11 | arr8 [element 2] | semmle.label | arr8 [element 2] |
109115
| test.ps1:39:6:39:21 | ...[...] | semmle.label | ...[...] |
116+
| test.ps1:41:6:41:17 | Source | semmle.label | Source |
117+
| test.ps1:43:11:43:19 | ...,... [element 2] | semmle.label | ...,... [element 2] |
118+
| test.ps1:43:17:43:19 | y | semmle.label | y |
119+
| test.ps1:46:6:46:11 | arr9 [element 2] | semmle.label | arr9 [element 2] |
120+
| test.ps1:46:6:46:14 | ...[...] | semmle.label | ...[...] |
121+
| test.ps1:47:6:47:11 | arr9 [element 2] | semmle.label | arr9 [element 2] |
122+
| test.ps1:47:6:47:21 | ...[...] | semmle.label | ...[...] |
110123
| test.ps1:52:22:54:6 | this [field] | semmle.label | this [field] |
111124
| test.ps1:53:14:53:19 | this [field] | semmle.label | this [field] |
112125
| test.ps1:53:14:53:25 | field | semmle.label | field |
@@ -142,6 +155,8 @@ testFailures
142155
| test.ps1:31:6:31:33 | ...[...] | test.ps1:29:31:29:41 | Source | test.ps1:31:6:31:33 | ...[...] | $@ | test.ps1:29:31:29:41 | Source | Source |
143156
| test.ps1:38:6:38:14 | ...[...] | test.ps1:33:6:33:17 | Source | test.ps1:38:6:38:14 | ...[...] | $@ | test.ps1:33:6:33:17 | Source | Source |
144157
| test.ps1:39:6:39:21 | ...[...] | test.ps1:33:6:33:17 | Source | test.ps1:39:6:39:21 | ...[...] | $@ | test.ps1:33:6:33:17 | Source | Source |
158+
| test.ps1:46:6:46:14 | ...[...] | test.ps1:41:6:41:17 | Source | test.ps1:46:6:46:14 | ...[...] | $@ | test.ps1:41:6:41:17 | Source | Source |
159+
| test.ps1:47:6:47:21 | ...[...] | test.ps1:41:6:41:17 | Source | test.ps1:47:6:47:21 | ...[...] | $@ | test.ps1:41:6:41:17 | Source | Source |
145160
| test.ps1:53:14:53:25 | field | test.ps1:59:18:59:29 | Source | test.ps1:53:14:53:25 | field | $@ | test.ps1:59:18:59:29 | Source | Source |
146161
| test.ps1:72:6:72:11 | ...[...] | test.ps1:64:10:64:21 | Source | test.ps1:72:6:72:11 | ...[...] | $@ | test.ps1:64:10:64:21 | Source | Source |
147162
| test.ps1:72:6:72:11 | ...[...] | test.ps1:65:10:65:21 | Source | test.ps1:72:6:72:11 | ...[...] | $@ | test.ps1:65:10:65:21 | Source | Source |

‎powershell/ql/test/library-tests/dataflow/fields/test.ps1

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ $y = Source "11"
4343
$arr9 = @(0, 1, $y)
4444
Sink $arr9[0] # clean
4545
Sink $arr9[1] # clean
46-
Sink $arr9[2] # $ MISSING: hasValueFlow=11
47-
Sink $arr9[$unknown] # MISSING: hasValueFlow=11
46+
Sink $arr9[2] # $ hasValueFlow=11
47+
Sink $arr9[$unknown] # $ hasValueFlow=11
4848

4949
class MyClass {
5050
[string] $field

0 commit comments

Comments
 (0)