Skip to content

Commit 6a5d087

Browse files
MathiasVPMathiasVP
authored
Merge pull request #207 from microsoft/switch-parameters
PS: Proper AST support for switch arguments
2 parents 7462e40 + a98a7b8 commit 6a5d087

File tree

7 files changed

+144
-38
lines changed

7 files changed

+144
-38
lines changed

‎powershell/ql/lib/semmle/code/powershell/ast/internal/Constant.qll

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
private import AstImport
2+
private import codeql.util.Boolean
23

34
private newtype TConstantValue =
45
TConstInteger(int value) {
@@ -12,15 +13,7 @@ private newtype TConstantValue =
1213
)
1314
} or
1415
TConstString(string value) { exists(Raw::StringLiteral sl | sl.getValue() = value) } or
15-
TConstBoolean(boolean value) {
16-
exists(Raw::VarAccess va |
17-
value = true and
18-
va.getUserPath() = "true"
19-
or
20-
value = false and
21-
va.getUserPath() = "false"
22-
)
23-
} or
16+
TConstBoolean(Boolean b) or
2417
TNull()
2518

2619
/** A constant value. */
@@ -61,9 +54,7 @@ class ConstInteger extends ConstantValue, TConstInteger {
6154

6255
final override string serialize() { result = this.getValue() }
6356

64-
final override ConstExpr getAnExpr() {
65-
result.getValueString() = this.getValue()
66-
}
57+
final override ConstExpr getAnExpr() { result.getValueString() = this.getValue() }
6758
}
6859

6960
/** A constant floating point value. */

‎powershell/ql/lib/semmle/code/powershell/ast/internal/Raw/Command.qll

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,15 @@ class Cmd extends @command, CmdBase {
5353

5454
Redirection getARedirection() { result = this.getRedirection(_) }
5555

56-
Expr getArgument(int i) {
56+
/**
57+
* Gets the `i`th argument to this command.
58+
*
59+
* This is either an expression, or a CmdParameter with no expression.
60+
* The latter is only used to denote switch parameters.
61+
*/
62+
CmdElement getArgument(int i) {
5763
result =
58-
rank[i + 1](CmdElement e, Expr r, int j |
64+
rank[i + 1](CmdElement e, CmdElement r, int j |
5965
(
6066
// For most commands the 0'th element is the command name ...
6167
j > 0
@@ -71,7 +77,25 @@ class Cmd extends @command, CmdBase {
7177
not e instanceof CmdParameter and
7278
r = e
7379
or
74-
r = e.(CmdParameter).getExpr()
80+
exists(CmdParameter p | e = p |
81+
// If it has an expression, use that
82+
p.getExpr() = r
83+
or
84+
// Otherwise, if it doesn't have an expression it's either
85+
// because it's of the form (1) `-Name x`, (2) `-Name -SomethingElse`,
86+
// or (3) `-Name` (with no other elements).
87+
// In (1) we use `x` as the argument, and in (2) and (3) we use
88+
// `-Name` as the argument.
89+
not exists(p.getExpr()) and
90+
(
91+
this.getElement(j + 1) instanceof CmdParameter and
92+
p = r
93+
or
94+
// Case 3
95+
not exists(this.getElement(j + 1)) and
96+
r = p
97+
)
98+
)
7599
)
76100
|
77101
r order by j
@@ -80,16 +104,23 @@ class Cmd extends @command, CmdBase {
80104

81105
Expr getNamedArgument(string name) {
82106
exists(CmdParameter p, int index |
83-
result = this.getArgument(index) and
84-
p.getName() = name
107+
p = this.getElement(index) and
108+
p.getName().toLowerCase() = name
85109
|
86-
p.getExpr() = result
110+
result = p.getExpr()
87111
or
88-
exists(int jndex |
89-
not exists(p.getExpr()) and
90-
this.getElement(jndex) = p and
91-
this.getElement(jndex + 1) = result
92-
)
112+
not exists(p.getExpr()) and
113+
// `not result instanceof CmdParameter` is implied
114+
result = this.getElement(index + 1)
115+
)
116+
}
117+
118+
CmdParameter getSwitchArgument(string name) {
119+
not exists(this.getNamedArgument(name)) and
120+
exists(int index |
121+
result = this.getElement(index) and
122+
result.getName().toLowerCase() = name and
123+
not exists(result.getExpr())
93124
)
94125
}
95126
}

‎powershell/ql/lib/semmle/code/powershell/ast/internal/Synthesis.qll

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -550,33 +550,49 @@ private module CmdExprRemoval {
550550
private module CmdArguments {
551551
private class CmdParameterRemoval extends Synthesis {
552552
override predicate child(Raw::Ast parent, ChildIndex i, Child child) {
553-
exists(Raw::Expr e |
554-
this.rawChild(parent, i, e) and
555-
child = childRef(getResultAst(e))
553+
exists(Raw::CmdElement elem | this.rawChild(parent, i, elem) |
554+
elem instanceof Raw::Expr and
555+
child = childRef(getResultAst(elem))
556+
or
557+
// By construction of `Cmd::getArgument` this `CmdParameter` does not
558+
// have an expression attached to it.
559+
elem instanceof Raw::CmdParameter and
560+
child = SynthChild(BoolLiteralKind(true))
556561
)
557562
}
558563

559-
private predicate rawChild(Raw::Cmd cmd, ChildIndex i, Raw::Expr child) {
564+
private predicate rawChild(Raw::Cmd cmd, ChildIndex i, Raw::CmdElement child) {
560565
exists(int index |
561566
i = cmdArgument(index) and
562567
child = cmd.getArgument(index)
563568
)
564569
}
565570

566571
override predicate isNamedArgument(CmdCall call, int i, string name) {
567-
exists(Raw::Cmd cmd, Raw::Expr e, Raw::CmdParameter p |
568-
this.rawChild(cmd, cmdArgument(i), e) and
572+
exists(Raw::Cmd cmd, Raw::CmdElement elem |
569573
call = getResultAst(cmd) and
570-
p.getName().toLowerCase() = name
574+
cmd.getArgument(i) = elem
571575
|
572-
p.getExpr() = e
573-
or
574-
exists(ChildIndex j, int jndex |
575-
j = cmdElement_(jndex) and
576-
not exists(p.getExpr()) and
577-
cmd.getChild(toRawChildIndex(j)) = p and
578-
cmd.getChild(toRawChildIndex(cmdElement_(jndex + 1))) = e
579-
)
576+
elem = cmd.getNamedArgument(name) or cmd.getSwitchArgument(name) = elem
577+
)
578+
}
579+
580+
final override predicate isRelevant(Raw::Ast a) {
581+
a instanceof Raw::CmdParameter and
582+
this.rawChild(_, _, a)
583+
}
584+
585+
final override Expr getResultAstImpl(Raw::Ast r) {
586+
exists(Raw::Cmd cmd, ChildIndex i |
587+
this.rawChild(cmd, i, r) and
588+
result = TBoolLiteral(cmd, i)
589+
)
590+
}
591+
592+
final override predicate booleanValue(BoolLiteral b, boolean value) {
593+
exists(Raw::Ast parent, ChildIndex i |
594+
b = TBoolLiteral(parent, i) and
595+
this.child(parent, i, SynthChild(BoolLiteralKind(value)))
580596
)
581597
}
582598
}

‎powershell/ql/test/library-tests/ast/Arguments/arguments.expected

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
positionalArguments
2+
| arguments.ps1:1:5:1:5 | 1 | 0 |
3+
namedArguments
4+
| arguments.ps1:2:8:2:8 | 1 | x |
5+
| arguments.ps1:3:8:3:8 | 1 | x |
6+
| arguments.ps1:4:5:4:6 | true | x |
7+
| arguments.ps1:6:5:6:6 | true | x |
8+
| arguments.ps1:6:8:6:9 | true | y |
9+
| arguments.ps1:7:8:7:8 | 1 | x |
10+
| arguments.ps1:7:13:7:13 | 2 | y |
11+
| arguments.ps1:8:8:8:8 | 1 | x |
12+
| arguments.ps1:8:13:8:13 | 2 | y |

‎powershell/ql/test/library-tests/ast/Arguments/arguments.ps1

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
Foo 1
2+
Foo -x 1
3+
Foo -x:1
4+
Foo -x
5+
6+
Bar -x -y
7+
Bar -x 1 -y 2
8+
Bar -x:1 -y:2

‎powershell/ql/test/library-tests/ast/Arguments/arguments.ql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import powershell
2+
3+
query predicate positionalArguments(Argument a, int p) { p = a.getPosition() }
4+
5+
query predicate namedArguments(Argument a, string name) { name = a.getName() }

‎powershell/ql/test/library-tests/ast/parent.expected

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,36 @@
1+
| Arguments/arguments.ps1:1:1:1:3 | Foo | Arguments/arguments.ps1:1:1:1:5 | Call to Foo |
2+
| Arguments/arguments.ps1:1:1:1:5 | Call to Foo | Arguments/arguments.ps1:1:1:1:5 | [Stmt] Call to Foo |
3+
| Arguments/arguments.ps1:1:1:1:5 | [Stmt] Call to Foo | Arguments/arguments.ps1:1:1:8:13 | {...} |
4+
| Arguments/arguments.ps1:1:1:8:13 | {...} | Arguments/arguments.ps1:1:1:8:13 | toplevel function for arguments.ps1 |
5+
| Arguments/arguments.ps1:1:1:8:13 | {...} | Arguments/arguments.ps1:1:1:8:13 | {...} |
6+
| Arguments/arguments.ps1:1:5:1:5 | 1 | Arguments/arguments.ps1:1:1:1:5 | Call to Foo |
7+
| Arguments/arguments.ps1:2:1:2:3 | Foo | Arguments/arguments.ps1:2:1:2:8 | Call to Foo |
8+
| Arguments/arguments.ps1:2:1:2:8 | Call to Foo | Arguments/arguments.ps1:2:1:2:8 | [Stmt] Call to Foo |
9+
| Arguments/arguments.ps1:2:1:2:8 | [Stmt] Call to Foo | Arguments/arguments.ps1:1:1:8:13 | {...} |
10+
| Arguments/arguments.ps1:2:8:2:8 | 1 | Arguments/arguments.ps1:2:1:2:8 | Call to Foo |
11+
| Arguments/arguments.ps1:3:1:3:3 | Foo | Arguments/arguments.ps1:3:1:3:8 | Call to Foo |
12+
| Arguments/arguments.ps1:3:1:3:8 | Call to Foo | Arguments/arguments.ps1:3:1:3:8 | [Stmt] Call to Foo |
13+
| Arguments/arguments.ps1:3:1:3:8 | [Stmt] Call to Foo | Arguments/arguments.ps1:1:1:8:13 | {...} |
14+
| Arguments/arguments.ps1:3:8:3:8 | 1 | Arguments/arguments.ps1:3:1:3:8 | Call to Foo |
15+
| Arguments/arguments.ps1:4:1:4:3 | Foo | Arguments/arguments.ps1:4:1:4:6 | Call to Foo |
16+
| Arguments/arguments.ps1:4:1:4:6 | Call to Foo | Arguments/arguments.ps1:4:1:4:6 | [Stmt] Call to Foo |
17+
| Arguments/arguments.ps1:4:1:4:6 | [Stmt] Call to Foo | Arguments/arguments.ps1:1:1:8:13 | {...} |
18+
| Arguments/arguments.ps1:4:5:4:6 | true | Arguments/arguments.ps1:4:1:4:6 | Call to Foo |
19+
| Arguments/arguments.ps1:6:1:6:3 | Bar | Arguments/arguments.ps1:6:1:6:9 | Call to Bar |
20+
| Arguments/arguments.ps1:6:1:6:9 | Call to Bar | Arguments/arguments.ps1:6:1:6:9 | [Stmt] Call to Bar |
21+
| Arguments/arguments.ps1:6:1:6:9 | [Stmt] Call to Bar | Arguments/arguments.ps1:1:1:8:13 | {...} |
22+
| Arguments/arguments.ps1:6:5:6:6 | true | Arguments/arguments.ps1:6:1:6:9 | Call to Bar |
23+
| Arguments/arguments.ps1:6:8:6:9 | true | Arguments/arguments.ps1:6:1:6:9 | Call to Bar |
24+
| Arguments/arguments.ps1:7:1:7:3 | Bar | Arguments/arguments.ps1:7:1:7:13 | Call to Bar |
25+
| Arguments/arguments.ps1:7:1:7:13 | Call to Bar | Arguments/arguments.ps1:7:1:7:13 | [Stmt] Call to Bar |
26+
| Arguments/arguments.ps1:7:1:7:13 | [Stmt] Call to Bar | Arguments/arguments.ps1:1:1:8:13 | {...} |
27+
| Arguments/arguments.ps1:7:8:7:8 | 1 | Arguments/arguments.ps1:7:1:7:13 | Call to Bar |
28+
| Arguments/arguments.ps1:7:13:7:13 | 2 | Arguments/arguments.ps1:7:1:7:13 | Call to Bar |
29+
| Arguments/arguments.ps1:8:1:8:3 | Bar | Arguments/arguments.ps1:8:1:8:13 | Call to Bar |
30+
| Arguments/arguments.ps1:8:1:8:13 | Call to Bar | Arguments/arguments.ps1:8:1:8:13 | [Stmt] Call to Bar |
31+
| Arguments/arguments.ps1:8:1:8:13 | [Stmt] Call to Bar | Arguments/arguments.ps1:1:1:8:13 | {...} |
32+
| Arguments/arguments.ps1:8:8:8:8 | 1 | Arguments/arguments.ps1:8:1:8:13 | Call to Bar |
33+
| Arguments/arguments.ps1:8:13:8:13 | 2 | Arguments/arguments.ps1:8:1:8:13 | Call to Bar |
134
| Arrays/Arrays.ps1:0:0:0:-1 | {...} | Arrays/Arrays.ps1:14:41:14:43 | @(...) |
235
| Arrays/Arrays.ps1:1:1:1:7 | array1 | Arrays/Arrays.ps1:1:1:1:36 | ...=... |
336
| Arrays/Arrays.ps1:1:1:1:7 | array1 | Arrays/Arrays.ps1:1:1:15:14 | {...} |
@@ -189,6 +222,8 @@
189222
| Expressions/ConvertWithSecureString.ps1:2:19:2:40 | ConvertTo-SecureString | Expressions/ConvertWithSecureString.ps1:2:19:2:79 | Call to ConvertTo-SecureString |
190223
| Expressions/ConvertWithSecureString.ps1:2:19:2:79 | Call to ConvertTo-SecureString | Expressions/ConvertWithSecureString.ps1:2:1:2:79 | ...=... |
191224
| Expressions/ConvertWithSecureString.ps1:2:50:2:59 | UserInput | Expressions/ConvertWithSecureString.ps1:2:19:2:79 | Call to ConvertTo-SecureString |
225+
| Expressions/ConvertWithSecureString.ps1:2:61:2:72 | true | Expressions/ConvertWithSecureString.ps1:2:19:2:79 | Call to ConvertTo-SecureString |
226+
| Expressions/ConvertWithSecureString.ps1:2:74:2:79 | true | Expressions/ConvertWithSecureString.ps1:2:19:2:79 | Call to ConvertTo-SecureString |
192227
| Expressions/ExpandableString.ps1:1:1:1:39 | Date: $([DateTime]::Now)\nName: $name | Expressions/ExpandableString.ps1:1:1:1:39 | [Stmt] Date: $([DateTime]::Now)\nName: $name |
193228
| Expressions/ExpandableString.ps1:1:1:1:39 | [Stmt] Date: $([DateTime]::Now)\nName: $name | Expressions/ExpandableString.ps1:1:1:1:39 | {...} |
194229
| Expressions/ExpandableString.ps1:1:1:1:39 | {...} | Expressions/ExpandableString.ps1:1:1:1:39 | toplevel function for ExpandableString.ps1 |
@@ -199,6 +234,14 @@
199234
| Expressions/ExpandableString.ps1:1:23:1:37 | [Stmt] Now | Expressions/ExpandableString.ps1:1:23:1:37 | {...} |
200235
| Expressions/ExpandableString.ps1:1:23:1:37 | {...} | Expressions/ExpandableString.ps1:1:21:1:38 | $(...) |
201236
| Expressions/ExpandableString.ps1:1:35:1:37 | Now | Expressions/ExpandableString.ps1:1:23:1:37 | Now |
237+
| Expressions/MemberExpression.ps1:1:1:2:14 | [synth] pipeline | Expressions/MemberExpression.ps1:1:1:2:14 | {...} |
238+
| Expressions/MemberExpression.ps1:1:1:2:14 | {...} | Expressions/MemberExpression.ps1:1:1:2:14 | toplevel function for MemberExpression.ps1 |
239+
| Expressions/MemberExpression.ps1:1:1:2:14 | {...} | Expressions/MemberExpression.ps1:1:1:2:14 | {...} |
240+
| Expressions/MemberExpression.ps1:1:7:1:8 | x | Expressions/MemberExpression.ps1:1:1:2:14 | {...} |
241+
| Expressions/MemberExpression.ps1:2:1:2:10 | DateTime | Expressions/MemberExpression.ps1:2:1:2:14 | ... |
242+
| Expressions/MemberExpression.ps1:2:1:2:14 | ... | Expressions/MemberExpression.ps1:2:1:2:14 | [Stmt] ... |
243+
| Expressions/MemberExpression.ps1:2:1:2:14 | [Stmt] ... | Expressions/MemberExpression.ps1:1:1:2:14 | {...} |
244+
| Expressions/MemberExpression.ps1:2:13:2:14 | x | Expressions/MemberExpression.ps1:2:1:2:14 | ... |
202245
| Expressions/SubExpression.ps1:1:1:1:11 | $(...) | Expressions/SubExpression.ps1:1:1:1:23 | Call to AddDays |
203246
| Expressions/SubExpression.ps1:1:1:1:23 | Call to AddDays | Expressions/SubExpression.ps1:1:1:1:23 | [Stmt] Call to AddDays |
204247
| Expressions/SubExpression.ps1:1:1:1:23 | [Stmt] Call to AddDays | Expressions/SubExpression.ps1:1:1:2:21 | {...} |

0 commit comments

Comments
 (0)