Convert sqlx sql-injection sinks to MaD

Author: owen-mc

go/ql/lib/ext/github.com.jmoiron.sqlx.model.yml

@@ -0,0 +1,17 @@
+extensions:
+  - addsTo:
+      pack: codeql/go-all
+      extensible: sinkModel
+    data:
+      - ["github.com/jmoiron/sqlx", "DB", True, "Get", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["github.com/jmoiron/sqlx", "DB", True, "MustExec", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["github.com/jmoiron/sqlx", "DB", True, "NamedExec", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["github.com/jmoiron/sqlx", "DB", True, "NamedQuery", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["github.com/jmoiron/sqlx", "DB", True, "Queryx", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["github.com/jmoiron/sqlx", "DB", True, "Select", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["github.com/jmoiron/sqlx", "Tx", True, "Get", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["github.com/jmoiron/sqlx", "Tx", True, "MustExec", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["github.com/jmoiron/sqlx", "Tx", True, "NamedExec", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["github.com/jmoiron/sqlx", "Tx", True, "NamedQuery", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["github.com/jmoiron/sqlx", "Tx", True, "Queryx", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["github.com/jmoiron/sqlx", "Tx", True, "Select", "", "", "Argument[1]", "sql-injection", "manual"]

go/ql/lib/semmle/go/frameworks/SQL.qll

@@ -165,20 +165,6 @@ module SQL {
       )
     }
   }
-
-  /** A model for sinks of github.com/jmoiron/sqlx. */
-  private class SqlxSink extends SQL::QueryString::Range {
-    SqlxSink() {
-      exists(Method meth, string name, int n |
-        meth.hasQualifiedName(package("github.com/jmoiron/sqlx", ""), ["DB", "Tx"], name) and
-        this = meth.getACall().getArgument(n)
-      |
-        name = ["Select", "Get"] and n = 1
-        or
-        name = ["MustExec", "Queryx", "NamedExec", "NamedQuery"] and n = 0
-      )
-    }
-  }
 }
 
 /**

go/ql/test/library-tests/semmle/go/frameworks/SQL/Sqlx/QueryString.expected

@@ -0,0 +1,3 @@
+testFailures
+invalidModelRow
+failures

go/ql/test/library-tests/semmle/go/frameworks/SQL/Sqlx/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>>

go/ql/test/library-tests/semmle/go/frameworks/SQL/Sqlx/sqlx.expected

@@ -12,19 +12,19 @@ func main() {
 
 	db := sqlx.DB{}
 	untrusted := getUntrustedString()
-	db.Select(nil, untrusted)
-	db.Get(nil, untrusted)
-	db.MustExec(untrusted)
-	db.Queryx(untrusted)
-	db.NamedExec(untrusted, nil)
-	db.NamedQuery(untrusted, nil)
+	db.Select(nil, untrusted)     // $ querystring=untrusted
+	db.Get(nil, untrusted)        // $ querystring=untrusted
+	db.MustExec(untrusted)        // $ querystring=untrusted
+	db.Queryx(untrusted)          // $ querystring=untrusted
+	db.NamedExec(untrusted, nil)  // $ querystring=untrusted
+	db.NamedQuery(untrusted, nil) // $ querystring=untrusted
 
 	tx := sqlx.Tx{}
-	tx.Select(nil, untrusted)
-	tx.Get(nil, untrusted)
-	tx.MustExec(untrusted)
-	tx.Queryx(untrusted)
-	tx.NamedExec(untrusted, nil)
-	tx.NamedQuery(untrusted, nil)
+	tx.Select(nil, untrusted)     // $ querystring=untrusted
+	tx.Get(nil, untrusted)        // $ querystring=untrusted
+	tx.MustExec(untrusted)        // $ querystring=untrusted
+	tx.Queryx(untrusted)          // $ querystring=untrusted
+	tx.NamedExec(untrusted, nil)  // $ querystring=untrusted
+	tx.NamedQuery(untrusted, nil) // $ querystring=untrusted
 
 }