From b9fdc78c16841447d061f9e76524ecf266250aa3 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 22 Apr 2025 14:59:45 +0100 Subject: [PATCH 1/4] PS: Add argument tests. --- .../test/library-tests/ast/Arguments/arguments.expected | 9 +++++++++ .../ql/test/library-tests/ast/Arguments/arguments.ps1 | 8 ++++++++ .../ql/test/library-tests/ast/Arguments/arguments.ql | 5 +++++ powershell/ql/test/library-tests/ast/parent.expected | 8 ++++++++ 4 files changed, 30 insertions(+) create mode 100644 powershell/ql/test/library-tests/ast/Arguments/arguments.expected create mode 100644 powershell/ql/test/library-tests/ast/Arguments/arguments.ps1 create mode 100644 powershell/ql/test/library-tests/ast/Arguments/arguments.ql diff --git a/powershell/ql/test/library-tests/ast/Arguments/arguments.expected b/powershell/ql/test/library-tests/ast/Arguments/arguments.expected new file mode 100644 index 00000000000..030de88158e --- /dev/null +++ b/powershell/ql/test/library-tests/ast/Arguments/arguments.expected @@ -0,0 +1,9 @@ +positionalArguments +| arguments.ps1:1:5:1:5 | 1 | 0 | +namedArguments +| arguments.ps1:2:8:2:8 | 1 | x | +| arguments.ps1:3:8:3:8 | 1 | x | +| arguments.ps1:7:8:7:8 | 1 | x | +| arguments.ps1:7:13:7:13 | 2 | y | +| arguments.ps1:8:8:8:8 | 1 | x | +| arguments.ps1:8:13:8:13 | 2 | y | diff --git a/powershell/ql/test/library-tests/ast/Arguments/arguments.ps1 b/powershell/ql/test/library-tests/ast/Arguments/arguments.ps1 new file mode 100644 index 00000000000..88f98d2dbed --- /dev/null +++ b/powershell/ql/test/library-tests/ast/Arguments/arguments.ps1 @@ -0,0 +1,8 @@ +Foo 1 +Foo -x 1 +Foo -x:1 +Foo -x + +Bar -x -y +Bar -x 1 -y 2 +Bar -x:1 -y:2 \ No newline at end of file diff --git a/powershell/ql/test/library-tests/ast/Arguments/arguments.ql b/powershell/ql/test/library-tests/ast/Arguments/arguments.ql new file mode 100644 index 00000000000..fb5ab3c699f --- /dev/null +++ b/powershell/ql/test/library-tests/ast/Arguments/arguments.ql @@ -0,0 +1,5 @@ +import powershell + +query predicate positionalArguments(Argument a, int p) { p = a.getPosition() } + +query predicate namedArguments(Argument a, string name) { name = a.getName() } diff --git a/powershell/ql/test/library-tests/ast/parent.expected b/powershell/ql/test/library-tests/ast/parent.expected index 7c27e930667..026e6f3ff6a 100644 --- a/powershell/ql/test/library-tests/ast/parent.expected +++ b/powershell/ql/test/library-tests/ast/parent.expected @@ -199,6 +199,14 @@ | Expressions/ExpandableString.ps1:1:23:1:37 | [Stmt] Now | Expressions/ExpandableString.ps1:1:23:1:37 | {...} | | Expressions/ExpandableString.ps1:1:23:1:37 | {...} | Expressions/ExpandableString.ps1:1:21:1:38 | $(...) | | Expressions/ExpandableString.ps1:1:35:1:37 | Now | Expressions/ExpandableString.ps1:1:23:1:37 | Now | +| Expressions/MemberExpression.ps1:1:1:2:14 | [synth] pipeline | Expressions/MemberExpression.ps1:1:1:2:14 | {...} | +| Expressions/MemberExpression.ps1:1:1:2:14 | {...} | Expressions/MemberExpression.ps1:1:1:2:14 | toplevel function for MemberExpression.ps1 | +| Expressions/MemberExpression.ps1:1:1:2:14 | {...} | Expressions/MemberExpression.ps1:1:1:2:14 | {...} | +| Expressions/MemberExpression.ps1:1:7:1:8 | x | Expressions/MemberExpression.ps1:1:1:2:14 | {...} | +| Expressions/MemberExpression.ps1:2:1:2:10 | DateTime | Expressions/MemberExpression.ps1:2:1:2:14 | ... | +| Expressions/MemberExpression.ps1:2:1:2:14 | ... | Expressions/MemberExpression.ps1:2:1:2:14 | [Stmt] ... | +| Expressions/MemberExpression.ps1:2:1:2:14 | [Stmt] ... | Expressions/MemberExpression.ps1:1:1:2:14 | {...} | +| Expressions/MemberExpression.ps1:2:13:2:14 | x | Expressions/MemberExpression.ps1:2:1:2:14 | ... | | Expressions/SubExpression.ps1:1:1:1:11 | $(...) | Expressions/SubExpression.ps1:1:1:1:23 | Call to AddDays | | Expressions/SubExpression.ps1:1:1:1:23 | Call to AddDays | Expressions/SubExpression.ps1:1:1:1:23 | [Stmt] Call to AddDays | | Expressions/SubExpression.ps1:1:1:1:23 | [Stmt] Call to AddDays | Expressions/SubExpression.ps1:1:1:2:21 | {...} | From 72266cb0004a09a67605c08048f97ad579cd4dfa Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 22 Apr 2025 15:00:49 +0100 Subject: [PATCH 2/4] PS: Drive-by cleanup in Constant.qll --- .../code/powershell/ast/internal/Constant.qll | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/powershell/ql/lib/semmle/code/powershell/ast/internal/Constant.qll b/powershell/ql/lib/semmle/code/powershell/ast/internal/Constant.qll index f39a9bdfff7..b68cc5f6e49 100644 --- a/powershell/ql/lib/semmle/code/powershell/ast/internal/Constant.qll +++ b/powershell/ql/lib/semmle/code/powershell/ast/internal/Constant.qll @@ -1,4 +1,5 @@ private import AstImport +private import codeql.util.Boolean private newtype TConstantValue = TConstInteger(int value) { @@ -12,15 +13,7 @@ private newtype TConstantValue = ) } or TConstString(string value) { exists(Raw::StringLiteral sl | sl.getValue() = value) } or - TConstBoolean(boolean value) { - exists(Raw::VarAccess va | - value = true and - va.getUserPath() = "true" - or - value = false and - va.getUserPath() = "false" - ) - } or + TConstBoolean(Boolean b) or TNull() /** A constant value. */ @@ -61,9 +54,7 @@ class ConstInteger extends ConstantValue, TConstInteger { final override string serialize() { result = this.getValue() } - final override ConstExpr getAnExpr() { - result.getValueString() = this.getValue() - } + final override ConstExpr getAnExpr() { result.getValueString() = this.getValue() } } /** A constant floating point value. */ From e9fd50b67c35dc1da7a74d0d3d39f564dd277580 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 22 Apr 2025 15:03:21 +0100 Subject: [PATCH 3/4] PS: Handle switch arguments by synthesizing a boolean true literal and represent them as named arguments. --- .../powershell/ast/internal/Raw/Command.qll | 53 +++++++++++++++---- .../powershell/ast/internal/Synthesis.qll | 46 ++++++++++------ 2 files changed, 73 insertions(+), 26 deletions(-) diff --git a/powershell/ql/lib/semmle/code/powershell/ast/internal/Raw/Command.qll b/powershell/ql/lib/semmle/code/powershell/ast/internal/Raw/Command.qll index 7186de94065..da889b1f573 100644 --- a/powershell/ql/lib/semmle/code/powershell/ast/internal/Raw/Command.qll +++ b/powershell/ql/lib/semmle/code/powershell/ast/internal/Raw/Command.qll @@ -53,9 +53,15 @@ class Cmd extends @command, CmdBase { Redirection getARedirection() { result = this.getRedirection(_) } - Expr getArgument(int i) { + /** + * Gets the `i`th argument to this command. + * + * This is either an expression, or a CmdParameter with no expression. + * The latter is only used to denote switch parameters. + */ + CmdElement getArgument(int i) { result = - rank[i + 1](CmdElement e, Expr r, int j | + rank[i + 1](CmdElement e, CmdElement r, int j | ( // For most commands the 0'th element is the command name ... j > 0 @@ -71,7 +77,25 @@ class Cmd extends @command, CmdBase { not e instanceof CmdParameter and r = e or - r = e.(CmdParameter).getExpr() + exists(CmdParameter p | e = p | + // If it has an expression, use that + p.getExpr() = r + or + // Otherwise, if it doesn't have an expression it's either + // because it's of the form (1) `-Name x`, (2) `-Name -SomethingElse`, + // or (3) `-Name` (with no other elements). + // In (1) we use `x` as the argument, and in (2) and (3) we use + // `-Name` as the argument. + not exists(p.getExpr()) and + ( + this.getElement(j + 1) instanceof CmdParameter and + p = r + or + // Case 3 + not exists(this.getElement(j + 1)) and + r = p + ) + ) ) | r order by j @@ -80,16 +104,23 @@ class Cmd extends @command, CmdBase { Expr getNamedArgument(string name) { exists(CmdParameter p, int index | - result = this.getArgument(index) and - p.getName() = name + p = this.getElement(index) and + p.getName().toLowerCase() = name | - p.getExpr() = result + result = p.getExpr() or - exists(int jndex | - not exists(p.getExpr()) and - this.getElement(jndex) = p and - this.getElement(jndex + 1) = result - ) + not exists(p.getExpr()) and + // `not result instanceof CmdParameter` is implied + result = this.getElement(index + 1) + ) + } + + CmdParameter getSwitchArgument(string name) { + not exists(this.getNamedArgument(name)) and + exists(int index | + result = this.getElement(index) and + result.getName().toLowerCase() = name and + not exists(result.getExpr()) ) } } diff --git a/powershell/ql/lib/semmle/code/powershell/ast/internal/Synthesis.qll b/powershell/ql/lib/semmle/code/powershell/ast/internal/Synthesis.qll index b6634ad7dc0..24cb4d323b6 100644 --- a/powershell/ql/lib/semmle/code/powershell/ast/internal/Synthesis.qll +++ b/powershell/ql/lib/semmle/code/powershell/ast/internal/Synthesis.qll @@ -550,13 +550,18 @@ private module CmdExprRemoval { private module CmdArguments { private class CmdParameterRemoval extends Synthesis { override predicate child(Raw::Ast parent, ChildIndex i, Child child) { - exists(Raw::Expr e | - this.rawChild(parent, i, e) and - child = childRef(getResultAst(e)) + exists(Raw::CmdElement elem | this.rawChild(parent, i, elem) | + elem instanceof Raw::Expr and + child = childRef(getResultAst(elem)) + or + // By construction of `Cmd::getArgument` this `CmdParameter` does not + // have an expression attached to it. + elem instanceof Raw::CmdParameter and + child = SynthChild(BoolLiteralKind(true)) ) } - private predicate rawChild(Raw::Cmd cmd, ChildIndex i, Raw::Expr child) { + private predicate rawChild(Raw::Cmd cmd, ChildIndex i, Raw::CmdElement child) { exists(int index | i = cmdArgument(index) and child = cmd.getArgument(index) @@ -564,19 +569,30 @@ private module CmdArguments { } override predicate isNamedArgument(CmdCall call, int i, string name) { - exists(Raw::Cmd cmd, Raw::Expr e, Raw::CmdParameter p | - this.rawChild(cmd, cmdArgument(i), e) and + exists(Raw::Cmd cmd, Raw::CmdElement elem | call = getResultAst(cmd) and - p.getName().toLowerCase() = name + cmd.getArgument(i) = elem | - p.getExpr() = e - or - exists(ChildIndex j, int jndex | - j = cmdElement_(jndex) and - not exists(p.getExpr()) and - cmd.getChild(toRawChildIndex(j)) = p and - cmd.getChild(toRawChildIndex(cmdElement_(jndex + 1))) = e - ) + elem = cmd.getNamedArgument(name) or cmd.getSwitchArgument(name) = elem + ) + } + + final override predicate isRelevant(Raw::Ast a) { + a instanceof Raw::CmdParameter and + this.rawChild(_, _, a) + } + + final override Expr getResultAstImpl(Raw::Ast r) { + exists(Raw::Cmd cmd, ChildIndex i | + this.rawChild(cmd, i, r) and + result = TBoolLiteral(cmd, i) + ) + } + + final override predicate booleanValue(BoolLiteral b, boolean value) { + exists(Raw::Ast parent, ChildIndex i | + b = TBoolLiteral(parent, i) and + this.child(parent, i, SynthChild(BoolLiteralKind(value))) ) } } From 09ebc76a2302dade1f0cfd0344c27d169c9220af Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 22 Apr 2025 15:32:35 +0100 Subject: [PATCH 4/4] PS: Accept test changes. --- .../ast/Arguments/arguments.expected | 3 ++ .../ql/test/library-tests/ast/parent.expected | 35 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/powershell/ql/test/library-tests/ast/Arguments/arguments.expected b/powershell/ql/test/library-tests/ast/Arguments/arguments.expected index 030de88158e..3aee831fcb3 100644 --- a/powershell/ql/test/library-tests/ast/Arguments/arguments.expected +++ b/powershell/ql/test/library-tests/ast/Arguments/arguments.expected @@ -3,6 +3,9 @@ positionalArguments namedArguments | arguments.ps1:2:8:2:8 | 1 | x | | arguments.ps1:3:8:3:8 | 1 | x | +| arguments.ps1:4:5:4:6 | true | x | +| arguments.ps1:6:5:6:6 | true | x | +| arguments.ps1:6:8:6:9 | true | y | | arguments.ps1:7:8:7:8 | 1 | x | | arguments.ps1:7:13:7:13 | 2 | y | | arguments.ps1:8:8:8:8 | 1 | x | diff --git a/powershell/ql/test/library-tests/ast/parent.expected b/powershell/ql/test/library-tests/ast/parent.expected index 026e6f3ff6a..c4462fbebf8 100644 --- a/powershell/ql/test/library-tests/ast/parent.expected +++ b/powershell/ql/test/library-tests/ast/parent.expected @@ -1,3 +1,36 @@ +| Arguments/arguments.ps1:1:1:1:3 | Foo | Arguments/arguments.ps1:1:1:1:5 | Call to Foo | +| Arguments/arguments.ps1:1:1:1:5 | Call to Foo | Arguments/arguments.ps1:1:1:1:5 | [Stmt] Call to Foo | +| Arguments/arguments.ps1:1:1:1:5 | [Stmt] Call to Foo | Arguments/arguments.ps1:1:1:8:13 | {...} | +| Arguments/arguments.ps1:1:1:8:13 | {...} | Arguments/arguments.ps1:1:1:8:13 | toplevel function for arguments.ps1 | +| Arguments/arguments.ps1:1:1:8:13 | {...} | Arguments/arguments.ps1:1:1:8:13 | {...} | +| Arguments/arguments.ps1:1:5:1:5 | 1 | Arguments/arguments.ps1:1:1:1:5 | Call to Foo | +| Arguments/arguments.ps1:2:1:2:3 | Foo | Arguments/arguments.ps1:2:1:2:8 | Call to Foo | +| Arguments/arguments.ps1:2:1:2:8 | Call to Foo | Arguments/arguments.ps1:2:1:2:8 | [Stmt] Call to Foo | +| Arguments/arguments.ps1:2:1:2:8 | [Stmt] Call to Foo | Arguments/arguments.ps1:1:1:8:13 | {...} | +| Arguments/arguments.ps1:2:8:2:8 | 1 | Arguments/arguments.ps1:2:1:2:8 | Call to Foo | +| Arguments/arguments.ps1:3:1:3:3 | Foo | Arguments/arguments.ps1:3:1:3:8 | Call to Foo | +| Arguments/arguments.ps1:3:1:3:8 | Call to Foo | Arguments/arguments.ps1:3:1:3:8 | [Stmt] Call to Foo | +| Arguments/arguments.ps1:3:1:3:8 | [Stmt] Call to Foo | Arguments/arguments.ps1:1:1:8:13 | {...} | +| Arguments/arguments.ps1:3:8:3:8 | 1 | Arguments/arguments.ps1:3:1:3:8 | Call to Foo | +| Arguments/arguments.ps1:4:1:4:3 | Foo | Arguments/arguments.ps1:4:1:4:6 | Call to Foo | +| Arguments/arguments.ps1:4:1:4:6 | Call to Foo | Arguments/arguments.ps1:4:1:4:6 | [Stmt] Call to Foo | +| Arguments/arguments.ps1:4:1:4:6 | [Stmt] Call to Foo | Arguments/arguments.ps1:1:1:8:13 | {...} | +| Arguments/arguments.ps1:4:5:4:6 | true | Arguments/arguments.ps1:4:1:4:6 | Call to Foo | +| Arguments/arguments.ps1:6:1:6:3 | Bar | Arguments/arguments.ps1:6:1:6:9 | Call to Bar | +| Arguments/arguments.ps1:6:1:6:9 | Call to Bar | Arguments/arguments.ps1:6:1:6:9 | [Stmt] Call to Bar | +| Arguments/arguments.ps1:6:1:6:9 | [Stmt] Call to Bar | Arguments/arguments.ps1:1:1:8:13 | {...} | +| Arguments/arguments.ps1:6:5:6:6 | true | Arguments/arguments.ps1:6:1:6:9 | Call to Bar | +| Arguments/arguments.ps1:6:8:6:9 | true | Arguments/arguments.ps1:6:1:6:9 | Call to Bar | +| Arguments/arguments.ps1:7:1:7:3 | Bar | Arguments/arguments.ps1:7:1:7:13 | Call to Bar | +| Arguments/arguments.ps1:7:1:7:13 | Call to Bar | Arguments/arguments.ps1:7:1:7:13 | [Stmt] Call to Bar | +| Arguments/arguments.ps1:7:1:7:13 | [Stmt] Call to Bar | Arguments/arguments.ps1:1:1:8:13 | {...} | +| Arguments/arguments.ps1:7:8:7:8 | 1 | Arguments/arguments.ps1:7:1:7:13 | Call to Bar | +| Arguments/arguments.ps1:7:13:7:13 | 2 | Arguments/arguments.ps1:7:1:7:13 | Call to Bar | +| Arguments/arguments.ps1:8:1:8:3 | Bar | Arguments/arguments.ps1:8:1:8:13 | Call to Bar | +| Arguments/arguments.ps1:8:1:8:13 | Call to Bar | Arguments/arguments.ps1:8:1:8:13 | [Stmt] Call to Bar | +| Arguments/arguments.ps1:8:1:8:13 | [Stmt] Call to Bar | Arguments/arguments.ps1:1:1:8:13 | {...} | +| Arguments/arguments.ps1:8:8:8:8 | 1 | Arguments/arguments.ps1:8:1:8:13 | Call to Bar | +| Arguments/arguments.ps1:8:13:8:13 | 2 | Arguments/arguments.ps1:8:1:8:13 | Call to Bar | | Arrays/Arrays.ps1:0:0:0:-1 | {...} | Arrays/Arrays.ps1:14:41:14:43 | @(...) | | Arrays/Arrays.ps1:1:1:1:7 | array1 | Arrays/Arrays.ps1:1:1:1:36 | ...=... | | Arrays/Arrays.ps1:1:1:1:7 | array1 | Arrays/Arrays.ps1:1:1:15:14 | {...} | @@ -189,6 +222,8 @@ | Expressions/ConvertWithSecureString.ps1:2:19:2:40 | ConvertTo-SecureString | Expressions/ConvertWithSecureString.ps1:2:19:2:79 | Call to ConvertTo-SecureString | | Expressions/ConvertWithSecureString.ps1:2:19:2:79 | Call to ConvertTo-SecureString | Expressions/ConvertWithSecureString.ps1:2:1:2:79 | ...=... | | Expressions/ConvertWithSecureString.ps1:2:50:2:59 | UserInput | Expressions/ConvertWithSecureString.ps1:2:19:2:79 | Call to ConvertTo-SecureString | +| Expressions/ConvertWithSecureString.ps1:2:61:2:72 | true | Expressions/ConvertWithSecureString.ps1:2:19:2:79 | Call to ConvertTo-SecureString | +| Expressions/ConvertWithSecureString.ps1:2:74:2:79 | true | Expressions/ConvertWithSecureString.ps1:2:19:2:79 | Call to ConvertTo-SecureString | | 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 | | Expressions/ExpandableString.ps1:1:1:1:39 | [Stmt] Date: $([DateTime]::Now)\nName: $name | Expressions/ExpandableString.ps1:1:1:1:39 | {...} | | Expressions/ExpandableString.ps1:1:1:1:39 | {...} | Expressions/ExpandableString.ps1:1:1:1:39 | toplevel function for ExpandableString.ps1 |