Skip to content

Commit bfa9210

Browse files
MathiasVPMathiasVP
authored
Merge pull request #123 from microsoft/flow-through-ValueFromPipelineByPropertyName
PS: Flow through `ValueFromPipelineByPropertyName` parameters
2 parents a16b51a + 34c821f commit bfa9210

File tree

8 files changed

+80
-21
lines changed

8 files changed

+80
-21
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,10 @@ abstract private class AbstractFunction extends Ast {
5151
result.getIndex() = i
5252
}
5353

54-
final Parameter getParameterExcludingPipline(int i) {
54+
final Parameter getParameterExcludingPiplines(int i) {
5555
result = this.getFunctionParameter(i)
5656
or
57-
result = this.getBody().getParamBlock().getParameterExcludingPipline(i)
57+
result = this.getBody().getParamBlock().getParameterExcludingPiplines(i)
5858
}
5959

6060
final Parameter getThisParameter() {

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,7 @@ class NamedAttributeArgument extends @named_attribute_argument, Ast {
1313
class ValueFromPipelineAttribute extends NamedAttributeArgument {
1414
ValueFromPipelineAttribute() { this.getName() = "ValueFromPipeline" }
1515
}
16+
17+
class ValueFromPipelineByPropertyName extends NamedAttributeArgument {
18+
ValueFromPipelineByPropertyName() { this.getName() = "ValueFromPipelineByPropertyName" }
19+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ class ParamBlock extends @param_block, Ast {
1515

1616
Parameter getParameter(int i) { result.hasParameterBlock(this, i) }
1717

18-
Parameter getParameterExcludingPipline(int i) { result.hasParameterBlockExcludingPipeline(this, i) }
18+
Parameter getParameterExcludingPiplines(int i) { result.hasParameterBlockExcludingPipelines(this, i) }
1919

2020
Parameter getAParameter() { result = this.getParameter(_) }
2121
}

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

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,16 @@ private predicate hasParameterBlockImpl(Internal::Parameter p, ParamBlock block,
1010
param_block_parameter(block, i, p)
1111
}
1212

13-
private predicate hasParameterBlockExcludingPipelineImpl(
13+
private predicate hasParameterBlockExcludingPipelinesImpl(
1414
Internal::Parameter p, ParamBlock block, int i
1515
) {
1616
p =
1717
rank[i + 1](Internal::Parameter cand, int j |
1818
hasParameterBlockImpl(cand, block, j) and
1919
not cand.getAnAttribute().(Attribute).getANamedArgument() instanceof
20-
ValueFromPipelineAttribute
20+
ValueFromPipelineAttribute and
21+
not cand.getAnAttribute().(Attribute).getANamedArgument() instanceof
22+
ValueFromPipelineByPropertyName
2123
|
2224
cand order by j
2325
)
@@ -69,7 +71,7 @@ private class ParameterImpl extends TParameterImpl {
6971

7072
predicate hasParameterBlock(ParamBlock block, int i) { none() }
7173

72-
predicate hasParameterBlockExcludingPipeline(ParamBlock block, int i) { none() }
74+
predicate hasParameterBlockExcludingPipelines(ParamBlock block, int i) { none() }
7375

7476
predicate isFunctionParameter(Function f, int i) { none() }
7577

@@ -84,6 +86,8 @@ private class ParameterImpl extends TParameterImpl {
8486
}
8587

8688
abstract predicate isPipeline();
89+
90+
abstract predicate isPipelineByPropertyName();
8791
}
8892

8993
private class InternalParameter extends ParameterImpl, TInternalParameter {
@@ -101,8 +105,8 @@ private class InternalParameter extends ParameterImpl, TInternalParameter {
101105
hasParameterBlockImpl(p, block, i)
102106
}
103107

104-
override predicate hasParameterBlockExcludingPipeline(ParamBlock block, int i) {
105-
hasParameterBlockExcludingPipelineImpl(p, block, i)
108+
override predicate hasParameterBlockExcludingPipelines(ParamBlock block, int i) {
109+
hasParameterBlockExcludingPipelinesImpl(p, block, i)
106110
}
107111

108112
override predicate isFunctionParameter(Function f, int i) { isFunctionParameterImpl(p, f, i) }
@@ -114,6 +118,10 @@ private class InternalParameter extends ParameterImpl, TInternalParameter {
114118
override predicate isPipeline() {
115119
this.getAnAttribute().getANamedArgument() instanceof ValueFromPipelineAttribute
116120
}
121+
122+
override predicate isPipelineByPropertyName() {
123+
this.getAnAttribute().getANamedArgument() instanceof ValueFromPipelineByPropertyName
124+
}
117125
}
118126

119127
/**
@@ -147,6 +155,8 @@ private class Underscore extends ParameterImpl, TUnderscore {
147155

148156
final override predicate isPipeline() { any() }
149157

158+
final override predicate isPipelineByPropertyName() { none() }
159+
150160
final override predicate isFunctionParameter(Function f, int i) { f.getBody() = scope and i = -1 }
151161
}
152162

@@ -164,6 +174,8 @@ private class ThisParameter extends ParameterImpl, TThisParameter {
164174
final override Attribute getAnAttribute() { none() }
165175

166176
final override predicate isPipeline() { none() }
177+
178+
final override predicate isPipelineByPropertyName() { none() }
167179
}
168180

169181
private newtype TVariable =
@@ -241,8 +253,8 @@ class Parameter extends AbstractLocalScopeVariable, TParameter {
241253

242254
predicate hasParameterBlock(ParamBlock block, int i) { p.hasParameterBlock(block, i) }
243255

244-
predicate hasParameterBlockExcludingPipeline(ParamBlock block, int i) {
245-
p.hasParameterBlockExcludingPipeline(block, i)
256+
predicate hasParameterBlockExcludingPipelines(ParamBlock block, int i) {
257+
p.hasParameterBlockExcludingPipelines(block, i)
246258
}
247259

248260
predicate isFunctionParameter(Function f, int i) { p.isFunctionParameter(f, i) }
@@ -261,14 +273,14 @@ class Parameter extends AbstractLocalScopeVariable, TParameter {
261273
*/
262274
int getIndex() { result = this.getFunctionIndex() or result = this.getBlockIndex() }
263275

264-
int getIndexExcludingPipeline() {
265-
result = this.getFunctionIndex() or result = this.getBlockIndexExcludingPipeline()
276+
int getIndexExcludingPipelines() {
277+
result = this.getFunctionIndex() or result = this.getBlockIndexExcludingPipelines()
266278
}
267279

268280
/** Gets the index of this parameter in the parameter block, if any. */
269281
int getBlockIndex() { this.hasParameterBlock(_, result) }
270282

271-
int getBlockIndexExcludingPipeline() { this.hasParameterBlockExcludingPipeline(_, result) }
283+
int getBlockIndexExcludingPipelines() { this.hasParameterBlockExcludingPipelines(_, result) }
272284

273285
/** Gets the index of this parameter in the function, if any. */
274286
int getFunctionIndex() { this.isFunctionParameter(_, result) }
@@ -278,6 +290,8 @@ class Parameter extends AbstractLocalScopeVariable, TParameter {
278290
Attribute getAnAttribute() { result = p.getAnAttribute() }
279291

280292
predicate isPipeline() { p.isPipeline() }
293+
294+
predicate isPipelineByPropertyName() { p.isPipelineByPropertyName() }
281295
}
282296

283297
class PipelineParameter extends Parameter {

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

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,9 @@ module SsaFlow {
8484
}
8585

8686
predicate localFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo, boolean isUseStep) {
87-
Impl::localFlowStep(def, asNode(nodeFrom), asNode(nodeTo), isUseStep)
87+
Impl::localFlowStep(def, asNode(nodeFrom), asNode(nodeTo), isUseStep) and
88+
// Flow out of property name parameter nodes are covered by `readStep`.
89+
not nodeFrom instanceof PipelineByPropertyNameParameter
8890
}
8991

9092
predicate localMustFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo) {
@@ -471,7 +473,7 @@ private module ParameterNodes {
471473
// keywords in S are specified.
472474
exists(int i, int j, string name, NamedSet ns, Function f |
473475
pos.isPositional(j, ns) and
474-
parameter.getIndexExcludingPipeline() = i and
476+
parameter.getIndexExcludingPipelines() = i and
475477
f = parameter.getFunction() and
476478
f = ns.getAFunction() and
477479
name = parameter.getName() and
@@ -480,12 +482,12 @@ private module ParameterNodes {
480482
i -
481483
count(int k, Parameter p |
482484
k < i and
483-
p = f.getParameterExcludingPipline(k) and
485+
p = f.getParameterExcludingPiplines(k) and
484486
p.getName() = ns.getAName()
485487
)
486488
)
487489
or
488-
parameter.isPipeline() and
490+
(parameter.isPipeline() or parameter.isPipelineByPropertyName()) and
489491
pos.isPipeline()
490492
)
491493
}
@@ -498,6 +500,12 @@ private module ParameterNodes {
498500

499501
override string toStringImpl() { result = parameter.toString() }
500502
}
503+
504+
class PipelineByPropertyNameParameter extends NormalParameterNode {
505+
PipelineByPropertyNameParameter() { this.getParameter().isPipelineByPropertyName() }
506+
507+
string getPropretyName() { result = this.getParameter().getName() }
508+
}
501509
}
502510

503511
import ParameterNodes
@@ -741,14 +749,21 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
741749
exists(CfgNode cfgNode |
742750
node1 = TPreReturnNodeImpl(cfgNode, true) and
743751
node2 = TImplicitWrapNode(cfgNode, true) and
744-
c.isSingleton(any(Content::KnownElementContent ec))
752+
c.isSingleton(any(Content::KnownElementContent ec | exists(ec.getIndex().asInt())))
745753
)
746754
or
747755
c.isAnyElement() and
748756
exists(SsaImpl::DefinitionExt def |
749757
node1.(ProcessNode).getIteratorVariable() = def.getSourceVariable() and
750758
SsaImpl::firstRead(def, node2.asExpr())
751759
)
760+
or
761+
exists(Content::KnownElementContent ec, SsaImpl::DefinitionExt def |
762+
c.isSingleton(ec) and
763+
node1.(PipelineByPropertyNameParameter).getPropretyName() = ec.getIndex().asString() and
764+
def.getSourceVariable() = node1.(PipelineByPropertyNameParameter).getParameter() and
765+
SsaImpl::firstRead(def, node2.asExpr())
766+
)
752767
}
753768

754769
/**
@@ -770,13 +785,18 @@ predicate clearsContent(Node n, ContentSet c) {
770785
*/
771786
predicate expectsContent(Node n, ContentSet c) {
772787
n = TPreReturnNodeImpl(_, true) and
773-
c.isKnownOrUnknownElement(_)
788+
c.isKnownOrUnknownElement(any(Content::KnownElementContent ec | exists(ec.getIndex().asInt())))
774789
or
775790
n = TImplicitWrapNode(_, false) and
776791
c.isSingleton(any(Content::UnknownElementContent ec))
777792
or
778793
n instanceof ProcessNode and
779794
c.isAnyElement()
795+
or
796+
exists(Content::KnownElementContent ec |
797+
ec.getIndex().asString() = n.(PipelineByPropertyNameParameter).getPropretyName() and
798+
c.isSingleton(ec)
799+
)
780800
}
781801

782802
class DataFlowType extends TDataFlowType {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ import Cached
340340
* Only intended for internal use.
341341
*/
342342
class DefinitionExt extends Impl::DefinitionExt {
343-
VarReadAccessCfgNode getARead() { result = getARead(this) }
343+
AstCfgNode getARead() { result = getARead(this) }
344344

345345
override string toString() { result = this.(Ssa::Definition).toString() }
346346

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,11 @@ edges
3030
| test.ps1:31:1:31:7 | ...,... [element 1] | test.ps1:25:14:25:16 | _ [element 1] | provenance | |
3131
| test.ps1:31:1:31:7 | ...,... [element 1] | test.ps1:31:1:31:7 | ...,... [element 1] | provenance | |
3232
| test.ps1:31:5:31:7 | y | test.ps1:31:1:31:7 | ...,... [element 1] | provenance | |
33+
| test.ps1:34:11:34:58 | x [element x] | test.ps1:36:10:36:12 | x | provenance | |
34+
| test.ps1:39:6:39:17 | Source | test.ps1:40:23:40:25 | x | provenance | |
35+
| test.ps1:40:1:40:26 | [...]... [element x] | test.ps1:34:11:34:58 | x [element x] | provenance | |
36+
| test.ps1:40:17:40:26 | ${...} [element x] | test.ps1:40:1:40:26 | [...]... [element x] | provenance | |
37+
| test.ps1:40:23:40:25 | x | test.ps1:40:17:40:26 | ${...} [element x] | provenance | |
3338
nodes
3439
| test.ps1:2:10:2:21 | Source | semmle.label | Source |
3540
| test.ps1:3:10:3:21 | Source | semmle.label | Source |
@@ -63,6 +68,12 @@ nodes
6368
| test.ps1:31:1:31:7 | ...,... [element 1] | semmle.label | ...,... [element 1] |
6469
| test.ps1:31:1:31:7 | ...,... [element 1] | semmle.label | ...,... [element 1] |
6570
| test.ps1:31:5:31:7 | y | semmle.label | y |
71+
| test.ps1:34:11:34:58 | x [element x] | semmle.label | x [element x] |
72+
| test.ps1:36:10:36:12 | x | semmle.label | x |
73+
| test.ps1:39:6:39:17 | Source | semmle.label | Source |
74+
| test.ps1:40:1:40:26 | [...]... [element x] | semmle.label | [...]... [element x] |
75+
| test.ps1:40:17:40:26 | ${...} [element x] | semmle.label | ${...} [element x] |
76+
| test.ps1:40:23:40:25 | x | semmle.label | x |
6677
subpaths
6778
testFailures
6879
#select
@@ -73,3 +84,4 @@ testFailures
7384
| test.ps1:13:14:13:16 | x | test.ps1:20:6:20:17 | Source | test.ps1:13:14:13:16 | x | $@ | test.ps1:20:6:20:17 | Source | Source |
7485
| test.ps1:25:14:25:16 | _ | test.ps1:29:6:29:17 | Source | test.ps1:25:14:25:16 | _ | $@ | test.ps1:29:6:29:17 | Source | Source |
7586
| test.ps1:25:14:25:16 | _ | test.ps1:30:6:30:17 | Source | test.ps1:25:14:25:16 | _ | $@ | test.ps1:30:6:30:17 | Source | Source |
87+
| test.ps1:36:10:36:12 | x | test.ps1:39:6:39:17 | Source | test.ps1:36:10:36:12 | x | $@ | test.ps1:39:6:39:17 | Source | Source |

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,13 @@ function consume2 {
2828

2929
$x = Source "21"
3030
$y = Source "22"
31-
$x, $y | consume2
31+
$x, $y | consume2
32+
33+
function consumeValueFromPipelineByPropertyName {
34+
Param([Parameter(ValueFromPipelineByPropertyName)] $x)
35+
36+
Sink $x # $ hasValueFlow=23
37+
}
38+
39+
$x = Source "23"
40+
[pscustomobject]@{x = $x} | consumeValueFromPipelineByPropertyName

0 commit comments

Comments
 (0)