Skip to content

Commit 1e27ddf

Browse files
hvitvedhvitved
committed
Ruby: Data flow for keyword arguments/parameters
1 parent c70a2be commit 1e27ddf

File tree

11 files changed

+353
-108
lines changed

11 files changed

+353
-108
lines changed

‎ruby/ql/lib/codeql/ruby/dataflow/FlowSummary.qll

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,10 @@ module SummaryComponent {
2323
predicate content = SC::content/1;
2424

2525
/** Gets a summary component that represents a qualifier. */
26-
SummaryComponent qualifier() { result = argument(-1) }
26+
SummaryComponent qualifier() { result = argument(any(ParameterPosition pos | pos.isSelf())) }
2727

2828
/** Gets a summary component that represents a block argument. */
29-
SummaryComponent block() { result = argument(-2) }
29+
SummaryComponent block() { result = argument(any(ParameterPosition pos | pos.isBlock())) }
3030

3131
/** Gets a summary component that represents the return value of a call. */
3232
SummaryComponent return() { result = SC::return(any(NormalReturnKind rk)) }
@@ -102,10 +102,10 @@ abstract class SummarizedCallable extends LibraryCallable {
102102

103103
/**
104104
* Holds if values stored inside `content` are cleared on objects passed as
105-
* the `i`th argument to this callable.
105+
* arguments at position `pos` to this callable.
106106
*/
107107
pragma[nomagic]
108-
predicate clearsContent(int i, DataFlow::Content content) { none() }
108+
predicate clearsContent(ParameterPosition pos, DataFlow::Content content) { none() }
109109
}
110110

111111
private class SummarizedCallableAdapter extends Impl::Public::SummarizedCallable {
@@ -119,8 +119,8 @@ private class SummarizedCallableAdapter extends Impl::Public::SummarizedCallable
119119
sc.propagatesFlow(input, output, preservesValue)
120120
}
121121

122-
final override predicate clearsContent(ParameterPosition i, DataFlow::Content content) {
123-
sc.clearsContent(i, content)
122+
final override predicate clearsContent(ParameterPosition pos, DataFlow::Content content) {
123+
sc.clearsContent(pos, content)
124124
}
125125
}
126126

‎ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll

Lines changed: 81 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ private import DataFlowPrivate
44
private import codeql.ruby.typetracking.TypeTracker
55
private import codeql.ruby.ast.internal.Module
66
private import FlowSummaryImpl as FlowSummaryImpl
7+
private import FlowSummaryImplSpecific as FlowSummaryImplSpecific
78
private import codeql.ruby.dataflow.FlowSummary
89

910
newtype TReturnKind =
@@ -230,6 +231,30 @@ private module Cached {
230231
result = yieldCall(call)
231232
)
232233
}
234+
235+
cached
236+
newtype TArgumentPosition =
237+
TSelfArgumentPosition() or
238+
TBlockArgumentPosition() or
239+
TPositionalArgumentPosition(int pos) {
240+
exists(Call c | exists(c.getArgument(pos)))
241+
or
242+
FlowSummaryImplSpecific::ParsePositions::isParsedParameterPosition(_, pos)
243+
} or
244+
TKeywordArgumentPosition(string name) { name = any(KeywordParameter kp).getName() }
245+
246+
cached
247+
newtype TParameterPosition =
248+
TSelfParameterPosition() or
249+
TBlockParameterPosition() or
250+
TPositionalParameterPosition(int pos) {
251+
pos = any(Parameter p).getPosition()
252+
or
253+
exists(Call c | exists(c.getArgument(pos))) // TODO: remove once `Argument[_]` summaries are replaced with `Argument[i..]`
254+
or
255+
FlowSummaryImplSpecific::ParsePositions::isParsedArgumentPosition(_, pos)
256+
} or
257+
TKeywordParameterPosition(string name) { name = any(KeywordParameter kp).getName() }
233258
}
234259

235260
import Cached
@@ -458,18 +483,66 @@ predicate exprNodeReturnedFrom(DataFlow::ExprNode e, Callable c) {
458483
)
459484
}
460485

461-
private int parameterPosition() { result in [-2 .. max([any(Parameter p).getPosition(), 10])] }
486+
/** A parameter position. */
487+
class ParameterPosition extends TParameterPosition {
488+
/** Holds if this position represents a `self` parameter. */
489+
predicate isSelf() { this = TSelfParameterPosition() }
462490

463-
/** A parameter position represented by an integer. */
464-
class ParameterPosition extends int {
465-
ParameterPosition() { this = parameterPosition() }
491+
/** Holds if this position represents a block parameter. */
492+
predicate isBlock() { this = TBlockParameterPosition() }
493+
494+
/** Holds if this position represents a positional parameter at position `pos`. */
495+
predicate isPositional(int pos) { this = TPositionalParameterPosition(pos) }
496+
497+
/** Holds if this position represents a keyword parameter named `name`. */
498+
predicate isKeyword(string name) { this = TKeywordParameterPosition(name) }
499+
500+
/** Gets a textual representation of this position. */
501+
string toString() {
502+
this.isSelf() and result = "self"
503+
or
504+
this.isBlock() and result = "block"
505+
or
506+
exists(int pos | this.isPositional(pos) and result = "position " + pos)
507+
or
508+
exists(string name | this.isKeyword(name) and result = "keyword " + name)
509+
}
466510
}
467511

468-
/** An argument position represented by an integer. */
469-
class ArgumentPosition extends int {
470-
ArgumentPosition() { this = parameterPosition() }
512+
/** An argument position. */
513+
class ArgumentPosition extends TArgumentPosition {
514+
/** Holds if this position represents a `self` argument. */
515+
predicate isSelf() { this = TSelfArgumentPosition() }
516+
517+
/** Holds if this position represents a block argument. */
518+
predicate isBlock() { this = TBlockArgumentPosition() }
519+
520+
/** Holds if this position represents a positional argument at position `pos`. */
521+
predicate isPositional(int pos) { this = TPositionalArgumentPosition(pos) }
522+
523+
/** Holds if this position represents a keyword argument named `name`. */
524+
predicate isKeyword(string name) { this = TKeywordArgumentPosition(name) }
525+
526+
/** Gets a textual representation of this position. */
527+
string toString() {
528+
this.isSelf() and result = "self"
529+
or
530+
this.isBlock() and result = "block"
531+
or
532+
exists(int pos | this.isPositional(pos) and result = "position " + pos)
533+
or
534+
exists(string name | this.isKeyword(name) and result = "keyword " + name)
535+
}
471536
}
472537

473538
/** Holds if arguments at position `apos` match parameters at position `ppos`. */
474539
pragma[inline]
475-
predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) { ppos = apos }
540+
predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
541+
ppos.isSelf() and apos.isSelf()
542+
or
543+
ppos.isBlock() and apos.isBlock()
544+
or
545+
exists(int pos | ppos.isPositional(pos) and apos.isPositional(pos))
546+
or
547+
exists(string name | ppos.isKeyword(name) and apos.isKeyword(name))
548+
}

‎ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll

Lines changed: 72 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -152,17 +152,29 @@ module LocalFlow {
152152
/** An argument of a call (including qualifier arguments, excluding block arguments). */
153153
private class Argument extends CfgNodes::ExprCfgNode {
154154
private CfgNodes::ExprNodes::CallCfgNode call;
155-
private int arg;
155+
private ArgumentPosition arg;
156156

157157
Argument() {
158-
this = call.getArgument(arg) and
159-
not this.getExpr() instanceof BlockArgument
158+
exists(int i |
159+
this = call.getArgument(i) and
160+
not this.getExpr() instanceof BlockArgument and
161+
not exists(this.getExpr().(Pair).getKey().getValueText()) and
162+
arg.isPositional(i)
163+
)
164+
or
165+
exists(CfgNodes::ExprNodes::PairCfgNode p |
166+
p = call.getArgument(_) and
167+
this = p.getValue() and
168+
arg.isKeyword(p.getKey().getValueText())
169+
)
160170
or
161-
this = call.getReceiver() and arg = -1
171+
this = call.getReceiver() and arg.isSelf()
162172
}
163173

164174
/** Holds if this expression is the `i`th argument of `c`. */
165-
predicate isArgumentOf(CfgNodes::ExprNodes::CallCfgNode c, int i) { c = call and i = arg }
175+
predicate isArgumentOf(CfgNodes::ExprNodes::CallCfgNode c, ArgumentPosition pos) {
176+
c = call and pos = arg
177+
}
166178
}
167179

168180
/** A collection of cached types and predicates to be evaluated in the same stage. */
@@ -179,7 +191,11 @@ private module Cached {
179191
)
180192
} or
181193
TSsaDefinitionNode(Ssa::Definition def) or
182-
TNormalParameterNode(Parameter p) { not p instanceof BlockParameter } or
194+
TNormalParameterNode(Parameter p) {
195+
p instanceof SimpleParameter or
196+
p instanceof OptionalParameter or
197+
p instanceof KeywordParameter
198+
} or
183199
TSelfParameterNode(MethodBase m) or
184200
TBlockParameterNode(MethodBase m) or
185201
TExprPostUpdateNode(CfgNodes::ExprCfgNode n) { n instanceof Argument } or
@@ -189,8 +205,8 @@ private module Cached {
189205
) {
190206
FlowSummaryImpl::Private::summaryNodeRange(c, state)
191207
} or
192-
TSummaryParameterNode(FlowSummaryImpl::Public::SummarizedCallable c, int i) {
193-
FlowSummaryImpl::Private::summaryParameterNodeRange(c, i)
208+
TSummaryParameterNode(FlowSummaryImpl::Public::SummarizedCallable c, ParameterPosition pos) {
209+
FlowSummaryImpl::Private::summaryParameterNodeRange(c, pos)
194210
}
195211

196212
class TParameterNode =
@@ -331,10 +347,10 @@ private module ParameterNodes {
331347
abstract class ParameterNodeImpl extends NodeImpl {
332348
abstract Parameter getParameter();
333349

334-
abstract predicate isSourceParameterOf(Callable c, int i);
350+
abstract predicate isSourceParameterOf(Callable c, ParameterPosition pos);
335351

336-
predicate isParameterOf(DataFlowCallable c, int i) {
337-
this.isSourceParameterOf(c.asCallable(), i)
352+
predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
353+
this.isSourceParameterOf(c.asCallable(), pos)
338354
}
339355
}
340356

@@ -349,10 +365,22 @@ private module ParameterNodes {
349365

350366
override Parameter getParameter() { result = parameter }
351367

352-
override predicate isSourceParameterOf(Callable c, int i) { c.getParameter(i) = parameter }
368+
override predicate isSourceParameterOf(Callable c, ParameterPosition pos) {
369+
exists(int i | pos.isPositional(i) and c.getParameter(i) = parameter |
370+
parameter instanceof SimpleParameter
371+
or
372+
parameter instanceof OptionalParameter
373+
)
374+
or
375+
parameter =
376+
any(KeywordParameter kp |
377+
c.getAParameter() = kp and
378+
pos.isKeyword(kp.getName())
379+
)
380+
}
353381

354-
override predicate isParameterOf(DataFlowCallable c, int i) {
355-
this.isSourceParameterOf(c.asCallable(), i)
382+
override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
383+
this.isSourceParameterOf(c.asCallable(), pos)
356384
}
357385

358386
override CfgScope getCfgScope() { result = parameter.getCallable() }
@@ -375,10 +403,12 @@ private module ParameterNodes {
375403

376404
override Parameter getParameter() { none() }
377405

378-
override predicate isSourceParameterOf(Callable c, int i) { method = c and i = -1 }
406+
override predicate isSourceParameterOf(Callable c, ParameterPosition pos) {
407+
method = c and pos.isSelf()
408+
}
379409

380-
override predicate isParameterOf(DataFlowCallable c, int i) {
381-
this.isSourceParameterOf(c.asCallable(), i)
410+
override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
411+
this.isSourceParameterOf(c.asCallable(), pos)
382412
}
383413

384414
override CfgScope getCfgScope() { result = method }
@@ -403,10 +433,12 @@ private module ParameterNodes {
403433
result = method.getAParameter() and result instanceof BlockParameter
404434
}
405435

406-
override predicate isSourceParameterOf(Callable c, int i) { c = method and i = -2 }
436+
override predicate isSourceParameterOf(Callable c, ParameterPosition pos) {
437+
c = method and pos.isBlock()
438+
}
407439

408-
override predicate isParameterOf(DataFlowCallable c, int i) {
409-
this.isSourceParameterOf(c.asCallable(), i)
440+
override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
441+
this.isSourceParameterOf(c.asCallable(), pos)
410442
}
411443

412444
override CfgScope getCfgScope() { result = method }
@@ -427,23 +459,25 @@ private module ParameterNodes {
427459
/** A parameter for a library callable with a flow summary. */
428460
class SummaryParameterNode extends ParameterNodeImpl, TSummaryParameterNode {
429461
private FlowSummaryImpl::Public::SummarizedCallable sc;
430-
private int pos;
462+
private ParameterPosition pos_;
431463

432-
SummaryParameterNode() { this = TSummaryParameterNode(sc, pos) }
464+
SummaryParameterNode() { this = TSummaryParameterNode(sc, pos_) }
433465

434466
override Parameter getParameter() { none() }
435467

436-
override predicate isSourceParameterOf(Callable c, int i) { none() }
468+
override predicate isSourceParameterOf(Callable c, ParameterPosition pos) { none() }
437469

438-
override predicate isParameterOf(DataFlowCallable c, int i) { sc = c and i = pos }
470+
override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
471+
sc = c and pos = pos_
472+
}
439473

440474
override CfgScope getCfgScope() { none() }
441475

442476
override DataFlowCallable getEnclosingCallable() { result = sc }
443477

444478
override EmptyLocation getLocationImpl() { any() }
445479

446-
override string toStringImpl() { result = "parameter " + pos + " of " + sc }
480+
override string toStringImpl() { result = "parameter " + pos_ + " of " + sc }
447481
}
448482
}
449483

@@ -468,9 +502,9 @@ class SummaryNode extends NodeImpl, TSummaryNode {
468502
/** A data-flow node that represents a call argument. */
469503
abstract class ArgumentNode extends Node {
470504
/** Holds if this argument occurs at the given position in the given call. */
471-
abstract predicate argumentOf(DataFlowCall call, int pos);
505+
abstract predicate argumentOf(DataFlowCall call, ArgumentPosition pos);
472506

473-
abstract predicate sourceArgumentOf(CfgNodes::ExprNodes::CallCfgNode call, int pos);
507+
abstract predicate sourceArgumentOf(CfgNodes::ExprNodes::CallCfgNode call, ArgumentPosition pos);
474508

475509
/** Gets the call in which this node is an argument. */
476510
final DataFlowCall getCall() { this.argumentOf(result, _) }
@@ -483,18 +517,18 @@ private module ArgumentNodes {
483517

484518
ExplicitArgumentNode() { this.asExpr() = arg }
485519

486-
override predicate argumentOf(DataFlowCall call, int pos) {
520+
override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
487521
this.sourceArgumentOf(call.asCall(), pos)
488522
}
489523

490-
override predicate sourceArgumentOf(CfgNodes::ExprNodes::CallCfgNode call, int pos) {
524+
override predicate sourceArgumentOf(CfgNodes::ExprNodes::CallCfgNode call, ArgumentPosition pos) {
491525
arg.isArgumentOf(call, pos)
492526
}
493527
}
494528

495529
/** A data-flow node that represents the `self` argument of a call. */
496530
class SelfArgumentNode extends ExplicitArgumentNode {
497-
SelfArgumentNode() { arg.isArgumentOf(_, -1) }
531+
SelfArgumentNode() { arg.isArgumentOf(_, any(ArgumentPosition pos | pos.isSelf())) }
498532
}
499533

500534
/** A data-flow node that represents a block argument. */
@@ -504,12 +538,12 @@ private module ArgumentNodes {
504538
exists(CfgNodes::ExprNodes::CallCfgNode c | c.getBlock() = this.asExpr())
505539
}
506540

507-
override predicate argumentOf(DataFlowCall call, int pos) {
541+
override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
508542
this.sourceArgumentOf(call.asCall(), pos)
509543
}
510544

511-
override predicate sourceArgumentOf(CfgNodes::ExprNodes::CallCfgNode call, int pos) {
512-
pos = -2 and
545+
override predicate sourceArgumentOf(CfgNodes::ExprNodes::CallCfgNode call, ArgumentPosition pos) {
546+
pos.isBlock() and
513547
(
514548
this.asExpr() = call.getBlock()
515549
or
@@ -525,9 +559,11 @@ private module ArgumentNodes {
525559
private class SummaryArgumentNode extends SummaryNode, ArgumentNode {
526560
SummaryArgumentNode() { FlowSummaryImpl::Private::summaryArgumentNode(_, this, _) }
527561

528-
override predicate sourceArgumentOf(CfgNodes::ExprNodes::CallCfgNode call, int pos) { none() }
562+
override predicate sourceArgumentOf(CfgNodes::ExprNodes::CallCfgNode call, ArgumentPosition pos) {
563+
none()
564+
}
529565

530-
override predicate argumentOf(DataFlowCall call, int pos) {
566+
override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
531567
FlowSummaryImpl::Private::summaryArgumentNode(call, this, pos)
532568
}
533569
}
@@ -838,7 +874,7 @@ predicate lambdaCall(DataFlowCall call, LambdaCallKind kind, Node receiver) {
838874
)
839875
or
840876
receiver = call.(SummaryCall).getReceiver() and
841-
if receiver.(ParameterNodeImpl).isParameterOf(_, -2)
877+
if receiver.(ParameterNodeImpl).isParameterOf(_, any(ParameterPosition pos | pos.isBlock()))
842878
then kind = TYieldCallKind()
843879
else kind = TLambdaCallKind()
844880
}

0 commit comments

Comments
 (0)