Skip to content

Commit 30ff18a

Browse files
hmachmac
authored
Merge pull request #9919 from hmac/hmac/ar-associations
Ruby: ActiveRecord associations
2 parents 0c6f280 + 22d7b04 commit 30ff18a

File tree

4 files changed

+369
-0
lines changed

4 files changed

+369
-0
lines changed

‎ruby/ql/lib/change-notes/2022-08-05-active-record-associations.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Calls to methods generated by ActiveRecord associations are now recognised as
5+
instantiations of ActiveRecord objects. This increases the sensitivity of
6+
queries such as `rb/sql-injection` and `rb/stored-xss`.

‎ruby/ql/lib/codeql/ruby/frameworks/ActiveRecord.qll‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -516,3 +516,177 @@ private module Persistence {
516516
override DataFlow::Node getValue() { assignNode.getRhs() = result.asExpr() }
517517
}
518518
}
519+
520+
/**
521+
* A method call inside an ActiveRecord model class that establishes an
522+
* association between this model and another model.
523+
*
524+
* ```rb
525+
* class User
526+
* has_many :posts
527+
* has_one :profile
528+
* end
529+
* ```
530+
*/
531+
private class ActiveRecordAssociation extends DataFlow::CallNode {
532+
private ActiveRecordModelClass modelClass;
533+
534+
ActiveRecordAssociation() {
535+
not exists(this.asExpr().getExpr().getEnclosingMethod()) and
536+
this.asExpr().getExpr().getEnclosingModule() = modelClass and
537+
this.getMethodName() = ["has_one", "has_many", "belongs_to", "has_and_belongs_to_many"]
538+
}
539+
540+
/**
541+
* Gets the class which declares this association.
542+
* For example, in
543+
* ```rb
544+
* class User
545+
* has_many :posts
546+
* end
547+
* ```
548+
* the source class is `User`.
549+
*/
550+
ActiveRecordModelClass getSourceClass() { result = modelClass }
551+
552+
/**
553+
* Gets the class which this association refers to.
554+
* For example, in
555+
* ```rb
556+
* class User
557+
* has_many :posts
558+
* end
559+
* ```
560+
* the target class is `Post`.
561+
*/
562+
ActiveRecordModelClass getTargetClass() {
563+
result.getName().toLowerCase() = this.getTargetModelName()
564+
}
565+
566+
/**
567+
* Gets the (lowercase) name of the model this association targets.
568+
* For example, in `has_many :posts`, this is `post`.
569+
*/
570+
string getTargetModelName() {
571+
exists(string s |
572+
s = this.getArgument(0).asExpr().getExpr().getConstantValue().getStringlikeValue()
573+
|
574+
// has_one :profile
575+
// belongs_to :user
576+
this.isSingular() and
577+
result = s
578+
or
579+
// has_many :posts
580+
// has_many :stories
581+
this.isCollection() and
582+
pluralize(result) = s
583+
)
584+
}
585+
586+
/** Holds if this association is one-to-one */
587+
predicate isSingular() { this.getMethodName() = ["has_one", "belongs_to"] }
588+
589+
/** Holds if this association is one-to-many or many-to-many */
590+
predicate isCollection() { this.getMethodName() = ["has_many", "has_and_belongs_to_many"] }
591+
}
592+
593+
/**
594+
* Converts `input` to plural form.
595+
*/
596+
bindingset[input]
597+
bindingset[result]
598+
private string pluralize(string input) {
599+
exists(string stem | stem + "y" = input | result = stem + "ies")
600+
or
601+
result = input + "s"
602+
}
603+
604+
/**
605+
* A call to a method generated by an ActiveRecord association.
606+
* These yield ActiveRecord collection proxies, which act like collections but
607+
* add some additional methods.
608+
* We exclude `<model>_changed?` and `<model>_previously_changed?` because these
609+
* do not yield ActiveRecord instances.
610+
* https://api.rubyonrails.org/classes/ActiveRecord/Associations/ClassMethods.html
611+
*/
612+
private class ActiveRecordAssociationMethodCall extends DataFlow::CallNode {
613+
ActiveRecordAssociation assoc;
614+
615+
ActiveRecordAssociationMethodCall() {
616+
exists(string model | model = assoc.getTargetModelName() |
617+
this.getReceiver().(ActiveRecordInstance).getClass() = assoc.getSourceClass() and
618+
(
619+
assoc.isCollection() and
620+
(
621+
this.getMethodName() = pluralize(model) + ["", "="]
622+
or
623+
this.getMethodName() = "<<"
624+
or
625+
this.getMethodName() = model + ["_ids", "_ids="]
626+
)
627+
or
628+
assoc.isSingular() and
629+
(
630+
this.getMethodName() = model + ["", "="] or
631+
this.getMethodName() = ["build_", "reload_"] + model or
632+
this.getMethodName() = "create_" + model + ["!", ""]
633+
)
634+
)
635+
)
636+
}
637+
638+
ActiveRecordAssociation getAssociation() { result = assoc }
639+
}
640+
641+
/**
642+
* A method call on an ActiveRecord collection proxy that yields one or more
643+
* ActiveRecord instances.
644+
* Example:
645+
* ```rb
646+
* class User < ActiveRecord::Base
647+
* has_many :posts
648+
* end
649+
*
650+
* User.new.posts.create
651+
* ```
652+
* https://api.rubyonrails.org/classes/ActiveRecord/Associations/ClassMethods.html
653+
*/
654+
private class ActiveRecordCollectionProxyMethodCall extends DataFlow::CallNode {
655+
ActiveRecordCollectionProxyMethodCall() {
656+
this.getMethodName() =
657+
[
658+
"push", "concat", "build", "create", "create!", "delete", "delete_all", "destroy",
659+
"destroy_all", "find", "distinct", "reset", "reload"
660+
] and
661+
(
662+
this.getReceiver().(ActiveRecordAssociationMethodCall).getAssociation().isCollection()
663+
or
664+
exists(ActiveRecordCollectionProxyMethodCall receiver | receiver = this.getReceiver() |
665+
receiver.getAssociation().isCollection() and
666+
receiver.getMethodName() = ["reset", "reload", "distinct"]
667+
)
668+
)
669+
}
670+
671+
ActiveRecordAssociation getAssociation() {
672+
result = this.getReceiver().(ActiveRecordAssociationMethodCall).getAssociation()
673+
}
674+
}
675+
676+
/**
677+
* A call to an association method which yields ActiveRecord instances.
678+
*/
679+
private class ActiveRecordAssociationModelInstantiation extends ActiveRecordModelInstantiation instanceof ActiveRecordAssociationMethodCall {
680+
override ActiveRecordModelClass getClass() {
681+
result = this.(ActiveRecordAssociationMethodCall).getAssociation().getTargetClass()
682+
}
683+
}
684+
685+
/**
686+
* A call to a method on a collection proxy which yields ActiveRecord instances.
687+
*/
688+
private class ActiveRecordCollectionProxyModelInstantiation extends ActiveRecordModelInstantiation instanceof ActiveRecordCollectionProxyMethodCall {
689+
override ActiveRecordModelClass getClass() {
690+
result = this.(ActiveRecordCollectionProxyMethodCall).getAssociation().getTargetClass()
691+
}
692+
}

‎ruby/ql/test/library-tests/frameworks/active_record/ActiveRecord.expected‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,102 @@ activeRecordModelClasses
22
| ActiveRecord.rb:1:1:3:3 | UserGroup |
33
| ActiveRecord.rb:5:1:15:3 | User |
44
| ActiveRecord.rb:17:1:21:3 | Admin |
5+
| associations.rb:1:1:3:3 | Author |
6+
| associations.rb:5:1:9:3 | Post |
7+
| associations.rb:11:1:13:3 | Tag |
8+
| associations.rb:15:1:17:3 | Comment |
59
activeRecordInstances
610
| ActiveRecord.rb:9:5:9:68 | call to find |
711
| ActiveRecord.rb:13:5:13:40 | call to find_by |
12+
| ActiveRecord.rb:13:5:13:46 | call to users |
813
| ActiveRecord.rb:36:5:36:30 | call to find_by_name |
914
| ActiveRecord.rb:55:5:57:7 | if ... |
1015
| ActiveRecord.rb:55:43:56:40 | then ... |
1116
| ActiveRecord.rb:56:7:56:40 | call to find_by |
1217
| ActiveRecord.rb:60:5:60:33 | call to find_by |
1318
| ActiveRecord.rb:62:5:62:34 | call to find |
19+
| associations.rb:19:1:19:20 | ... = ... |
20+
| associations.rb:19:1:19:20 | ... = ... |
21+
| associations.rb:19:11:19:20 | call to new |
22+
| associations.rb:21:1:21:28 | ... = ... |
23+
| associations.rb:21:1:21:28 | ... = ... |
24+
| associations.rb:21:9:21:15 | author1 |
25+
| associations.rb:21:9:21:21 | call to posts |
26+
| associations.rb:21:9:21:28 | call to create |
27+
| associations.rb:23:1:23:32 | ... = ... |
28+
| associations.rb:23:12:23:16 | post1 |
29+
| associations.rb:23:12:23:25 | call to comments |
30+
| associations.rb:23:12:23:32 | call to create |
31+
| associations.rb:25:1:25:22 | ... = ... |
32+
| associations.rb:25:1:25:22 | ... = ... |
33+
| associations.rb:25:11:25:15 | post1 |
34+
| associations.rb:25:11:25:22 | call to author |
35+
| associations.rb:27:1:27:28 | ... = ... |
36+
| associations.rb:27:1:27:28 | ... = ... |
37+
| associations.rb:27:9:27:15 | author2 |
38+
| associations.rb:27:9:27:21 | call to posts |
39+
| associations.rb:27:9:27:28 | call to create |
40+
| associations.rb:29:1:29:7 | author2 |
41+
| associations.rb:29:1:29:13 | call to posts |
42+
| associations.rb:29:1:29:22 | ... << ... |
43+
| associations.rb:29:18:29:22 | post2 |
44+
| associations.rb:31:1:31:5 | post1 |
45+
| associations.rb:31:1:31:12 | __synth__0 |
46+
| associations.rb:31:1:31:12 | call to author= |
47+
| associations.rb:31:1:31:22 | ... |
48+
| associations.rb:31:16:31:22 | ... = ... |
49+
| associations.rb:31:16:31:22 | ... = ... |
50+
| associations.rb:31:16:31:22 | author2 |
51+
| associations.rb:35:1:35:5 | post2 |
52+
| associations.rb:35:1:35:14 | call to comments |
53+
| associations.rb:35:1:35:21 | call to create |
54+
| associations.rb:37:1:37:7 | author1 |
55+
| associations.rb:37:1:37:13 | call to posts |
56+
| associations.rb:37:1:37:20 | call to reload |
57+
| associations.rb:37:1:37:27 | call to create |
58+
| associations.rb:39:1:39:5 | post1 |
59+
| associations.rb:40:1:40:5 | post1 |
60+
| associations.rb:42:1:42:7 | author1 |
61+
| associations.rb:42:1:42:13 | call to posts |
62+
| associations.rb:42:1:42:25 | call to push |
63+
| associations.rb:42:20:42:24 | post2 |
64+
| associations.rb:43:1:43:7 | author1 |
65+
| associations.rb:43:1:43:13 | call to posts |
66+
| associations.rb:43:1:43:27 | call to concat |
67+
| associations.rb:43:22:43:26 | post2 |
68+
| associations.rb:44:1:44:7 | author1 |
69+
| associations.rb:44:1:44:13 | call to posts |
70+
| associations.rb:44:1:44:19 | call to build |
71+
| associations.rb:45:1:45:7 | author1 |
72+
| associations.rb:45:1:45:13 | call to posts |
73+
| associations.rb:45:1:45:20 | call to create |
74+
| associations.rb:46:1:46:7 | author1 |
75+
| associations.rb:46:1:46:13 | call to posts |
76+
| associations.rb:46:1:46:21 | call to create! |
77+
| associations.rb:47:1:47:7 | author1 |
78+
| associations.rb:47:1:47:13 | call to posts |
79+
| associations.rb:47:1:47:20 | call to delete |
80+
| associations.rb:48:1:48:7 | author1 |
81+
| associations.rb:48:1:48:13 | call to posts |
82+
| associations.rb:48:1:48:24 | call to delete_all |
83+
| associations.rb:49:1:49:7 | author1 |
84+
| associations.rb:49:1:49:13 | call to posts |
85+
| associations.rb:49:1:49:21 | call to destroy |
86+
| associations.rb:50:1:50:7 | author1 |
87+
| associations.rb:50:1:50:13 | call to posts |
88+
| associations.rb:50:1:50:25 | call to destroy_all |
89+
| associations.rb:51:1:51:7 | author1 |
90+
| associations.rb:51:1:51:13 | call to posts |
91+
| associations.rb:51:1:51:22 | call to distinct |
92+
| associations.rb:51:1:51:36 | call to find |
93+
| associations.rb:52:1:52:7 | author1 |
94+
| associations.rb:52:1:52:13 | call to posts |
95+
| associations.rb:52:1:52:19 | call to reset |
96+
| associations.rb:52:1:52:33 | call to find |
97+
| associations.rb:53:1:53:7 | author1 |
98+
| associations.rb:53:1:53:13 | call to posts |
99+
| associations.rb:53:1:53:20 | call to reload |
100+
| associations.rb:53:1:53:34 | call to find |
14101
activeRecordSqlExecutionRanges
15102
| ActiveRecord.rb:9:33:9:67 | "name='#{...}' and pass='#{...}'" |
16103
| ActiveRecord.rb:19:16:19:24 | condition |
@@ -53,6 +140,13 @@ activeRecordModelClassMethodCalls
53140
| ActiveRecord.rb:92:5:92:71 | call to update |
54141
| ActiveRecord.rb:98:13:98:54 | call to annotate |
55142
| ActiveRecord.rb:102:13:102:77 | call to annotate |
143+
| associations.rb:2:3:2:17 | call to has_many |
144+
| associations.rb:6:3:6:20 | call to belongs_to |
145+
| associations.rb:7:3:7:20 | call to has_many |
146+
| associations.rb:8:3:8:31 | call to has_and_belongs_to_many |
147+
| associations.rb:12:3:12:32 | call to has_and_belongs_to_many |
148+
| associations.rb:16:3:16:18 | call to belongs_to |
149+
| associations.rb:19:11:19:20 | call to new |
56150
potentiallyUnsafeSqlExecutingMethodCall
57151
| ActiveRecord.rb:9:5:9:68 | call to find |
58152
| ActiveRecord.rb:19:5:19:25 | call to destroy_by |
@@ -68,10 +162,51 @@ potentiallyUnsafeSqlExecutingMethodCall
68162
activeRecordModelInstantiations
69163
| ActiveRecord.rb:9:5:9:68 | call to find | ActiveRecord.rb:5:1:15:3 | User |
70164
| ActiveRecord.rb:13:5:13:40 | call to find_by | ActiveRecord.rb:1:1:3:3 | UserGroup |
165+
| ActiveRecord.rb:13:5:13:46 | call to users | ActiveRecord.rb:5:1:15:3 | User |
71166
| ActiveRecord.rb:36:5:36:30 | call to find_by_name | ActiveRecord.rb:5:1:15:3 | User |
72167
| ActiveRecord.rb:56:7:56:40 | call to find_by | ActiveRecord.rb:5:1:15:3 | User |
73168
| ActiveRecord.rb:60:5:60:33 | call to find_by | ActiveRecord.rb:5:1:15:3 | User |
74169
| ActiveRecord.rb:62:5:62:34 | call to find | ActiveRecord.rb:5:1:15:3 | User |
170+
| associations.rb:19:11:19:20 | call to new | associations.rb:1:1:3:3 | Author |
171+
| associations.rb:21:9:21:21 | call to posts | associations.rb:5:1:9:3 | Post |
172+
| associations.rb:21:9:21:28 | call to create | associations.rb:5:1:9:3 | Post |
173+
| associations.rb:23:12:23:25 | call to comments | associations.rb:15:1:17:3 | Comment |
174+
| associations.rb:23:12:23:32 | call to create | associations.rb:15:1:17:3 | Comment |
175+
| associations.rb:25:11:25:22 | call to author | associations.rb:1:1:3:3 | Author |
176+
| associations.rb:27:9:27:21 | call to posts | associations.rb:5:1:9:3 | Post |
177+
| associations.rb:27:9:27:28 | call to create | associations.rb:5:1:9:3 | Post |
178+
| associations.rb:29:1:29:13 | call to posts | associations.rb:5:1:9:3 | Post |
179+
| associations.rb:29:1:29:22 | ... << ... | associations.rb:11:1:13:3 | Tag |
180+
| associations.rb:29:1:29:22 | ... << ... | associations.rb:15:1:17:3 | Comment |
181+
| associations.rb:31:1:31:12 | call to author= | associations.rb:1:1:3:3 | Author |
182+
| associations.rb:35:1:35:14 | call to comments | associations.rb:15:1:17:3 | Comment |
183+
| associations.rb:35:1:35:21 | call to create | associations.rb:15:1:17:3 | Comment |
184+
| associations.rb:37:1:37:13 | call to posts | associations.rb:5:1:9:3 | Post |
185+
| associations.rb:37:1:37:20 | call to reload | associations.rb:5:1:9:3 | Post |
186+
| associations.rb:42:1:42:13 | call to posts | associations.rb:5:1:9:3 | Post |
187+
| associations.rb:42:1:42:25 | call to push | associations.rb:5:1:9:3 | Post |
188+
| associations.rb:43:1:43:13 | call to posts | associations.rb:5:1:9:3 | Post |
189+
| associations.rb:43:1:43:27 | call to concat | associations.rb:5:1:9:3 | Post |
190+
| associations.rb:44:1:44:13 | call to posts | associations.rb:5:1:9:3 | Post |
191+
| associations.rb:44:1:44:19 | call to build | associations.rb:5:1:9:3 | Post |
192+
| associations.rb:45:1:45:13 | call to posts | associations.rb:5:1:9:3 | Post |
193+
| associations.rb:45:1:45:20 | call to create | associations.rb:5:1:9:3 | Post |
194+
| associations.rb:46:1:46:13 | call to posts | associations.rb:5:1:9:3 | Post |
195+
| associations.rb:46:1:46:21 | call to create! | associations.rb:5:1:9:3 | Post |
196+
| associations.rb:47:1:47:13 | call to posts | associations.rb:5:1:9:3 | Post |
197+
| associations.rb:47:1:47:20 | call to delete | associations.rb:5:1:9:3 | Post |
198+
| associations.rb:48:1:48:13 | call to posts | associations.rb:5:1:9:3 | Post |
199+
| associations.rb:48:1:48:24 | call to delete_all | associations.rb:5:1:9:3 | Post |
200+
| associations.rb:49:1:49:13 | call to posts | associations.rb:5:1:9:3 | Post |
201+
| associations.rb:49:1:49:21 | call to destroy | associations.rb:5:1:9:3 | Post |
202+
| associations.rb:50:1:50:13 | call to posts | associations.rb:5:1:9:3 | Post |
203+
| associations.rb:50:1:50:25 | call to destroy_all | associations.rb:5:1:9:3 | Post |
204+
| associations.rb:51:1:51:13 | call to posts | associations.rb:5:1:9:3 | Post |
205+
| associations.rb:51:1:51:22 | call to distinct | associations.rb:5:1:9:3 | Post |
206+
| associations.rb:52:1:52:13 | call to posts | associations.rb:5:1:9:3 | Post |
207+
| associations.rb:52:1:52:19 | call to reset | associations.rb:5:1:9:3 | Post |
208+
| associations.rb:53:1:53:13 | call to posts | associations.rb:5:1:9:3 | Post |
209+
| associations.rb:53:1:53:20 | call to reload | associations.rb:5:1:9:3 | Post |
75210
persistentWriteAccesses
76211
| ActiveRecord.rb:72:5:72:24 | call to create | ActiveRecord.rb:72:18:72:23 | call to params |
77212
| ActiveRecord.rb:76:5:76:66 | call to create | ActiveRecord.rb:76:24:76:36 | ...[...] |
@@ -82,3 +217,4 @@ persistentWriteAccesses
82217
| ActiveRecord.rb:88:5:88:69 | call to update | ActiveRecord.rb:88:27:88:39 | ...[...] |
83218
| ActiveRecord.rb:88:5:88:69 | call to update | ActiveRecord.rb:88:52:88:68 | ...[...] |
84219
| ActiveRecord.rb:92:5:92:71 | call to update | ActiveRecord.rb:92:21:92:70 | call to [] |
220+
| associations.rb:31:16:31:22 | ... = ... | associations.rb:31:16:31:22 | author2 |

0 commit comments

Comments
 (0)