Convert Gorm sql-injection sinks to MaD

2026-04-20 06:24:03 +02:00 · 2024-08-15 15:24:56 +01:00
parent 7ad63fc3e6
commit ba310417a8
7 changed files with 113 additions and 71 deletions
--- a/go/ql/lib/ext/gorm.io.gorm.model.yml
+++ b/go/ql/lib/ext/gorm.io.gorm.model.yml
@@ -0,0 +1,25 @@
+extensions:
+  - addsTo:
+      pack: codeql/go-all
+      extensible: packageGrouping
+    data:
+      - ["gorm", "gorm.io/gorm"]
+      - ["gorm", "github.com/jinzhu/gorm"]
+      - ["gorm", "github.com/go-gorm/gorm"]
+  - addsTo:
+      pack: codeql/go-all
+      extensible: sinkModel
+    data:
+      - ["group:gorm", "DB", True, "Where", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["group:gorm", "DB", True, "Raw", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["group:gorm", "DB", True, "Order", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["group:gorm", "DB", True, "Not", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["group:gorm", "DB", True, "Or", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["group:gorm", "DB", True, "Select", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["group:gorm", "DB", True, "Table", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["group:gorm", "DB", True, "Group", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["group:gorm", "DB", True, "Having", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["group:gorm", "DB", True, "Joins", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["group:gorm", "DB", True, "Exec", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["group:gorm", "DB", True, "Distinct", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["group:gorm", "DB", True, "Pluck", "", "", "Argument[0]", "sql-injection", "manual"]
--- a/go/ql/lib/semmle/go/frameworks/SQL.qll
+++ b/go/ql/lib/semmle/go/frameworks/SQL.qll
@@ -150,21 +150,6 @@ module SQL {
      }
    }
  }
-
-  /** A model for sinks of GORM. */
-  private class GormSink extends SQL::QueryString::Range {
-    GormSink() {
-      exists(Method meth, string package, string name |
-        meth.hasQualifiedName(package, "DB", name) and
-        this = meth.getACall().getSyntacticArgument(0) and
-        package = Gorm::packagePath() and
-        name in [
-            "Where", "Raw", "Order", "Not", "Or", "Select", "Table", "Group", "Having", "Joins",
-            "Exec", "Distinct", "Pluck"
-          ]
-      )
-    }
-  }
 }

 /**
--- a/go/ql/test/library-tests/semmle/go/frameworks/SQL/Gorm/QueryString.expected
+++ b/go/ql/test/library-tests/semmle/go/frameworks/SQL/Gorm/QueryString.expected
@@ -0,0 +1,3 @@
+testFailures
+invalidModelRow
+failures
--- a/go/ql/test/library-tests/semmle/go/frameworks/SQL/Gorm/QueryString.ql
+++ b/go/ql/test/library-tests/semmle/go/frameworks/SQL/Gorm/QueryString.ql
@@ -0,0 +1,60 @@
+import go
+import semmle.go.dataflow.ExternalFlow
+import ModelValidation
+import TestUtilities.InlineExpectationsTest
+
+module SqlTest implements TestSig {
+  string getARelevantTag() { result = "query" }
+
+  predicate hasActualResult(Location location, string element, string tag, string value) {
+    tag = "query" and
+    exists(SQL::Query q, SQL::QueryString qs | qs = q.getAQueryString() |
+      q.hasLocationInfo(location.getFile().getAbsolutePath(), location.getStartLine(),
+        location.getStartColumn(), location.getEndLine(), location.getEndColumn()) and
+      element = q.toString() and
+      value = qs.toString()
+    )
+  }
+}
+
+module QueryString implements TestSig {
+  string getARelevantTag() { result = "querystring" }
+
+  predicate hasActualResult(Location location, string element, string tag, string value) {
+    tag = "querystring" and
+    element = "" and
+    exists(SQL::QueryString qs | not exists(SQL::Query q | qs = q.getAQueryString()) |
+      qs.hasLocationInfo(location.getFile().getAbsolutePath(), location.getStartLine(),
+        location.getStartColumn(), location.getEndLine(), location.getEndColumn()) and
+      value = qs.toString()
+    )
+  }
+}
+
+module Config implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node n) { n.asExpr() instanceof StringLit }
+
+  predicate isSink(DataFlow::Node n) {
+    n = any(DataFlow::CallNode cn | cn.getTarget().getName() = "sink").getAnArgument()
+  }
+}
+
+module Flow = TaintTracking::Global<Config>;
+
+module TaintFlow implements TestSig {
+  string getARelevantTag() { result = "flowfrom" }
+
+  predicate hasActualResult(Location location, string element, string tag, string value) {
+    tag = "flowfrom" and
+    element = "" and
+    exists(DataFlow::Node fromNode, DataFlow::Node toNode |
+      toNode
+          .hasLocationInfo(location.getFile().getAbsolutePath(), location.getStartLine(),
+            location.getStartColumn(), location.getEndLine(), location.getEndColumn()) and
+      Flow::flow(fromNode, toNode) and
+      value = fromNode.asExpr().(StringLit).getValue()
+    )
+  }
+}
+
+import MakeTest<MergeTests3<SqlTest, QueryString, TaintFlow>>
--- a/go/ql/test/library-tests/semmle/go/frameworks/SQL/Gorm/gorm.expected
+++ b/go/ql/test/library-tests/semmle/go/frameworks/SQL/Gorm/gorm.expected
@@ -1,25 +0,0 @@
-| gorm.go:20:12:20:20 | untrusted | github.com/jinzhu/gorm | DB | Where |
-| gorm.go:21:10:21:18 | untrusted | github.com/jinzhu/gorm | DB | Raw |
-| gorm.go:22:10:22:18 | untrusted | github.com/jinzhu/gorm | DB | Not |
-| gorm.go:23:12:23:20 | untrusted | github.com/jinzhu/gorm | DB | Order |
-| gorm.go:24:9:24:17 | untrusted | github.com/jinzhu/gorm | DB | Or |
-| gorm.go:25:13:25:21 | untrusted | github.com/jinzhu/gorm | DB | Select |
-| gorm.go:26:12:26:20 | untrusted | github.com/jinzhu/gorm | DB | Table |
-| gorm.go:27:12:27:20 | untrusted | github.com/jinzhu/gorm | DB | Group |
-| gorm.go:28:13:28:21 | untrusted | github.com/jinzhu/gorm | DB | Having |
-| gorm.go:29:12:29:20 | untrusted | github.com/jinzhu/gorm | DB | Joins |
-| gorm.go:30:11:30:19 | untrusted | github.com/jinzhu/gorm | DB | Exec |
-| gorm.go:31:12:31:20 | untrusted | github.com/jinzhu/gorm | DB | Pluck |
-| gorm.go:34:12:34:20 | untrusted | gorm.io/gorm | DB | Where |
-| gorm.go:35:10:35:18 | untrusted | gorm.io/gorm | DB | Raw |
-| gorm.go:36:10:36:18 | untrusted | gorm.io/gorm | DB | Not |
-| gorm.go:37:12:37:20 | untrusted | gorm.io/gorm | DB | Order |
-| gorm.go:38:9:38:17 | untrusted | gorm.io/gorm | DB | Or |
-| gorm.go:39:13:39:21 | untrusted | gorm.io/gorm | DB | Select |
-| gorm.go:40:12:40:20 | untrusted | gorm.io/gorm | DB | Table |
-| gorm.go:41:12:41:20 | untrusted | gorm.io/gorm | DB | Group |
-| gorm.go:42:13:42:21 | untrusted | gorm.io/gorm | DB | Having |
-| gorm.go:43:12:43:20 | untrusted | gorm.io/gorm | DB | Joins |
-| gorm.go:44:11:44:19 | untrusted | gorm.io/gorm | DB | Exec |
-| gorm.go:45:15:45:23 | untrusted | gorm.io/gorm | DB | Distinct |
-| gorm.go:46:12:46:20 | untrusted | gorm.io/gorm | DB | Pluck |
--- a/go/ql/test/library-tests/semmle/go/frameworks/SQL/Gorm/gorm.go
+++ b/go/ql/test/library-tests/semmle/go/frameworks/SQL/Gorm/gorm.go
@@ -13,36 +13,35 @@ func getUntrustedString() string {
 }

 func main() {
-
 	untrusted := getUntrustedString()

 	db1 := gorm1.DB{}
-	db1.Where(untrusted)
-	db1.Raw(untrusted)
-	db1.Not(untrusted)
-	db1.Order(untrusted)
-	db1.Or(untrusted)
-	db1.Select(untrusted)
-	db1.Table(untrusted)
-	db1.Group(untrusted)
-	db1.Having(untrusted)
-	db1.Joins(untrusted)
-	db1.Exec(untrusted)
-	db1.Pluck(untrusted, nil)
+	db1.Where(untrusted)      // $ querystring=untrusted
+	db1.Raw(untrusted)        // $ querystring=untrusted
+	db1.Not(untrusted)        // $ querystring=untrusted
+	db1.Order(untrusted)      // $ querystring=untrusted
+	db1.Or(untrusted)         // $ querystring=untrusted
+	db1.Select(untrusted)     // $ querystring=untrusted
+	db1.Table(untrusted)      // $ querystring=untrusted
+	db1.Group(untrusted)      // $ querystring=untrusted
+	db1.Having(untrusted)     // $ querystring=untrusted
+	db1.Joins(untrusted)      // $ querystring=untrusted
+	db1.Exec(untrusted)       // $ querystring=untrusted
+	db1.Pluck(untrusted, nil) // $ querystring=untrusted

 	db2 := gorm2.DB{}
-	db2.Where(untrusted)
-	db2.Raw(untrusted)
-	db2.Not(untrusted)
-	db2.Order(untrusted)
-	db2.Or(untrusted)
-	db2.Select(untrusted)
-	db2.Table(untrusted)
-	db2.Group(untrusted)
-	db2.Having(untrusted)
-	db2.Joins(untrusted)
-	db2.Exec(untrusted)
-	db2.Distinct(untrusted)
-	db2.Pluck(untrusted, nil)
+	db2.Where(untrusted)      // $ querystring=untrusted
+	db2.Raw(untrusted)        // $ querystring=untrusted
+	db2.Not(untrusted)        // $ querystring=untrusted
+	db2.Order(untrusted)      // $ querystring=untrusted
+	db2.Or(untrusted)         // $ querystring=untrusted
+	db2.Select(untrusted)     // $ querystring=untrusted
+	db2.Table(untrusted)      // $ querystring=untrusted
+	db2.Group(untrusted)      // $ querystring=untrusted
+	db2.Having(untrusted)     // $ querystring=untrusted
+	db2.Joins(untrusted)      // $ querystring=untrusted
+	db2.Exec(untrusted)       // $ querystring=untrusted
+	db2.Distinct(untrusted)   // $ querystring=untrusted
+	db2.Pluck(untrusted, nil) // $ querystring=untrusted

 }
--- a/go/ql/test/library-tests/semmle/go/frameworks/SQL/Gorm/gorm.ql
+++ b/go/ql/test/library-tests/semmle/go/frameworks/SQL/Gorm/gorm.ql
@@ -1,5 +0,0 @@
-import go
-
-from SQL::QueryString qs, Method meth, string a, string b, string c
-where meth.hasQualifiedName(a, b, c) and qs = meth.getACall().getSyntacticArgument(0)
-select qs, a, b, c