Skip to content

Commit e4fe1d5

Browse files
committed
check for superclass method definitions in ActiveRecordModelClass#methodMayAccessField
1 parent fb5cfcc commit e4fe1d5

File tree

1 file changed

+11
-5
lines changed

1 file changed

+11
-5
lines changed

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,17 @@ class ActiveRecordModelClass extends ClassDeclaration {
6363
)
6464
}
6565

66+
// Gets the class declaration for this class and all of its super classes
67+
private ModuleBase getAllClassDeclarations() {
68+
result = this.getModule().getSuperClass*().getADeclaration()
69+
}
70+
6671
/**
6772
* Gets methods defined in this class that may access a field from the database.
6873
*/
6974
Method methodMayAccessField() {
70-
result = this.getAMethod() and
75+
// It's a method on this class or one of its super classes
76+
result = this.getAllClassDeclarations().getAMethod() and
7177
// There is a value that can be returned by this method which may include field data
7278
exists(DataFlow::Node returned, ActiveRecordInstanceMethodCall cNode, MethodCall c |
7379
exprNodeReturnedFrom(returned, result) and
@@ -77,10 +83,10 @@ class ActiveRecordModelClass extends ClassDeclaration {
7783
// The referenced method is not built-in, and...
7884
not isBuiltInMethodForActiveRecordModelInstance(c.getMethodName()) and
7985
(
80-
// TODO: this would be more accurate if we also checked methods defined in
81-
// super classes and mixins
82-
// ...There is no matching method definition in the class, or...
83-
not exists(cNode.getInstance().getClass().getMethod(c.getMethodName()))
86+
// ...The receiver does not have a matching method definition, or...
87+
not exists(
88+
cNode.getInstance().getClass().getAllClassDeclarations().getMethod(c.getMethodName())
89+
)
8490
or
8591
// ...the called method can access a field
8692
c.getATarget() = cNode.getInstance().getClass().methodMayAccessField()

0 commit comments

Comments
 (0)