Skip to content

Commit 5fba5e4

Browse files
authored
Merge pull request github#11718 from hvitved/ruby/self-allocate
Ruby: Recognize custom `self.new` methods that return `self.allocate`
2 parents b2856c1 + accf4ca commit 5fba5e4

File tree

7 files changed

+720
-604
lines changed

7 files changed

+720
-604
lines changed

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -473,9 +473,12 @@ private DataFlow::LocalSourceNode trackModuleAccess(Module m) {
473473
}
474474

475475
pragma[nomagic]
476-
private predicate hasUserDefinedSelf(Module m) {
477-
// cannot use `lookupSingletonMethod` due to negative recursion
478-
singletonMethodOnModule(_, "new", m.getSuperClass*()) // not `getAnAncestor` because singleton methods cannot be included
476+
private predicate hasUserDefinedNew(Module m) {
477+
exists(DataFlow::MethodNode method |
478+
// not `getAnAncestor` because singleton methods cannot be included
479+
singletonMethodOnModule(method.asCallableAstNode(), "new", m.getSuperClass*()) and
480+
not method.getSelfParameter().getAMethodCall("allocate").flowsTo(method.getAReturningNode())
481+
)
479482
}
480483

481484
/** Holds if `n` is an instance of type `tp`. */
@@ -536,7 +539,7 @@ private predicate isInstance(DataFlow::Node n, Module tp, boolean exact) {
536539
flowsToMethodCallReceiver(call, sourceNode, "new") and
537540
n.asExpr() = call and
538541
// `tp` should not have a user-defined `self.new` method
539-
not hasUserDefinedSelf(tp)
542+
not hasUserDefinedNew(tp)
540543
|
541544
// `C.new`
542545
sourceNode = trackModuleAccess(tp) and

ruby/ql/test/library-tests/modules/ancestors.expected

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,12 @@ calls.rb:
142142
#-----| super -> Object
143143
#-----| include -> Included
144144

145+
# 633| CustomNew1
146+
#-----| super -> Object
147+
148+
# 641| CustomNew2
149+
#-----| super -> Object
150+
145151
hello.rb:
146152
# 1| EnglishWords
147153

ruby/ql/test/library-tests/modules/callgraph.expected

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,14 @@ getTarget
237237
| calls.rb:620:9:620:16 | call to bar | calls.rb:628:5:630:7 | bar |
238238
| calls.rb:627:5:627:20 | call to include | calls.rb:108:5:110:7 | include |
239239
| calls.rb:629:9:629:13 | call to super | calls.rb:622:5:623:7 | bar |
240+
| calls.rb:635:9:635:14 | call to new | calls.rb:117:5:117:16 | new |
241+
| calls.rb:639:1:639:14 | call to new | calls.rb:117:5:117:16 | new |
242+
| calls.rb:639:1:639:14 | call to new | calls.rb:634:5:636:7 | new |
243+
| calls.rb:639:1:639:23 | call to instance | calls.rb:326:5:328:7 | instance |
244+
| calls.rb:647:9:647:34 | call to puts | calls.rb:102:5:102:30 | puts |
245+
| calls.rb:651:1:651:14 | call to new | calls.rb:117:5:117:16 | new |
246+
| calls.rb:651:1:651:14 | call to new | calls.rb:642:5:644:7 | new |
247+
| calls.rb:651:1:651:23 | call to instance | calls.rb:646:5:648:7 | instance |
240248
| hello.rb:12:5:12:24 | call to include | calls.rb:108:5:110:7 | include |
241249
| hello.rb:14:16:14:20 | call to hello | hello.rb:2:5:4:7 | hello |
242250
| hello.rb:20:16:20:20 | call to super | hello.rb:13:5:15:7 | message |
@@ -361,6 +369,7 @@ unresolvedCall
361369
| calls.rb:562:1:562:13 | call to [] |
362370
| calls.rb:562:1:562:39 | call to each |
363371
| calls.rb:570:5:570:14 | call to singleton2 |
372+
| calls.rb:643:9:643:21 | call to allocate |
364373
| hello.rb:20:16:20:26 | ... + ... |
365374
| hello.rb:20:16:20:34 | ... + ... |
366375
| hello.rb:20:16:20:40 | ... + ... |
@@ -483,6 +492,9 @@ publicMethod
483492
| calls.rb:619:5:621:7 | foo |
484493
| calls.rb:622:5:623:7 | bar |
485494
| calls.rb:628:5:630:7 | bar |
495+
| calls.rb:634:5:636:7 | new |
496+
| calls.rb:642:5:644:7 | new |
497+
| calls.rb:646:5:648:7 | instance |
486498
| hello.rb:2:5:4:7 | hello |
487499
| hello.rb:5:5:7:7 | world |
488500
| hello.rb:13:5:15:7 | message |

ruby/ql/test/library-tests/modules/calls.rb

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -630,3 +630,22 @@ def bar
630630
end
631631
end
632632

633+
class CustomNew1
634+
def self.new
635+
C1.new
636+
end
637+
end
638+
639+
CustomNew1.new.instance
640+
641+
class CustomNew2
642+
def self.new
643+
self.allocate
644+
end
645+
646+
def instance
647+
puts "CustomNew2#instance"
648+
end
649+
end
650+
651+
CustomNew2.new.instance

ruby/ql/test/library-tests/modules/methods.expected

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ getMethod
4848
| calls.rb:618:1:624:3 | Included | bar | calls.rb:622:5:623:7 | bar |
4949
| calls.rb:618:1:624:3 | Included | foo | calls.rb:619:5:621:7 | foo |
5050
| calls.rb:626:1:631:3 | IncludesIncluded | bar | calls.rb:628:5:630:7 | bar |
51+
| calls.rb:641:1:649:3 | CustomNew2 | instance | calls.rb:646:5:648:7 | instance |
5152
| hello.rb:1:1:8:3 | EnglishWords | hello | hello.rb:2:5:4:7 | hello |
5253
| hello.rb:1:1:8:3 | EnglishWords | world | hello.rb:5:5:7:7 | world |
5354
| hello.rb:11:1:16:3 | Greeting | message | hello.rb:13:5:15:7 | message |
@@ -497,6 +498,35 @@ lookupMethod
497498
| calls.rb:626:1:631:3 | IncludesIncluded | private_on_main | calls.rb:185:1:186:3 | private_on_main |
498499
| calls.rb:626:1:631:3 | IncludesIncluded | puts | calls.rb:102:5:102:30 | puts |
499500
| calls.rb:626:1:631:3 | IncludesIncluded | to_s | calls.rb:172:5:173:7 | to_s |
501+
| calls.rb:633:1:637:3 | CustomNew1 | add_singleton | calls.rb:367:1:371:3 | add_singleton |
502+
| calls.rb:633:1:637:3 | CustomNew1 | call_block | calls.rb:81:1:83:3 | call_block |
503+
| calls.rb:633:1:637:3 | CustomNew1 | call_instance_m | calls.rb:39:1:41:3 | call_instance_m |
504+
| calls.rb:633:1:637:3 | CustomNew1 | create | calls.rb:278:1:286:3 | create |
505+
| calls.rb:633:1:637:3 | CustomNew1 | foo | calls.rb:1:1:3:3 | foo |
506+
| calls.rb:633:1:637:3 | CustomNew1 | foo | calls.rb:85:1:89:3 | foo |
507+
| calls.rb:633:1:637:3 | CustomNew1 | funny | calls.rb:140:1:142:3 | funny |
508+
| calls.rb:633:1:637:3 | CustomNew1 | indirect | calls.rb:158:1:160:3 | indirect |
509+
| calls.rb:633:1:637:3 | CustomNew1 | new | calls.rb:117:5:117:16 | new |
510+
| calls.rb:633:1:637:3 | CustomNew1 | optional_arg | calls.rb:76:1:79:3 | optional_arg |
511+
| calls.rb:633:1:637:3 | CustomNew1 | pattern_dispatch | calls.rb:343:1:359:3 | pattern_dispatch |
512+
| calls.rb:633:1:637:3 | CustomNew1 | private_on_main | calls.rb:185:1:186:3 | private_on_main |
513+
| calls.rb:633:1:637:3 | CustomNew1 | puts | calls.rb:102:5:102:30 | puts |
514+
| calls.rb:633:1:637:3 | CustomNew1 | to_s | calls.rb:172:5:173:7 | to_s |
515+
| calls.rb:641:1:649:3 | CustomNew2 | add_singleton | calls.rb:367:1:371:3 | add_singleton |
516+
| calls.rb:641:1:649:3 | CustomNew2 | call_block | calls.rb:81:1:83:3 | call_block |
517+
| calls.rb:641:1:649:3 | CustomNew2 | call_instance_m | calls.rb:39:1:41:3 | call_instance_m |
518+
| calls.rb:641:1:649:3 | CustomNew2 | create | calls.rb:278:1:286:3 | create |
519+
| calls.rb:641:1:649:3 | CustomNew2 | foo | calls.rb:1:1:3:3 | foo |
520+
| calls.rb:641:1:649:3 | CustomNew2 | foo | calls.rb:85:1:89:3 | foo |
521+
| calls.rb:641:1:649:3 | CustomNew2 | funny | calls.rb:140:1:142:3 | funny |
522+
| calls.rb:641:1:649:3 | CustomNew2 | indirect | calls.rb:158:1:160:3 | indirect |
523+
| calls.rb:641:1:649:3 | CustomNew2 | instance | calls.rb:646:5:648:7 | instance |
524+
| calls.rb:641:1:649:3 | CustomNew2 | new | calls.rb:117:5:117:16 | new |
525+
| calls.rb:641:1:649:3 | CustomNew2 | optional_arg | calls.rb:76:1:79:3 | optional_arg |
526+
| calls.rb:641:1:649:3 | CustomNew2 | pattern_dispatch | calls.rb:343:1:359:3 | pattern_dispatch |
527+
| calls.rb:641:1:649:3 | CustomNew2 | private_on_main | calls.rb:185:1:186:3 | private_on_main |
528+
| calls.rb:641:1:649:3 | CustomNew2 | puts | calls.rb:102:5:102:30 | puts |
529+
| calls.rb:641:1:649:3 | CustomNew2 | to_s | calls.rb:172:5:173:7 | to_s |
500530
| file://:0:0:0:0 | Class | include | calls.rb:108:5:110:7 | include |
501531
| file://:0:0:0:0 | Class | module_eval | calls.rb:107:5:107:24 | module_eval |
502532
| file://:0:0:0:0 | Class | new | calls.rb:117:5:117:16 | new |
@@ -987,6 +1017,14 @@ enclosingMethod
9871017
| calls.rb:620:9:620:12 | self | calls.rb:619:5:621:7 | foo |
9881018
| calls.rb:620:9:620:16 | call to bar | calls.rb:619:5:621:7 | foo |
9891019
| calls.rb:629:9:629:13 | call to super | calls.rb:628:5:630:7 | bar |
1020+
| calls.rb:635:9:635:10 | C1 | calls.rb:634:5:636:7 | new |
1021+
| calls.rb:635:9:635:14 | call to new | calls.rb:634:5:636:7 | new |
1022+
| calls.rb:643:9:643:12 | self | calls.rb:642:5:644:7 | new |
1023+
| calls.rb:643:9:643:21 | call to allocate | calls.rb:642:5:644:7 | new |
1024+
| calls.rb:647:9:647:34 | call to puts | calls.rb:646:5:648:7 | instance |
1025+
| calls.rb:647:9:647:34 | self | calls.rb:646:5:648:7 | instance |
1026+
| calls.rb:647:14:647:34 | "CustomNew2#instance" | calls.rb:646:5:648:7 | instance |
1027+
| calls.rb:647:15:647:33 | CustomNew2#instance | calls.rb:646:5:648:7 | instance |
9901028
| hello.rb:3:9:3:22 | return | hello.rb:2:5:4:7 | hello |
9911029
| hello.rb:3:16:3:22 | "hello" | hello.rb:2:5:4:7 | hello |
9921030
| hello.rb:3:17:3:21 | hello | hello.rb:2:5:4:7 | hello |

0 commit comments

Comments
 (0)