+
+
+Database calls in loops are slower than running a single query and consume more resources. This
+can lead to denial of service attacks if the loop bounds can be controlled by an attacker.
+
+
+
+Ensure that where possible, database queries are not run in a loop, instead running a single query to get all relevant data.
+
+
+
+
+In the example below, users in a database are queried one by one in a loop:
+
+
+
+This is corrected by running a single query that selects all of the users at once:
+
+
+
+
+
+
+
+
diff --git a/ql/src/experimental/CWE-400/DatabaseCallInLoop.ql b/ql/src/experimental/CWE-400/DatabaseCallInLoop.ql
new file mode 100644
index 00000000000..2e4f25fe495
--- /dev/null
+++ b/ql/src/experimental/CWE-400/DatabaseCallInLoop.ql
@@ -0,0 +1,69 @@
+/**
+ * @name Database call in loop
+ * @description Detects database operations within loops.
+ * Doing operations in series can be slow and lead to N+1 situations.
+ * @kind path-problem
+ * @problem.severity warning
+ * @precision high
+ * @id go/examples/database-call-in-loop
+ */
+
+import go
+
+class DatabaseAccess extends DataFlow::MethodCallNode {
+ DatabaseAccess() {
+ exists(string name |
+ this.getTarget().hasQualifiedName(Gorm::packagePath(), "DB", name) and
+ // all terminating Gorm methods
+ name =
+ [
+ "Find", "Take", "Last", "Scan", "Row", "Rows", "ScanRows", "Pluck", "Count", "First",
+ "FirstOrInit", "FindOrCreate", "Update", "Updates", "UpdateColumn", "UpdateColumns",
+ "Save", "Create", "Delete", "Exec"
+ ]
+ )
+ }
+}
+
+class CallGraphNode extends Locatable {
+ CallGraphNode() {
+ this instanceof LoopStmt
+ or
+ this instanceof CallExpr
+ or
+ this instanceof FuncDef
+ }
+}
+
+/**
+ * Holds if `pred` calls `succ`, i.e. is an edge in the call graph,
+ * This includes explicit edges from call -> callee, to produce better paths.
+ */
+predicate callGraphEdge(CallGraphNode pred, CallGraphNode succ) {
+ // Go from a loop to an enclosed expression.
+ pred.(LoopStmt).getBody().getAChild*() = succ.(CallExpr)
+ or
+ // Go from a call to the called function.
+ pred.(CallExpr) = succ.(FuncDef).getACall().asExpr()
+ or
+ // Go from a function to an enclosed loop.
+ pred.(FuncDef) = succ.(LoopStmt).getEnclosingFunction()
+ or
+ // Go from a function to an enclosed call.
+ pred.(FuncDef) = succ.(CallExpr).getEnclosingFunction()
+}
+
+query predicate edges(CallGraphNode pred, CallGraphNode succ) {
+ callGraphEdge(pred, succ) and
+ // Limit the range of edges to only those that are relevant.
+ // This helps to speed up the query by reducing the size of the outputted path information.
+ exists(LoopStmt loop, DatabaseAccess dbAccess |
+ // is between a loop and a db access
+ callGraphEdge*(loop, pred) and
+ callGraphEdge*(succ, dbAccess.asExpr())
+ )
+}
+
+from LoopStmt loop, DatabaseAccess dbAccess
+where edges*(loop, dbAccess.asExpr())
+select dbAccess, loop, dbAccess, "$@ is called in $@", dbAccess, dbAccess.toString(), loop, "a loop"
diff --git a/ql/src/experimental/CWE-400/DatabaseCallInLoopGood.go b/ql/src/experimental/CWE-400/DatabaseCallInLoopGood.go
new file mode 100644
index 00000000000..22928a6abc2
--- /dev/null
+++ b/ql/src/experimental/CWE-400/DatabaseCallInLoopGood.go
@@ -0,0 +1,9 @@
+package main
+
+import "gorm.io/gorm"
+
+func getUsersGood(db *gorm.DB, names []string) []User {
+ res := make([]User, 0, len(names))
+ db.Where("name IN ?", names).Find(&res)
+ return res
+}
diff --git a/ql/test/experimental/CWE-400/DatabaseCallInLoop.expected b/ql/test/experimental/CWE-400/DatabaseCallInLoop.expected
new file mode 100644
index 00000000000..bb197203f22
--- /dev/null
+++ b/ql/test/experimental/CWE-400/DatabaseCallInLoop.expected
@@ -0,0 +1,13 @@
+edges
+| DatabaseCallInLoop.go:7:2:11:2 | range statement | DatabaseCallInLoop.go:9:3:9:41 | call to First |
+| test.go:10:1:12:1 | function declaration | test.go:11:2:11:13 | call to Take |
+| test.go:14:1:16:1 | function declaration | test.go:15:2:15:13 | call to runQuery |
+| test.go:15:2:15:13 | call to runQuery | test.go:10:1:12:1 | function declaration |
+| test.go:20:2:22:2 | for statement | test.go:21:3:21:14 | call to runQuery |
+| test.go:21:3:21:14 | call to runQuery | test.go:10:1:12:1 | function declaration |
+| test.go:24:2:26:2 | for statement | test.go:25:3:25:17 | call to runRunQuery |
+| test.go:25:3:25:17 | call to runRunQuery | test.go:14:1:16:1 | function declaration |
+#select
+| DatabaseCallInLoop.go:9:3:9:41 | call to First | DatabaseCallInLoop.go:7:2:11:2 | range statement | DatabaseCallInLoop.go:9:3:9:41 | call to First | $@ is called in $@ | DatabaseCallInLoop.go:9:3:9:41 | call to First | call to First | DatabaseCallInLoop.go:7:2:11:2 | range statement | a loop |
+| test.go:11:2:11:13 | call to Take | test.go:20:2:22:2 | for statement | test.go:11:2:11:13 | call to Take | $@ is called in $@ | test.go:11:2:11:13 | call to Take | call to Take | test.go:20:2:22:2 | for statement | a loop |
+| test.go:11:2:11:13 | call to Take | test.go:24:2:26:2 | for statement | test.go:11:2:11:13 | call to Take | $@ is called in $@ | test.go:11:2:11:13 | call to Take | call to Take | test.go:24:2:26:2 | for statement | a loop |
diff --git a/ql/test/experimental/CWE-400/DatabaseCallInLoop.go b/ql/test/experimental/CWE-400/DatabaseCallInLoop.go
new file mode 100644
index 00000000000..138bbbcd9d4
--- /dev/null
+++ b/ql/test/experimental/CWE-400/DatabaseCallInLoop.go
@@ -0,0 +1,13 @@
+package main
+
+import "gorm.io/gorm"
+
+func getUsers(db *gorm.DB, names []string) []User {
+ res := make([]User, 0, len(names))
+ for _, name := range names {
+ var user User
+ db.Where("name = ?", name).First(&user)
+ res = append(res, user)
+ }
+ return res
+}
diff --git a/ql/test/experimental/CWE-400/DatabaseCallInLoop.qlref b/ql/test/experimental/CWE-400/DatabaseCallInLoop.qlref
new file mode 100644
index 00000000000..63f27c9b41f
--- /dev/null
+++ b/ql/test/experimental/CWE-400/DatabaseCallInLoop.qlref
@@ -0,0 +1 @@
+experimental/CWE-400/DatabaseCallInLoop.ql
diff --git a/ql/test/experimental/CWE-400/DatabaseCallInLoopGood.go b/ql/test/experimental/CWE-400/DatabaseCallInLoopGood.go
new file mode 100644
index 00000000000..22928a6abc2
--- /dev/null
+++ b/ql/test/experimental/CWE-400/DatabaseCallInLoopGood.go
@@ -0,0 +1,9 @@
+package main
+
+import "gorm.io/gorm"
+
+func getUsersGood(db *gorm.DB, names []string) []User {
+ res := make([]User, 0, len(names))
+ db.Where("name IN ?", names).Find(&res)
+ return res
+}
diff --git a/ql/test/experimental/CWE-400/go.mod b/ql/test/experimental/CWE-400/go.mod
new file mode 100644
index 00000000000..8fa98bf285f
--- /dev/null
+++ b/ql/test/experimental/CWE-400/go.mod
@@ -0,0 +1,5 @@
+module query-tests/databasecallinloop
+
+go 1.16
+
+require gorm.io/gorm v1.21.12
diff --git a/ql/test/experimental/CWE-400/test.go b/ql/test/experimental/CWE-400/test.go
new file mode 100644
index 00000000000..725fb541b38
--- /dev/null
+++ b/ql/test/experimental/CWE-400/test.go
@@ -0,0 +1,27 @@
+package main
+
+import "gorm.io/gorm"
+
+type User struct {
+ Id int64
+ Name string
+}
+
+func runQuery(db *gorm.DB) {
+ db.Take(nil)
+}
+
+func runRunQuery(db *gorm.DB) {
+ runQuery(db)
+}
+
+func main() {
+ var db *gorm.DB
+ for i := 0; i < 10; i++ {
+ runQuery(db)
+ }
+
+ for i := 10; i > 0; i-- {
+ runRunQuery(db)
+ }
+}
diff --git a/ql/test/experimental/CWE-400/vendor/gorm.io/gorm/License b/ql/test/experimental/CWE-400/vendor/gorm.io/gorm/License
new file mode 100644
index 00000000000..037e1653e69
--- /dev/null
+++ b/ql/test/experimental/CWE-400/vendor/gorm.io/gorm/License
@@ -0,0 +1,21 @@
+The MIT License (MIT)
+
+Copyright (c) 2013-NOW Jinzhu