Convert database/sql sql-injection sinks to MaD

Author: owen-mc

go/ql/lib/ext/database.sql.model.yml

@@ -1,4 +1,32 @@
 extensions:
+  - addsTo:
+      pack: codeql/go-all
+      extensible: sinkModel
+    data:
+      - ["database/sql", "Conn", False, "Exec", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["database/sql", "Conn", False, "ExecContext", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["database/sql", "Conn", False, "Prepare", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["database/sql", "Conn", False, "PrepareContext", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["database/sql", "Conn", False, "Query", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["database/sql", "Conn", False, "QueryContext", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["database/sql", "Conn", False, "QueryRow", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["database/sql", "Conn", False, "QueryRowContext", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["database/sql", "DB", False, "Exec", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["database/sql", "DB", False, "ExecContext", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["database/sql", "DB", False, "Prepare", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["database/sql", "DB", False, "PrepareContext", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["database/sql", "DB", False, "Query", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["database/sql", "DB", False, "QueryContext", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["database/sql", "DB", False, "QueryRow", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["database/sql", "DB", False, "QueryRowContext", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["database/sql", "Tx", False, "Exec", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["database/sql", "Tx", False, "ExecContext", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["database/sql", "Tx", False, "Prepare", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["database/sql", "Tx", False, "PrepareContext", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["database/sql", "Tx", False, "Query", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["database/sql", "Tx", False, "QueryContext", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["database/sql", "Tx", False, "QueryRow", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["database/sql", "Tx", False, "QueryRowContext", "", "", "Argument[1]", "sql-injection", "manual"]
   - addsTo:
       pack: codeql/go-all
       extensible: summaryModel

go/ql/lib/semmle/go/frameworks/stdlib/DatabaseSql.qll

@@ -26,32 +26,14 @@ module DatabaseSql {
     override DataFlow::Node getAResult() { result = this.getResult(0) }
 
     override SQL::QueryString getAQueryString() {
-      result = this.getAnArgument()
+      result = this.getASyntacticArgument()
       or
       // attempt to resolve a `QueryString` for `Stmt`s using local data flow.
       t = "Stmt" and
       result = this.getReceiver().getAPredecessor*().(DataFlow::MethodCallNode).getAnArgument()
     }
   }
 
-  /** A query string used in an API function of the `database/sql` package. */
-  private class QueryString extends SQL::QueryString::Range {
-    QueryString() {
-      exists(Method meth, string base, string t, string m, int n |
-        t = ["DB", "Tx", "Conn"] and
-        meth.hasQualifiedName("database/sql", t, m) and
-        this = meth.getACall().getArgument(n)
-      |
-        base = ["Exec", "Prepare", "Query", "QueryRow"] and
-        (
-          m = base and n = 0
-          or
-          m = base + "Context" and n = 1
-        )
-      )
-    }
-  }
-
   /** A query in the standard `database/sql/driver` package. */
   private class DriverQuery extends SQL::Query::Range, DataFlow::MethodCallNode {
     DriverQuery() {

go/ql/test/library-tests/semmle/go/frameworks/XNetHtml/SqlInjection.expected

@@ -1,11 +1,12 @@
 #select
 | test.go:57:11:57:41 | call to EscapeString | test.go:56:2:56:42 | ... := ...[0] | test.go:57:11:57:41 | call to EscapeString | This query depends on a $@. | test.go:56:2:56:42 | ... := ...[0] | user-provided value |
 edges
-| test.go:56:2:56:42 | ... := ...[0] | test.go:57:29:57:40 | selection of Value | provenance | Src:MaD:2  |
-| test.go:57:29:57:40 | selection of Value | test.go:57:11:57:41 | call to EscapeString | provenance | MaD:1 |
+| test.go:56:2:56:42 | ... := ...[0] | test.go:57:29:57:40 | selection of Value | provenance | Src:MaD:3  |
+| test.go:57:29:57:40 | selection of Value | test.go:57:11:57:41 | call to EscapeString | provenance | MaD:2 Sink:MaD:1 |
 models
-| 1 | Summary: golang.org/x/net/html; ; false; EscapeString; ; ; Argument[0]; ReturnValue; taint; manual |
-| 2 | Source: net/http; Request; true; Cookie; ; ; ReturnValue[0]; remote; manual |
+| 1 | Sink: database/sql; DB; false; Query; ; ; Argument[0]; sql-injection; manual |
+| 2 | Summary: golang.org/x/net/html; ; false; EscapeString; ; ; Argument[0]; ReturnValue; taint; manual |
+| 3 | Source: net/http; Request; true; Cookie; ; ; ReturnValue[0]; remote; manual |
 nodes
 | test.go:56:2:56:42 | ... := ...[0] | semmle.label | ... := ...[0] |
 | test.go:57:11:57:41 | call to EscapeString | semmle.label | call to EscapeString |