Skip to content

Commit e7676a0

Browse files
authored
Merge pull request #15370 from hvitved/ruby/erb-flow
Ruby: Model flow through `ViewComponent` render methods
2 parents abf015b + 4cfdf8b commit e7676a0

22 files changed

+726
-300
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Flow is now tracked through Rails `render` calls, when the argument is a `ViewComponent`. In this case, data flow is tracked into the accompanying `.html.erb` file.

ruby/ql/lib/codeql/ruby/Frameworks.qll

+1
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,4 @@ private import codeql.ruby.frameworks.Yaml
3838
private import codeql.ruby.frameworks.Sequel
3939
private import codeql.ruby.frameworks.Ldap
4040
private import codeql.ruby.frameworks.Jwt
41+
private import codeql.ruby.frameworks.ViewComponent

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

+131-29
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ private import codeql.ruby.AST
22
private import codeql.ruby.CFG
33
private import DataFlowPrivate
44
private import codeql.ruby.typetracking.internal.TypeTrackingImpl
5-
private import codeql.ruby.ast.internal.Module
5+
private import codeql.ruby.ast.internal.Module as Module
66
private import FlowSummaryImpl as FlowSummaryImpl
77
private import codeql.ruby.dataflow.FlowSummary
88
private import codeql.ruby.dataflow.SSA
@@ -82,6 +82,12 @@ abstract class LibraryCallable extends string {
8282
Call getACallSimple() { none() }
8383
}
8484

85+
/** A callable defined in library code, which should be taken into account in type tracking. */
86+
abstract class LibraryCallableToIncludeInTypeTracking extends LibraryCallable {
87+
bindingset[this]
88+
LibraryCallableToIncludeInTypeTracking() { exists(this) }
89+
}
90+
8591
/**
8692
* A callable. This includes callables from source code, as well as callables
8793
* defined in library code.
@@ -184,6 +190,91 @@ class NormalCall extends DataFlowCall, TNormalCall {
184190
override Location getLocation() { result = c.getLocation() }
185191
}
186192

193+
/**
194+
* Provides modeling of flow through the `render` method of view components.
195+
*
196+
* ```rb
197+
* # view.rb
198+
* class View < ViewComponent::Base
199+
* def initialize(x)
200+
* @x = x
201+
* end
202+
*
203+
* def foo
204+
* sink(@x)
205+
* end
206+
* end
207+
* ```
208+
*
209+
* ```erb
210+
* # view.html.erb
211+
* <%= foo() %> # 1
212+
* ```
213+
*
214+
* ```rb
215+
* # app.rb
216+
* class App
217+
* def run
218+
* view = View.new(taint) # 2
219+
* render(view) # 3
220+
* end
221+
* end
222+
* ```
223+
*
224+
* The `render` call (3) is modeled using a flow summary. The summary specifies
225+
* that the first argument (`view`) will have a special method invoked on it (we
226+
* call the method `__invoke__toplevel__erb__`), which targets the top-level of the
227+
* matching ERB file (`view.html.erb`). The `view` argument will flow into the receiver
228+
* of the synthesized method call, from there into the implicit `self` parameter of
229+
* the ERB file, and from there to the implicit `self` receiver of the call to `foo` (1).
230+
*
231+
* Since it is not actually possible to specify such flow summaries, we instead
232+
* specify a call-back summary, and adjust the generated call to target the special
233+
* `__invoke__toplevel__erb__` method.
234+
*
235+
* In order to resolve the target of the adjusted method call, we need to take
236+
* the `render` summary into account when constructing the call graph. That is, we
237+
* need to track the `View` instance (2) into the receiver of the adjusted method
238+
* call, in order to figure out that the call target is in fact `view.html.erb`.
239+
*/
240+
private module ViewComponentRenderModeling {
241+
private import codeql.ruby.frameworks.ViewComponent
242+
243+
private class RenderMethod extends SummarizedCallable, LibraryCallableToIncludeInTypeTracking {
244+
RenderMethod() { this = "render view component" }
245+
246+
override MethodCall getACallSimple() { result.getMethodName() = "render" }
247+
248+
override predicate propagatesFlow(string input, string output, boolean preservesValue) {
249+
input = "Argument[0]" and
250+
// use a call-back summary, and adjust it to a method call below
251+
output = "Argument[0].Parameter[self]" and
252+
preservesValue = true
253+
}
254+
}
255+
256+
private string invokeToplevelName() { result = "__invoke__toplevel__erb__" }
257+
258+
/** Holds if `call` should be adjusted to be a method call to `name` on `receiver`. */
259+
predicate adjustedMethodCall(DataFlowCall call, FlowSummaryNode receiver, string name) {
260+
exists(RenderMethod render |
261+
call = TSummaryCall(render, receiver.getSummaryNode()) and
262+
name = invokeToplevelName()
263+
)
264+
}
265+
266+
/** Holds if `self` belongs to the top-level of an ERB file with matching view class `view`. */
267+
pragma[nomagic]
268+
predicate selfInErbToplevel(SelfVariable self, ViewComponent::ComponentClass view) {
269+
self.getDeclaringScope().(Toplevel).getFile() = view.getTemplate()
270+
}
271+
272+
Toplevel lookupMethod(ViewComponent::ComponentClass m, string name) {
273+
result.getFile() = m.getTemplate() and
274+
name = invokeToplevelName()
275+
}
276+
}
277+
187278
/** A call for which we want to compute call targets. */
188279
private class RelevantCall extends CfgNodes::ExprNodes::CallCfgNode {
189280
pragma[nomagic]
@@ -200,6 +291,8 @@ private predicate methodCall(DataFlowCall call, DataFlow::Node receiver, string
200291
method = rc.getExpr().(MethodCall).getMethodName() and
201292
receiver.asExpr() = rc.getReceiver()
202293
)
294+
or
295+
ViewComponentRenderModeling::adjustedMethodCall(call, receiver, method)
203296
}
204297

205298
pragma[nomagic]
@@ -253,8 +346,11 @@ private predicate selfInMethod(SelfVariable self, MethodBase method, Module m) {
253346
/** Holds if `self` belongs to the top-level. */
254347
pragma[nomagic]
255348
private predicate selfInToplevel(SelfVariable self, Module m) {
349+
ViewComponentRenderModeling::selfInErbToplevel(self, m)
350+
or
351+
not ViewComponentRenderModeling::selfInErbToplevel(self, _) and
256352
self.getDeclaringScope() instanceof Toplevel and
257-
m = TResolved("Object")
353+
m = Module::TResolved("Object")
258354
}
259355

260356
/**
@@ -271,7 +367,7 @@ private predicate selfInToplevel(SelfVariable self, Module m) {
271367
*/
272368
private predicate asModulePattern(SsaDefinitionExtNode def, Module m) {
273369
exists(AsPattern ap |
274-
m = resolveConstantReadAccess(ap.getPattern()) and
370+
m = Module::resolveConstantReadAccess(ap.getPattern()) and
275371
def.getDefinitionExt().(Ssa::WriteDefinition).getWriteAccess().getAstNode() =
276372
ap.getVariableAccess()
277373
)
@@ -295,7 +391,7 @@ private predicate hasAdjacentTypeCheckedReads(
295391
exists(
296392
CfgNodes::ExprCfgNode pattern, ConditionBlock cb, CfgNodes::ExprNodes::CaseExprCfgNode case
297393
|
298-
m = resolveConstantReadAccess(pattern.getExpr()) and
394+
m = Module::resolveConstantReadAccess(pattern.getExpr()) and
299395
cb.getLastNode() = pattern and
300396
cb.controls(read2.getBasicBlock(),
301397
any(SuccessorTypes::MatchingSuccessor match | match.getValue() = true)) and
@@ -313,7 +409,7 @@ predicate isUserDefinedNew(SingletonMethod new) {
313409
exists(Expr object | singletonMethod(new, "new", object) |
314410
selfInModule(object.(SelfVariableReadAccess).getVariable(), _)
315411
or
316-
exists(resolveConstantReadAccess(object))
412+
exists(Module::resolveConstantReadAccess(object))
317413
)
318414
}
319415

@@ -351,7 +447,7 @@ private predicate extendCall(DataFlow::ExprNode receiver, Module m) {
351447
extendCall.getMethodName() = "extend" and
352448
exists(DataFlow::LocalSourceNode sourceNode | sourceNode.flowsTo(extendCall.getArgument(_)) |
353449
selfInModule(sourceNode.(SelfLocalSourceNode).getVariable(), m) or
354-
m = resolveConstantReadAccess(sourceNode.asExpr().getExpr())
450+
m = Module::resolveConstantReadAccess(sourceNode.asExpr().getExpr())
355451
) and
356452
receiver = extendCall.getReceiver()
357453
)
@@ -364,10 +460,16 @@ private predicate extendCallModule(Module m, Module n) {
364460
receiver.flowsTo(e) and extendCall(e, n)
365461
|
366462
selfInModule(receiver.(SelfLocalSourceNode).getVariable(), m) or
367-
m = resolveConstantReadAccess(receiver.asExpr().getExpr())
463+
m = Module::resolveConstantReadAccess(receiver.asExpr().getExpr())
368464
)
369465
}
370466

467+
private CfgScope lookupMethod(Module m, string name) {
468+
result = Module::lookupMethod(m, name)
469+
or
470+
result = ViewComponentRenderModeling::lookupMethod(m, name)
471+
}
472+
371473
/**
372474
* Gets a method available in module `m`, or in one of `m`'s transitive
373475
* sub classes when `exact = false`.
@@ -377,7 +479,7 @@ private CfgScope lookupMethod(Module m, string name, boolean exact) {
377479
result = lookupMethod(m, name) and
378480
exact in [false, true]
379481
or
380-
result = lookupMethodInSubClasses(m, name) and
482+
result = Module::lookupMethodInSubClasses(m, name) and
381483
exact = false
382484
}
383485

@@ -490,7 +592,7 @@ private module TrackModuleInput implements CallGraphConstruction::Simple::InputS
490592
class State = Module;
491593

492594
predicate start(DataFlow::Node start, Module m) {
493-
m = resolveConstantReadAccess(start.asExpr().getExpr())
595+
m = Module::resolveConstantReadAccess(start.asExpr().getExpr())
494596
}
495597

496598
// We exclude steps into `self` parameters, and instead rely on the type of the
@@ -547,55 +649,55 @@ private module TrackInstanceInput implements CallGraphConstruction::InputSig {
547649
pragma[nomagic]
548650
private predicate isInstanceNoCall(DataFlow::Node n, Module tp, boolean exact) {
549651
n.asExpr().getExpr() instanceof NilLiteral and
550-
tp = TResolved("NilClass") and
652+
tp = Module::TResolved("NilClass") and
551653
exact = true
552654
or
553655
n.asExpr().getExpr().(BooleanLiteral).isFalse() and
554-
tp = TResolved("FalseClass") and
656+
tp = Module::TResolved("FalseClass") and
555657
exact = true
556658
or
557659
n.asExpr().getExpr().(BooleanLiteral).isTrue() and
558-
tp = TResolved("TrueClass") and
660+
tp = Module::TResolved("TrueClass") and
559661
exact = true
560662
or
561663
n.asExpr().getExpr() instanceof IntegerLiteral and
562-
tp = TResolved("Integer") and
664+
tp = Module::TResolved("Integer") and
563665
exact = true
564666
or
565667
n.asExpr().getExpr() instanceof FloatLiteral and
566-
tp = TResolved("Float") and
668+
tp = Module::TResolved("Float") and
567669
exact = true
568670
or
569671
n.asExpr().getExpr() instanceof RationalLiteral and
570-
tp = TResolved("Rational") and
672+
tp = Module::TResolved("Rational") and
571673
exact = true
572674
or
573675
n.asExpr().getExpr() instanceof ComplexLiteral and
574-
tp = TResolved("Complex") and
676+
tp = Module::TResolved("Complex") and
575677
exact = true
576678
or
577679
n.asExpr().getExpr() instanceof StringlikeLiteral and
578-
tp = TResolved("String") and
680+
tp = Module::TResolved("String") and
579681
exact = true
580682
or
581683
n.asExpr() instanceof CfgNodes::ExprNodes::ArrayLiteralCfgNode and
582-
tp = TResolved("Array") and
684+
tp = Module::TResolved("Array") and
583685
exact = true
584686
or
585687
n.asExpr() instanceof CfgNodes::ExprNodes::HashLiteralCfgNode and
586-
tp = TResolved("Hash") and
688+
tp = Module::TResolved("Hash") and
587689
exact = true
588690
or
589691
n.asExpr().getExpr() instanceof MethodBase and
590-
tp = TResolved("Symbol") and
692+
tp = Module::TResolved("Symbol") and
591693
exact = true
592694
or
593695
n.asParameter() instanceof BlockParameter and
594-
tp = TResolved("Proc") and
696+
tp = Module::TResolved("Proc") and
595697
exact = true
596698
or
597699
n.asExpr().getExpr() instanceof Lambda and
598-
tp = TResolved("Proc") and
700+
tp = Module::TResolved("Proc") and
599701
exact = true
600702
or
601703
// `self` reference in method or top-level (but not in module or singleton method,
@@ -646,11 +748,11 @@ private module TrackInstanceInput implements CallGraphConstruction::InputSig {
646748
isInstance(start, tp, exact)
647749
or
648750
exists(Module m |
649-
(if m.isClass() then tp = TResolved("Class") else tp = TResolved("Module")) and
751+
(if m.isClass() then tp = Module::TResolved("Class") else tp = Module::TResolved("Module")) and
650752
exact = true
651753
|
652754
// needed for e.g. `C.new`
653-
m = resolveConstantReadAccess(start.asExpr().getExpr())
755+
m = Module::resolveConstantReadAccess(start.asExpr().getExpr())
654756
or
655757
// needed for e.g. `self.include`
656758
selfInModule(start.(SelfLocalSourceNode).getVariable(), m)
@@ -805,7 +907,7 @@ private predicate singletonMethodOnModule(MethodBase method, string name, Module
805907
)
806908
or
807909
exists(DataFlow::LocalSourceNode sourceNode |
808-
m = resolveConstantReadAccess(sourceNode.asExpr().getExpr()) and
910+
m = Module::resolveConstantReadAccess(sourceNode.asExpr().getExpr()) and
809911
flowsToSingletonMethodObject(sourceNode, method, name)
810912
)
811913
or
@@ -821,7 +923,7 @@ private MethodBase lookupSingletonMethodDirect(Module m, string name) {
821923
or
822924
exists(DataFlow::LocalSourceNode sourceNode |
823925
sourceNode = trackModuleAccess(m) and
824-
not m = resolveConstantReadAccess(sourceNode.asExpr().getExpr()) and
926+
not m = Module::resolveConstantReadAccess(sourceNode.asExpr().getExpr()) and
825927
flowsToSingletonMethodObject(sourceNode, result, name)
826928
)
827929
}
@@ -847,7 +949,7 @@ private MethodBase lookupSingletonMethodInSubClasses(Module m, string name) {
847949
// The 'self' inside such a singleton method could then be any class, leading to self-calls
848950
// being resolved to arbitrary singleton methods.
849951
// To remedy this, we do not allow following super-classes all the way to Object.
850-
not m = TResolved("Object") and
952+
not m = Module::TResolved("Object") and
851953
exists(Module sub |
852954
sub.getSuperClass() = m // not `getAnImmediateAncestor` because singleton methods cannot be included
853955
|
@@ -901,7 +1003,7 @@ predicate singletonMethodOnInstance(MethodBase method, string name, Expr object)
9011003
singletonMethod(method, name, object) and
9021004
not selfInModule(object.(SelfVariableReadAccess).getVariable(), _) and
9031005
// cannot use `trackModuleAccess` because of negative recursion
904-
not exists(resolveConstantReadAccess(object))
1006+
not exists(Module::resolveConstantReadAccess(object))
9051007
or
9061008
exists(DataFlow::ExprNode receiver, Module other |
9071009
extendCall(receiver, other) and
@@ -948,7 +1050,7 @@ private module TrackSingletonMethodOnInstanceInput implements CallGraphConstruct
9481050
RelevantCall call, DataFlow::Node arg, DataFlow::ParameterNode p,
9491051
CfgNodes::ExprCfgNode nodeFromPreExpr
9501052
|
951-
callStep(call, arg, p) and
1053+
sourceCallStep(call, arg, p) and
9521054
nodeTo.getPreUpdateNode() = arg and
9531055
summary.toString() = "return" and
9541056
(

0 commit comments

Comments
 (0)