Skip to content

Commit abce224

Browse files
hvitvedhvitved
committed
temp
1 parent d85ed38 commit abce224

File tree

7 files changed

+121
-69
lines changed

7 files changed

+121
-69
lines changed

‎ruby/ql/lib/codeql/ruby/controlflow/CfgNodes.qll

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -813,20 +813,59 @@ module ExprNodes {
813813
override SelfVariableAccess getExpr() { result = VariableAccessCfgNode.super.getExpr() }
814814
}
815815

816+
private class VariableWriteAccessChildMapping extends ExprChildMapping, VariableWriteAccess {
817+
override predicate relevantChild(AstNode n) { this.isExplicitWrite(n) }
818+
}
819+
816820
/** A control-flow node that wraps a `VariableWriteAccess` AST expression. */
817821
class VariableWriteAccessCfgNode extends VariableAccessCfgNode {
822+
/**
823+
* Holds if this access is a write access belonging to the explicit
824+
* assignment `assignment`. For example, in
825+
*
826+
* ```rb
827+
* a = foo
828+
* ```
829+
*
830+
* both `a` is write accesses belonging to the assignment `a = foo`.
831+
*/
832+
predicate isExplicitWrite(CfgNode assignment) {
833+
e.hasCfgChild(any(AstNode n | e.isExplicitWrite(n)), this, assignment)
834+
}
835+
836+
/**
837+
* Holds if this access is a write access belonging to an implicit assignment.
838+
* For example, in
839+
*
840+
* ```rb
841+
* def m elements
842+
* for e in elements do
843+
* puts e
844+
* end
845+
* end
846+
* ```
847+
*
848+
* the access to `elements` in the parameter list is an implicit assignment,
849+
* as is the first access to `e`.
850+
*/
851+
predicate isImplicitWrite() { e.isImplicitWrite() }
852+
818853
override string getAPrimaryQlClass() { result = "VariableWriteAccessCfgNode" }
819854

820-
override VariableWriteAccess e;
855+
override VariableWriteAccessChildMapping e;
821856

822857
override VariableWriteAccess getExpr() { result = VariableAccessCfgNode.super.getExpr() }
823858
}
824859

860+
private class LocalVariableWriteAccessChildMapping extends VariableWriteAccessChildMapping,
861+
LocalVariableWriteAccess
862+
{ }
863+
825864
/** A control-flow node that wraps a `LocalVariableWriteAccess` AST expression. */
826865
class LocalVariableWriteAccessCfgNode extends VariableWriteAccessCfgNode {
827866
override string getAPrimaryQlClass() { result = "LocalVariableWriteAccessCfgNode" }
828867

829-
override LocalVariableWriteAccess e;
868+
override LocalVariableWriteAccessChildMapping e;
830869

831870
final override LocalVariableWriteAccess getExpr() { result = super.getExpr() }
832871

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ module Ssa {
192192
* ```
193193
*/
194194
class WriteDefinition extends Definition, SsaImpl::WriteDefinition {
195-
private VariableWriteAccess write;
195+
private VariableWriteAccessCfgNode write;
196196

197197
WriteDefinition() {
198198
exists(BasicBlock bb, int i, Variable v |
@@ -202,7 +202,7 @@ module Ssa {
202202
}
203203

204204
/** Gets the underlying write access. */
205-
final VariableWriteAccess getWriteAccess() { result = write }
205+
final VariableWriteAccessCfgNode getWriteAccess() { result = write }
206206

207207
/**
208208
* Holds if this SSA definition represents a direct assignment of `value`

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

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -83,15 +83,33 @@ class CapturedVariable extends LocalVariable {
8383
CapturedVariable() { this.isCaptured() }
8484
}
8585

86+
/** A variable scope that declares one or more captured variables. */
87+
class CapturedScope extends Scope {
88+
CapturedScope() { this = any(CapturedVariable v).getDeclaringScope() }
89+
90+
/** Gets a captured variable declared in this scope. */
91+
CapturedVariable getACapturedVariable() { result.getDeclaringScope() = this }
92+
93+
/** Gets a CFG scope that is contained within this variable scope. */
94+
Cfg::CfgScope getACfgScope() { result.(Scope).getOuterScope*() = this }
95+
}
96+
97+
private predicate entryDef(Cfg::EntryBasicBlock bb) {
98+
bb.getScope() = any(CapturedScope s).getACfgScope()
99+
}
100+
86101
cached
87102
private newtype TCaptureAccess =
88-
TEntryDef(Cfg::EntryBasicBlock bb) or
103+
TEntryDef(Cfg::EntryBasicBlock bb) { entryDef(bb) and not bb.getScope() instanceof MethodBase } or
89104
TVariableAccess(VariableAccessCfgNode access) { access.getVariable() instanceof CapturedVariable } or
90-
TLambdaAccess(Callable c) or
91-
TCallAccess(Cfg::BasicBlock bb, int i) {
92-
bb.getNode(i) instanceof DataFlowPrivate::Argument
93-
or
94-
DataFlowPrivate::isBlockArgument(_, bb.getNode(i))
105+
TLambdaAccess(Callable c) { c.getCfgScope() = any(CapturedScope s).getACfgScope() } or
106+
TArgumentAccess(Cfg::BasicBlock bb, int i) {
107+
bb.getScope() = any(CapturedScope s).getACfgScope() and
108+
(
109+
bb.getNode(i) instanceof DataFlowPrivate::Argument
110+
or
111+
DataFlowPrivate::isBlockArgument(_, bb.getNode(i))
112+
)
95113
}
96114

97115
class CaptureAccess extends TCaptureAccess {
@@ -113,7 +131,7 @@ class CaptureAccess extends TCaptureAccess {
113131
)
114132
or
115133
exists(Cfg::BasicBlock bb, int i |
116-
this = TCallAccess(bb, i) and
134+
this = TArgumentAccess(bb, i) and
117135
result = bb.getNode(i).getLocation()
118136
)
119137
}
@@ -134,7 +152,7 @@ class CaptureAccess extends TCaptureAccess {
134152
)
135153
or
136154
exists(Cfg::BasicBlock bb |
137-
this = TCallAccess(bb, _) and
155+
this = TArgumentAccess(bb, _) and
138156
result = bb.getScope()
139157
)
140158
}
@@ -143,14 +161,14 @@ class CaptureAccess extends TCaptureAccess {
143161
this = TLambdaAccess(bb.getNode(i).getNode())
144162
}
145163

146-
predicate isCallAccess(Cfg::BasicBlock bb, int i) { this = TCallAccess(bb, i) }
164+
predicate isArgumentAccess(Cfg::BasicBlock bb, int i) { this = TArgumentAccess(bb, i) }
147165

148166
predicate isRead(Cfg::BasicBlock bb, int i) {
149167
this = TVariableAccess(bb.getNode(i))
150168
or
151169
this.isLambdaAccess(bb, i)
152170
or
153-
this.isCallAccess(bb, i)
171+
this.isArgumentAccess(bb, i)
154172
}
155173

156174
predicate isWrite(Cfg::BasicBlock bb, int i) { this = TEntryDef(bb) and i = -1 }
@@ -159,6 +177,18 @@ class CaptureAccess extends TCaptureAccess {
159177
this.isRead(bb, i) or
160178
this.isWrite(bb, i)
161179
}
180+
181+
pragma[nomagic]
182+
predicate isWriteDef(Ssa::WriteDefinition def, CapturedVariable v) {
183+
v = def.getSourceVariable() and
184+
this = TVariableAccess(def.getWriteAccess())
185+
}
186+
187+
pragma[nomagic]
188+
predicate isReadAccess(VariableReadAccessCfgNode read, CapturedVariable v) {
189+
v = read.getVariable() and
190+
this = TVariableAccess(read)
191+
}
162192
}
163193

164194
private module SsaInput implements SsaImplCommon::InputSig {
@@ -179,7 +209,8 @@ private module SsaInput implements SsaImplCommon::InputSig {
179209
* `certain` is true if the write definitely occurs.
180210
*/
181211
predicate variableWrite(BasicBlock bb, int i, SourceVariable v, boolean certain) {
182-
any(CaptureAccess access).isWrite(bb, i) and
212+
entryDef(bb) and
213+
i = -1 and
183214
certain = true and
184215
exists(v)
185216
}

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

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,8 @@ private predicate selfInToplevel(SelfVariable self, Module m) {
250250
private predicate asModulePattern(SsaDefinitionExtNode def, Module m) {
251251
exists(AsPattern ap |
252252
m = resolveConstantReadAccess(ap.getPattern()) and
253-
def.getDefinitionExt().(Ssa::WriteDefinition).getWriteAccess() = ap.getVariableAccess()
253+
def.getDefinitionExt().(Ssa::WriteDefinition).getWriteAccess().getNode() =
254+
ap.getVariableAccess()
254255
)
255256
}
256257

@@ -423,8 +424,7 @@ private module Cached {
423424
THashSplatArgumentPosition() or
424425
TSplatAllArgumentPosition() or
425426
TAnyArgumentPosition() or
426-
TAnyKeywordArgumentPosition() or
427-
TCapturedVariableArgumentPosition()
427+
TAnyKeywordArgumentPosition()
428428

429429
cached
430430
newtype TParameterPosition =
@@ -446,8 +446,7 @@ private module Cached {
446446
THashSplatParameterPosition() or
447447
TSplatAllParameterPosition() or
448448
TAnyParameterPosition() or
449-
TAnyKeywordParameterPosition() or
450-
TCapturedVariableParameterPosition()
449+
TAnyKeywordParameterPosition()
451450
}
452451

453452
import Cached
@@ -1262,8 +1261,6 @@ class ParameterPosition extends TParameterPosition {
12621261
/** Holds if this position represents any positional parameter. */
12631262
predicate isAnyNamed() { this = TAnyKeywordParameterPosition() }
12641263

1265-
predicate isCapturedVariable() { this = TCapturedVariableParameterPosition() }
1266-
12671264
/** Gets a textual representation of this position. */
12681265
string toString() {
12691266
this.isSelf() and result = "self"
@@ -1283,8 +1280,6 @@ class ParameterPosition extends TParameterPosition {
12831280
this.isAny() and result = "any"
12841281
or
12851282
this.isAnyNamed() and result = "any-named"
1286-
or
1287-
this.isCapturedVariable() and result = "captured variable"
12881283
}
12891284
}
12901285

@@ -1311,8 +1306,6 @@ class ArgumentPosition extends TArgumentPosition {
13111306
/** Holds if this position represents any positional parameter. */
13121307
predicate isAnyNamed() { this = TAnyKeywordArgumentPosition() }
13131308

1314-
predicate isCapturedVariable() { this = TCapturedVariableArgumentPosition() }
1315-
13161309
/**
13171310
* Holds if this position represents a synthesized argument containing all keyword
13181311
* arguments wrapped in a hash.
@@ -1338,8 +1331,6 @@ class ArgumentPosition extends TArgumentPosition {
13381331
this.isHashSplat() and result = "**"
13391332
or
13401333
this.isSplatAll() and result = "*"
1341-
or
1342-
this.isCapturedVariable() and result = "captured variable"
13431334
}
13441335
}
13451336

@@ -1375,6 +1366,4 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
13751366
ppos.isAnyNamed() and apos.isKeyword(_)
13761367
or
13771368
apos.isAnyNamed() and ppos.isKeyword(_)
1378-
or
1379-
ppos.isCapturedVariable() and apos.isCapturedVariable()
13801369
}

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

Lines changed: 21 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,8 @@ module LocalFlow {
140140
* that parameter.
141141
*
142142
* This is intended to recover from flow not currently recognised by ordinary capture flow.
143+
*
144+
* TODO
143145
*/
144146
predicate localFlowSsaParamCaptureInput(Node nodeFrom, Node nodeTo) {
145147
exists(Ssa::CapturedEntryDefinition def |
@@ -324,13 +326,11 @@ private module Cached {
324326
or
325327
c.getAnArgument() instanceof CfgNodes::ExprNodes::PairCfgNode
326328
} or
327-
TCapturedVariableAcccessNode(CapturedVariableFlow::CaptureAccess access) {
328-
// not access.isCallAccess(_, _)
329-
any()
330-
} or
329+
TCapturedVariableAcccessNode(CapturedVariableFlow::CaptureAccess access) or
331330
TCapturedVariableAcccessPostUpdateNode(CapturedVariableFlow::CaptureAccess access) {
332-
// access.isCallAccess(_, _)
333-
any()
331+
access.isWriteDef(_, _)
332+
or
333+
access.isReadAccess(_, _)
334334
}
335335

336336
class TParameterNode =
@@ -389,7 +389,7 @@ private module Cached {
389389
or
390390
exists(BasicBlock bb, int i |
391391
nodeFrom.(PostUpdateNode).getPreUpdateNode().asExpr() = bb.getNode(i) and
392-
nodeTo.(CapturedVariableAcccessPostUpdateNode).getAccess().isCallAccess(bb, i)
392+
nodeTo.(CapturedVariableAcccessNode).getAccess().isArgumentAccess(bb, i)
393393
)
394394
}
395395

@@ -485,7 +485,7 @@ private module Cached {
485485
FlowSummaryImplSpecific::ParsePositions::isParsedElementLowerBoundPosition(_, includeUnknown,
486486
lower)
487487
} or
488-
TAnyCapturedVariableContent() or
488+
TAnyCapturedVariableContent(CfgScope scope) or
489489
TNoContentSet() // Only used by type-tracking
490490

491491
cached
@@ -522,9 +522,9 @@ private module Cached {
522522
name = [input, output].regexpFind("(?<=(^|\\.)Field\\[)[^\\]]+(?=\\])", _, _).trim()
523523
)
524524
} or
525+
TCapturedVariable(CapturedVariableFlow::CapturedVariable v) or
525526
// Only used by type-tracking
526-
TAttributeName(string name) { name = any(SetterMethodCall c).getTargetName() } or
527-
TCapturedVariable(CapturedVariableFlow::CapturedVariable v)
527+
TAttributeName(string name) { name = any(SetterMethodCall c).getTargetName() }
528528

529529
/**
530530
* Holds if `e` is an `ExprNode` that may be returned by a call to `c`.
@@ -889,7 +889,7 @@ private module ParameterNodes {
889889

890890
override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
891891
c.asCallable() = this.getCfgScope() and
892-
pos.isCapturedVariable()
892+
pos.isSelf()
893893
}
894894
}
895895
}
@@ -1016,12 +1016,12 @@ private module ArgumentNodes {
10161016
override string toStringImpl() { result = "**" }
10171017
}
10181018

1019-
class CaptureArgumentNode extends ArgumentNode {
1020-
CaptureArgumentNode() { lambdaCall(_, _, this) }
1019+
class LambdaSelfArgumentNode extends ArgumentNode {
1020+
LambdaSelfArgumentNode() { lambdaCall(_, _, this) }
10211021

10221022
override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
10231023
lambdaCall(call, _, this) and
1024-
pos.isCapturedVariable()
1024+
pos.isSelf()
10251025
}
10261026

10271027
override predicate sourceArgumentOf(CfgNodes::ExprNodes::CallCfgNode call, ArgumentPosition pos) {
@@ -1272,16 +1272,10 @@ pragma[nomagic]
12721272
private predicate capturedVariableStoreStep(
12731273
SsaDefinitionExtNode node1, ContentSet c, CapturedVariableAcccessPostUpdateNode node2
12741274
) {
1275-
exists(BasicBlock bb, int i, Ssa::WriteDefinition def |
1275+
exists(Ssa::WriteDefinition def, CapturedVariableFlow::CapturedVariable v |
12761276
def = node1.getDefinitionExt() and
1277-
bb.getNode(i).getNode() = def.getWriteAccess() and // TODO: splitting
1278-
c.isSingleton(any(Content::CapturedVariableContent cvc |
1279-
cvc.getVariable() = def.getSourceVariable()
1280-
))
1281-
|
1282-
node2.getAccess().isReadOrWrite(bb, i)
1283-
// or
1284-
// node2.getAccess().isWrite(bb, i)
1277+
node2.getAccess().isWriteDef(def, v) and
1278+
c.isSingleton(any(Content::CapturedVariableContent cvc | cvc.getVariable() = v))
12851279
)
12861280
}
12871281

@@ -1324,12 +1318,10 @@ private predicate capturedVariableReadStep(
13241318
CapturedVariableAcccessNode node1, ContentSet c, Node node2
13251319
) {
13261320
exists(
1327-
BasicBlock bb, int i, CfgNodes::ExprNodes::VariableReadAccessCfgNode read, LocalVariable v
1321+
CfgNodes::ExprNodes::VariableReadAccessCfgNode read, CapturedVariableFlow::CapturedVariable v
13281322
|
1329-
node1.getAccess().isRead(bb, i) and
1323+
node1.getAccess().isReadAccess(read, v) and
13301324
node2.asExpr() = read and
1331-
read = bb.getNode(i) and
1332-
v = read.getVariable() and
13331325
c.isSingleton(any(Content::CapturedVariableContent cvc | cvc.getVariable() = v))
13341326
)
13351327
}
@@ -1399,13 +1391,10 @@ predicate clearsContent(Node n, ContentSet c) {
13991391
* Holds if the value that is being tracked is expected to be stored inside content `c`
14001392
* at node `n`.
14011393
*/
1402-
predicate expectsContent(Node n, ContentSet c) {
1394+
predicate expectsContent(NodeImpl n, ContentSet c) {
14031395
FlowSummaryImpl::Private::Steps::summaryExpectsContent(n, c)
14041396
or
1405-
exists(CapturedVariableFlow::CaptureAccess access |
1406-
access.isReadOrWrite(_, _) and
1407-
c.isAnyCapturedVariable()
1408-
|
1397+
exists(CapturedVariableFlow::CaptureAccess access | c.isAnyCapturedVariable(n.getCfgScope()) |
14091398
n = TCapturedVariableAcccessNode(access) or
14101399
n = TCapturedVariableAcccessPostUpdateNode(access)
14111400
)

0 commit comments

Comments
 (0)