Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 19c7f7b

Browse filesBrowse files
alexrfordalexrford
authored
Merge pull request #8271 from github/alexrford/ruby/orm-write-access
Ruby: Add `OrmWriteAccess` concept to model writes to a DB using an ORM
2 parents d4808a7 + edf8a3f commit 19c7f7b
Copy full SHA for 19c7f7b

File tree

Expand file treeCollapse file tree

8 files changed

+261
-0
lines changed
Filter options
Expand file treeCollapse file tree

8 files changed

+261
-0
lines changed

‎ruby/ql/lib/change-notes/2022-02-28-orm-write-access.md

Copy file name to clipboard
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added `OrmWriteAccess` concept to model data written to a database using an object-relational mapping (ORM) library.

‎ruby/ql/lib/codeql/ruby/Concepts.qll

Copy file name to clipboardExpand all lines: ruby/ql/lib/codeql/ruby/Concepts.qll
+29Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,35 @@ module OrmInstantiation {
625625
}
626626
}
627627

628+
/**
629+
* A data flow node that writes persistent data.
630+
*
631+
* Extend this class to refine existing API models. If you want to model new APIs,
632+
* extend `PersistentWriteAccess::Range` instead.
633+
*/
634+
class PersistentWriteAccess extends DataFlow::Node instanceof PersistentWriteAccess::Range {
635+
/**
636+
* Gets the data flow node corresponding to the written value.
637+
*/
638+
DataFlow::Node getValue() { result = super.getValue() }
639+
}
640+
641+
/** Provides a class for modeling new persistent write access APIs. */
642+
module PersistentWriteAccess {
643+
/**
644+
* A data flow node that writes persistent data.
645+
*
646+
* Extend this class to model new APIs. If you want to refine existing API models,
647+
* extend `PersistentWriteAccess` instead.
648+
*/
649+
abstract class Range extends DataFlow::Node {
650+
/**
651+
* Gets the data flow node corresponding to the written value.
652+
*/
653+
abstract DataFlow::Node getValue();
654+
}
655+
}
656+
628657
/**
629658
* A data-flow node that may set or unset Cross-site request forgery protection.
630659
*

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

Copy file name to clipboardExpand all lines: ruby/ql/lib/codeql/ruby/frameworks/ActiveRecord.qll
+148Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,3 +316,151 @@ private class ActiveRecordInstanceMethodCall extends DataFlow::CallNode {
316316

317317
ActiveRecordInstance getInstance() { result = instance }
318318
}
319+
320+
/**
321+
* Provides modeling relating to the `ActiveRecord::Persistence` module.
322+
*/
323+
private module Persistence {
324+
/**
325+
* Holds if there is a hash literal argument to `call` at `argIndex`
326+
* containing a KV pair with value `value`.
327+
*/
328+
private predicate hashArgumentWithValue(
329+
DataFlow::CallNode call, int argIndex, DataFlow::ExprNode value
330+
) {
331+
exists(ExprNodes::HashLiteralCfgNode hash, ExprNodes::PairCfgNode pair |
332+
hash = call.getArgument(argIndex).asExpr() and
333+
pair = hash.getAKeyValuePair()
334+
|
335+
value.asExpr() = pair.getValue()
336+
)
337+
}
338+
339+
/**
340+
* Holds if `call` has a keyword argument of with value `value`.
341+
*/
342+
private predicate keywordArgumentWithValue(DataFlow::CallNode call, DataFlow::ExprNode value) {
343+
exists(ExprNodes::PairCfgNode pair | pair = call.getArgument(_).asExpr() |
344+
value.asExpr() = pair.getValue()
345+
)
346+
}
347+
348+
/** A call to e.g. `User.create(name: "foo")` */
349+
private class CreateLikeCall extends DataFlow::CallNode, PersistentWriteAccess::Range {
350+
private ActiveRecordModelClass modelCls;
351+
352+
CreateLikeCall() {
353+
modelCls = this.asExpr().getExpr().(ActiveRecordModelClassMethodCall).getReceiverClass() and
354+
this.getMethodName() =
355+
[
356+
"create", "create!", "create_or_find_by", "create_or_find_by!", "find_or_create_by",
357+
"find_or_create_by!", "insert", "insert!"
358+
]
359+
}
360+
361+
override DataFlow::Node getValue() {
362+
// attrs as hash elements in arg0
363+
hashArgumentWithValue(this, 0, result) or
364+
keywordArgumentWithValue(this, result)
365+
}
366+
}
367+
368+
/** A call to e.g. `User.update(1, name: "foo")` */
369+
private class UpdateLikeClassMethodCall extends DataFlow::CallNode, PersistentWriteAccess::Range {
370+
private ActiveRecordModelClass modelCls;
371+
372+
UpdateLikeClassMethodCall() {
373+
modelCls = this.asExpr().getExpr().(ActiveRecordModelClassMethodCall).getReceiverClass() and
374+
this.getMethodName() = ["update", "update!", "upsert"]
375+
}
376+
377+
override DataFlow::Node getValue() {
378+
keywordArgumentWithValue(this, result)
379+
or
380+
// Case where 2 array args are passed - the first an array of IDs, and the
381+
// second an array of hashes - each hash corresponding to an ID in the
382+
// first array.
383+
exists(ExprNodes::ArrayLiteralCfgNode hashesArray |
384+
this.getArgument(0).asExpr() instanceof ExprNodes::ArrayLiteralCfgNode and
385+
hashesArray = this.getArgument(1).asExpr()
386+
|
387+
exists(ExprNodes::HashLiteralCfgNode hash, ExprNodes::PairCfgNode pair |
388+
hash = hashesArray.getArgument(_) and
389+
pair = hash.getAKeyValuePair()
390+
|
391+
result.asExpr() = pair.getValue()
392+
)
393+
)
394+
}
395+
}
396+
397+
/** A call to e.g. `User.insert_all([{name: "foo"}, {name: "bar"}])` */
398+
private class InsertAllLikeCall extends DataFlow::CallNode, PersistentWriteAccess::Range {
399+
private ExprNodes::ArrayLiteralCfgNode arr;
400+
private ActiveRecordModelClass modelCls;
401+
402+
InsertAllLikeCall() {
403+
modelCls = this.asExpr().getExpr().(ActiveRecordModelClassMethodCall).getReceiverClass() and
404+
this.getMethodName() = ["insert_all", "insert_all!", "upsert_all"] and
405+
arr = this.getArgument(0).asExpr()
406+
}
407+
408+
override DataFlow::Node getValue() {
409+
// attrs as hash elements of members of array arg0
410+
exists(ExprNodes::HashLiteralCfgNode hash, ExprNodes::PairCfgNode pair |
411+
hash = arr.getArgument(_) and
412+
pair = hash.getAKeyValuePair()
413+
|
414+
result.asExpr() = pair.getValue()
415+
)
416+
}
417+
}
418+
419+
/** A call to e.g. `user.update(name: "foo")` */
420+
private class UpdateLikeInstanceMethodCall extends PersistentWriteAccess::Range,
421+
ActiveRecordInstanceMethodCall {
422+
UpdateLikeInstanceMethodCall() {
423+
this.getMethodName() = ["update", "update!", "update_attributes", "update_attributes!"]
424+
}
425+
426+
override DataFlow::Node getValue() {
427+
// attrs as hash elements in arg0
428+
hashArgumentWithValue(this, 0, result)
429+
or
430+
// keyword arg
431+
keywordArgumentWithValue(this, result)
432+
}
433+
}
434+
435+
/** A call to e.g. `user.update_attribute(name, "foo")` */
436+
private class UpdateAttributeCall extends PersistentWriteAccess::Range,
437+
ActiveRecordInstanceMethodCall {
438+
UpdateAttributeCall() { this.getMethodName() = "update_attribute" }
439+
440+
override DataFlow::Node getValue() {
441+
// e.g. `foo.update_attribute(key, value)`
442+
result = this.getArgument(1)
443+
}
444+
}
445+
446+
/**
447+
* An assignment like `user.name = "foo"`. Though this does not write to the
448+
* database without a subsequent call to persist the object, it is considered
449+
* as an `PersistentWriteAccess` to avoid missing cases where the path to a
450+
* subsequent write is not clear.
451+
*/
452+
private class AssignAttribute extends PersistentWriteAccess::Range {
453+
private ExprNodes::AssignExprCfgNode assignNode;
454+
455+
AssignAttribute() {
456+
exists(DataFlow::CallNode setter |
457+
assignNode = this.asExpr() and
458+
setter.getArgument(0) = this and
459+
setter instanceof ActiveRecordInstanceMethodCall and
460+
setter.asExpr().getExpr() instanceof SetterMethodCall
461+
)
462+
}
463+
464+
override DataFlow::Node getValue() { assignNode.getRhs() = result.asExpr() }
465+
}
466+
}

‎ruby/ql/test/library-tests/concepts/PersistentWriteAccess.expected

Copy file name to clipboard
+22Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
| app/controllers/users_controller.rb:5:7:5:44 | call to create! | app/controllers/users_controller.rb:5:26:5:29 | "U1" |
2+
| app/controllers/users_controller.rb:5:7:5:44 | call to create! | app/controllers/users_controller.rb:5:37:5:43 | call to get_uid |
3+
| app/controllers/users_controller.rb:6:7:6:29 | call to create | app/controllers/users_controller.rb:6:25:6:28 | "U2" |
4+
| app/controllers/users_controller.rb:7:7:7:31 | call to insert | app/controllers/users_controller.rb:7:26:7:29 | "U3" |
5+
| app/controllers/users_controller.rb:10:7:10:32 | call to update | app/controllers/users_controller.rb:10:28:10:31 | "U4" |
6+
| app/controllers/users_controller.rb:11:7:11:73 | call to update! | app/controllers/users_controller.rb:11:39:11:42 | "U5" |
7+
| app/controllers/users_controller.rb:11:7:11:73 | call to update! | app/controllers/users_controller.rb:11:53:11:56 | "U6" |
8+
| app/controllers/users_controller.rb:11:7:11:73 | call to update! | app/controllers/users_controller.rb:11:67:11:70 | "U7" |
9+
| app/controllers/users_controller.rb:14:7:14:66 | call to insert_all | app/controllers/users_controller.rb:14:31:14:34 | "U8" |
10+
| app/controllers/users_controller.rb:14:7:14:66 | call to insert_all | app/controllers/users_controller.rb:14:45:14:48 | "U9" |
11+
| app/controllers/users_controller.rb:14:7:14:66 | call to insert_all | app/controllers/users_controller.rb:14:59:14:63 | "U10" |
12+
| app/controllers/users_controller.rb:19:7:19:30 | call to update | app/controllers/users_controller.rb:19:25:19:29 | "U11" |
13+
| app/controllers/users_controller.rb:20:7:20:57 | call to update_attributes | app/controllers/users_controller.rb:20:37:20:41 | "U12" |
14+
| app/controllers/users_controller.rb:20:7:20:57 | call to update_attributes | app/controllers/users_controller.rb:20:49:20:55 | call to get_uid |
15+
| app/controllers/users_controller.rb:23:7:23:42 | call to update_attribute | app/controllers/users_controller.rb:23:37:23:41 | "U13" |
16+
| app/controllers/users_controller.rb:26:7:26:15 | ... = ... | app/controllers/users_controller.rb:26:19:26:23 | "U14" |
17+
| app/models/user.rb:4:5:4:28 | call to update | app/models/user.rb:4:23:4:27 | "U15" |
18+
| app/models/user.rb:5:5:5:23 | call to update | app/models/user.rb:5:18:5:22 | "U16" |
19+
| app/models/user.rb:6:5:6:56 | call to update_attributes | app/models/user.rb:6:35:6:39 | "U17" |
20+
| app/models/user.rb:6:5:6:56 | call to update_attributes | app/models/user.rb:6:51:6:54 | true |
21+
| app/models/user.rb:9:5:9:40 | call to update_attribute | app/models/user.rb:9:35:9:39 | "U18" |
22+
| app/models/user.rb:12:5:12:13 | ... = ... | app/models/user.rb:12:17:12:21 | "U19" |

‎ruby/ql/test/library-tests/concepts/PersistentWriteAccess.ql

Copy file name to clipboard
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import codeql.ruby.DataFlow
2+
import codeql.ruby.Concepts
3+
4+
query predicate persistentWriteAccesses(PersistentWriteAccess acc, DataFlow::Node value) {
5+
value = acc.getValue()
6+
}

‎ruby/ql/test/library-tests/concepts/app/controllers/users_controller.rb

Copy file name to clipboard
+34Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
module Users
2+
class UsersController < ApplicationController
3+
def create_or_modify
4+
# CreateLikeCall
5+
User.create!(name: "U1", uid: get_uid)
6+
User.create(name: "U2")
7+
User.insert({name: "U3"})
8+
9+
# UpdateLikeClassMethodCall
10+
User.update(4, name: "U4")
11+
User.update!([5, 6, 7], [{name: "U5"}, {name: "U6"}, {name: "U7"}])
12+
13+
# InsertAllLikeCall
14+
User.insert_all([{name: "U8"}, {name: "U9"}, {name: "U10"}])
15+
16+
user = User.find(5)
17+
18+
# UpdateLikeInstanceMethodCall
19+
user.update(name: "U11")
20+
user.update_attributes({name: "U12", uid: get_uid})
21+
22+
# UpdateAttributeCall
23+
user.update_attribute("name", "U13")
24+
25+
# AssignAttributeCall
26+
user.name = "U14"
27+
user.save
28+
end
29+
30+
def get_uid
31+
User.last.id + 1
32+
end
33+
end
34+
end

‎ruby/ql/test/library-tests/concepts/app/models/application_record.rb

Copy file name to clipboard
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
class ApplicationRecord < ActiveRecord::Base
2+
self.abstract_class = true
3+
end

‎ruby/ql/test/library-tests/concepts/app/models/user.rb

Copy file name to clipboard
+15Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
class User < ActiveRecord::Base
2+
def t1
3+
# UpdateLikeInstanceMethodCall
4+
self.update(name: "U15")
5+
update(name: "U16")
6+
self.update_attributes({name: "U17", isAdmin: true})
7+
8+
# UpdateAttributeCall
9+
self.update_attribute("name", "U18")
10+
11+
# AssignAttributeCall
12+
self.name = "U19"
13+
user.save
14+
end
15+
end

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.