diff --git a/go/ql/lib/change-notes/2025-01-03-database-sql-and-database-sql-driver-source-models.md b/go/ql/lib/change-notes/2025-01-03-database-sql-and-database-sql-driver-source-models.md new file mode 100644 index 000000000000..3ee41e073eca --- /dev/null +++ b/go/ql/lib/change-notes/2025-01-03-database-sql-and-database-sql-driver-source-models.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* `database` local source models have been added for the `database/sql` and `database/sql/driver` packages. diff --git a/go/ql/lib/ext/database.sql.driver.model.yml b/go/ql/lib/ext/database.sql.driver.model.yml index 10cfba9388a6..0f33a6e14b8c 100644 --- a/go/ql/lib/ext/database.sql.driver.model.yml +++ b/go/ql/lib/ext/database.sql.driver.model.yml @@ -1,4 +1,12 @@ extensions: + - addsTo: + pack: codeql/go-all + extensible: sourceModel + data: + - ["database/sql/driver", "Queryer", True, "Query", "", "", "ReturnValue[0]", "database", "manual"] + - ["database/sql/driver", "QueryerContext", True, "QueryContext", "", "", "ReturnValue[0]", "database", "manual"] + - ["database/sql/driver", "Stmt", True, "Query", "", "", "ReturnValue[0]", "database", "manual"] + - ["database/sql/driver", "StmtQueryContext", True, "QueryContext", "", "", "ReturnValue[0]", "database", "manual"] - addsTo: pack: codeql/go-all extensible: sinkModel @@ -15,5 +23,6 @@ extensions: data: - ["database/sql/driver", "Conn", True, "Prepare", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"] - ["database/sql/driver", "ConnPrepareContext", True, "PrepareContext", "", "", "Argument[1]", "ReturnValue[0]", "taint", "manual"] + - ["database/sql/driver", "Rows", True, "Next", "", "", "Argument[receiver]", "Argument[0]", "taint", "manual"] - ["database/sql/driver", "ValueConverter", True, "ConvertValue", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"] - ["database/sql/driver", "Valuer", True, "Value", "", "", "Argument[receiver]", "ReturnValue[0]", "taint", "manual"] diff --git a/go/ql/lib/ext/database.sql.model.yml b/go/ql/lib/ext/database.sql.model.yml index 5854ced527d5..34042a397744 100644 --- a/go/ql/lib/ext/database.sql.model.yml +++ b/go/ql/lib/ext/database.sql.model.yml @@ -1,4 +1,22 @@ extensions: + - addsTo: + pack: codeql/go-all + extensible: sourceModel + data: + - ["database/sql", "Conn", True, "QueryContext", "", "", "ReturnValue[0]", "database", "manual"] + - ["database/sql", "Conn", True, "QueryRowContext", "", "", "ReturnValue", "database", "manual"] + - ["database/sql", "DB", True, "Query", "", "", "ReturnValue[0]", "database", "manual"] + - ["database/sql", "DB", True, "QueryContext", "", "", "ReturnValue[0]", "database", "manual"] + - ["database/sql", "DB", True, "QueryRow", "", "", "ReturnValue", "database", "manual"] + - ["database/sql", "DB", True, "QueryRowContext", "", "", "ReturnValue", "database", "manual"] + - ["database/sql", "Stmt", True, "Query", "", "", "ReturnValue[0]", "database", "manual"] + - ["database/sql", "Stmt", True, "QueryContext", "", "", "ReturnValue[0]", "database", "manual"] + - ["database/sql", "Stmt", True, "QueryRow", "", "", "ReturnValue", "database", "manual"] + - ["database/sql", "Stmt", True, "QueryRowContext", "", "", "ReturnValue", "database", "manual"] + - ["database/sql", "Tx", True, "Query", "", "", "ReturnValue[0]", "database", "manual"] + - ["database/sql", "Tx", True, "QueryContext", "", "", "ReturnValue[0]", "database", "manual"] + - ["database/sql", "Tx", True, "QueryRow", "", "", "ReturnValue", "database", "manual"] + - ["database/sql", "Tx", True, "QueryRowContext", "", "", "ReturnValue", "database", "manual"] - addsTo: pack: codeql/go-all extensible: sinkModel @@ -35,6 +53,8 @@ extensions: - ["database/sql", "Conn", True, "PrepareContext", "", "", "Argument[1]", "ReturnValue[0]", "taint", "manual"] - ["database/sql", "DB", True, "Prepare", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"] - ["database/sql", "DB", True, "PrepareContext", "", "", "Argument[1]", "ReturnValue[0]", "taint", "manual"] + - ["database/sql", "Row", True, "Scan", "", "", "Argument[receiver]", "Argument[0].ArrayElement", "taint", "manual"] + - ["database/sql", "Rows", True, "Scan", "", "", "Argument[receiver]", "Argument[0].ArrayElement", "taint", "manual"] - ["database/sql", "Scanner", True, "Scan", "", "", "Argument[0]", "Argument[receiver]", "taint", "manual"] - ["database/sql", "Tx", True, "Prepare", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"] - ["database/sql", "Tx", True, "PrepareContext", "", "", "Argument[1]", "ReturnValue[0]", "taint", "manual"] diff --git a/go/ql/lib/semmle/go/frameworks/stdlib/DatabaseSql.qll b/go/ql/lib/semmle/go/frameworks/stdlib/DatabaseSql.qll index f41326887961..14534d2fa453 100644 --- a/go/ql/lib/semmle/go/frameworks/stdlib/DatabaseSql.qll +++ b/go/ql/lib/semmle/go/frameworks/stdlib/DatabaseSql.qll @@ -66,24 +66,4 @@ module DatabaseSql { result = this.getReceiver().getAPredecessor*().(DataFlow::MethodCallNode).getAnArgument() } } - - // These are expressed using TaintTracking::FunctionModel because varargs functions don't work with Models-as-Data sumamries yet. - private class SqlMethodModels extends TaintTracking::FunctionModel, Method { - FunctionInput inp; - FunctionOutput outp; - - SqlMethodModels() { - // signature: func (*Row) Scan(dest ...interface{}) error - this.hasQualifiedName("database/sql", "Row", "Scan") and - (inp.isReceiver() and outp.isParameter(_)) - or - // signature: func (*Rows) Scan(dest ...interface{}) error - this.hasQualifiedName("database/sql", "Rows", "Scan") and - (inp.isReceiver() and outp.isParameter(_)) - } - - override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { - input = inp and output = outp - } - } } diff --git a/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/sink.go b/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/sink.go new file mode 100644 index 000000000000..f46320ae7f5f --- /dev/null +++ b/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/sink.go @@ -0,0 +1,5 @@ +package test + +func sink(x ...any) {} + +func ignore(...any) {} diff --git a/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/source.expected b/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/source.expected new file mode 100644 index 000000000000..55e9aed2e93c --- /dev/null +++ b/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/source.expected @@ -0,0 +1,2 @@ +testFailures +invalidModelRow diff --git a/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/source.ext.yml b/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/source.ext.yml new file mode 100644 index 000000000000..853b9e9a719f --- /dev/null +++ b/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/source.ext.yml @@ -0,0 +1,6 @@ +extensions: + - addsTo: + pack: codeql/threat-models + extensible: threatModelConfiguration + data: + - ["database", true, 0] \ No newline at end of file diff --git a/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/source.ql b/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/source.ql new file mode 100644 index 000000000000..924c655bf655 --- /dev/null +++ b/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/source.ql @@ -0,0 +1,19 @@ +import go +import ModelValidation +import utils.test.InlineExpectationsTest + +module SourceTest implements TestSig { + string getARelevantTag() { result = "source" } + + predicate hasActualResult(Location location, string element, string tag, string value) { + exists(ActiveThreatModelSource s | + s.hasLocationInfo(location.getFile().getAbsolutePath(), location.getStartLine(), + location.getStartColumn(), location.getEndLine(), location.getEndColumn()) and + element = s.toString() and + value = "" and + tag = "source" + ) + } +} + +import MakeTest diff --git a/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test.expected b/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test.expected new file mode 100644 index 000000000000..55e9aed2e93c --- /dev/null +++ b/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test.expected @@ -0,0 +1,2 @@ +testFailures +invalidModelRow diff --git a/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test.ext.yml b/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test.ext.yml new file mode 100644 index 000000000000..00f4b3659c37 --- /dev/null +++ b/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test.ext.yml @@ -0,0 +1,7 @@ +extensions: + + - addsTo: + pack: codeql/threat-models + extensible: threatModelConfiguration + data: + - ["database", true, 0] diff --git a/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test.ql b/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test.ql new file mode 100644 index 000000000000..eda3a6be6eef --- /dev/null +++ b/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test.ql @@ -0,0 +1,15 @@ +import go +import semmle.go.dataflow.ExternalFlow +import ModelValidation +import experimental.frameworks.CleverGo +import utils.test.InlineFlowTest + +module Config implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource } + + predicate isSink(DataFlow::Node sink) { + sink.asExpr() = any(CallExpr c | c.getTarget().getName() = "sink").getAnArgument() + } +} + +import TaintFlowTest diff --git a/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test_database_sql.go b/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test_database_sql.go new file mode 100644 index 000000000000..beb8b14f9bd3 --- /dev/null +++ b/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test_database_sql.go @@ -0,0 +1,157 @@ +package test + +import ( + "database/sql" +) + +// test querying a Conn +func testConnQuery(conn *sql.Conn) { + rows, err := conn.QueryContext(nil, "SELECT * FROM users") // $ source + + if err != nil { + return + } + + defer rows.Close() + + for rows.Next() { + var id int + var name string + err = rows.Scan(&id, &name) + if err != nil { + return + } + + sink(id, name) // $ hasTaintFlow="id" hasTaintFlow="name" + } + + row := conn.QueryRowContext(nil, "SELECT * FROM users WHERE id = 1") // $ source + + var id int + var name string + + err = row.Scan(&id, &name) + + sink(id, name) // $ hasTaintFlow="id" hasTaintFlow="name" +} + +// test querying a DB +func testDBQuery(db *sql.DB) { + example, err := db.Query("SELECT * FROM users") // $ source + ignore(example) + + rows, err := db.QueryContext(nil, "SELECT * FROM users") // $ source + + if err != nil { + return + } + + defer rows.Close() + + for rows.Next() { + var id int + var name string + err = rows.Scan(&id, &name) + + if err != nil { + return + } + + sink(id, name) // $ hasTaintFlow="id" hasTaintFlow="name" + } + + row := db.QueryRowContext(nil, "SELECT * FROM users WHERE id = 1") // $ source + + var id int + var name string + + err = row.Scan(&id, &name) + + sink(id, name) // $ hasTaintFlow="id" hasTaintFlow="name" + + dog := db.QueryRow("SELECT * FROM dogs WHERE id = 1") // $ source + ignore(dog) +} + +// test querying a Stmt +func testStmtQuery(stmt *sql.Stmt) { + example, err := stmt.Query("SELECT * FROM users") // $ source + ignore(example) + + rows, err := stmt.QueryContext(nil, "SELECT * FROM users") // $ source + + if err != nil { + return + } + + defer rows.Close() + + for rows.Next() { + var id int + var name string + err = rows.Scan(&id, &name) + + if err != nil { + return + } + + sink(id, name) // $ hasTaintFlow="id" hasTaintFlow="name" + } + + row := stmt.QueryRowContext(nil, "SELECT * FROM users WHERE id = 1") // $ source + + var id int + var name string + + err = row.Scan(&id, &name) + + if err != nil { + return + } + + sink(id, name) // $ hasTaintFlow="id" hasTaintFlow="name" + + dog := stmt.QueryRow("SELECT * FROM dogs WHERE id = 1") // $ source + ignore(dog) +} + +// test querying a Tx +func testTxQuery(tx *sql.Tx) { + example, err := tx.Query("SELECT * FROM users") // $ source + ignore(example) + + rows, err := tx.QueryContext(nil, "SELECT * FROM users") // $ source + if err != nil { + return + } + + defer rows.Close() + + for rows.Next() { + var id int + var name string + err = rows.Scan(&id, &name) + + if err != nil { + return + } + + sink(id, name) // $ hasTaintFlow="id" hasTaintFlow="name" + } + + row := tx.QueryRowContext(nil, "SELECT * FROM users WHERE id = 1") // $ source + + var id int + var name string + + err = row.Scan(&id, &name) + + if err != nil { + return + } + + sink(id, name) // $ hasTaintFlow="id" hasTaintFlow="name" + + dog := tx.QueryRow("SELECT * FROM dogs WHERE id = 1") // $ source + ignore(dog) +} diff --git a/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test_database_sql_driver.go b/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test_database_sql_driver.go new file mode 100644 index 000000000000..7838fcaf871b --- /dev/null +++ b/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test_database_sql_driver.go @@ -0,0 +1,23 @@ +package test + +import "database/sql/driver" + +func testQueryer(q driver.Queryer) { + rows, err := q.Query("SELECT * FROM users", make([]driver.Value, 0)) // $ source + ignore(rows, err) +} + +func testQueryerContext(q driver.QueryerContext) { + rows, err := q.QueryContext(nil, "SELECT * FROM users", make([]driver.NamedValue, 0)) // $ source + ignore(rows, err) +} + +func testStmt(stmt driver.Stmt) { + rows, err := stmt.Query(make([]driver.Value, 0)) // $ source + ignore(rows, err) +} + +func testStmtContext(stmt driver.StmtQueryContext) { + rows, err := stmt.QueryContext(nil, make([]driver.NamedValue, 0)) // $ source + ignore(rows, err) +} diff --git a/go/ql/test/query-tests/Security/CWE-078/StoredCommand.expected b/go/ql/test/query-tests/Security/CWE-078/StoredCommand.expected index a0b34cd05b47..223c9fd7d394 100644 --- a/go/ql/test/query-tests/Security/CWE-078/StoredCommand.expected +++ b/go/ql/test/query-tests/Security/CWE-078/StoredCommand.expected @@ -1,16 +1,22 @@ #select | StoredCommand.go:14:22:14:28 | cmdName | StoredCommand.go:11:2:11:27 | ... := ...[0] | StoredCommand.go:14:22:14:28 | cmdName | This command depends on a $@. | StoredCommand.go:11:2:11:27 | ... := ...[0] | stored value | edges -| StoredCommand.go:11:2:11:27 | ... := ...[0] | StoredCommand.go:13:2:13:5 | rows | provenance | | -| StoredCommand.go:13:2:13:5 | rows | StoredCommand.go:13:12:13:19 | &... | provenance | FunctionModel | +| StoredCommand.go:11:2:11:27 | ... := ...[0] | StoredCommand.go:13:2:13:5 | rows | provenance | Src:MaD:2 | +| StoredCommand.go:13:2:13:5 | rows | StoredCommand.go:13:2:13:20 | []type{args} | provenance | MaD:3 | +| StoredCommand.go:13:2:13:5 | rows | StoredCommand.go:13:2:13:20 | []type{args} [array] | provenance | MaD:3 | +| StoredCommand.go:13:2:13:20 | []type{args} | StoredCommand.go:13:12:13:19 | &... | provenance | | +| StoredCommand.go:13:2:13:20 | []type{args} | StoredCommand.go:14:22:14:28 | cmdName | provenance | Sink:MaD:1 | | StoredCommand.go:13:2:13:20 | []type{args} [array] | StoredCommand.go:13:12:13:19 | &... | provenance | | | StoredCommand.go:13:12:13:19 | &... | StoredCommand.go:13:2:13:20 | []type{args} [array] | provenance | | | StoredCommand.go:13:12:13:19 | &... | StoredCommand.go:14:22:14:28 | cmdName | provenance | Sink:MaD:1 | models | 1 | Sink: os/exec; ; false; Command; ; ; Argument[0]; command-injection; manual | +| 2 | Source: database/sql; DB; true; Query; ; ; ReturnValue[0]; database; manual | +| 3 | Summary: database/sql; Rows; true; Scan; ; ; Argument[receiver]; Argument[0].ArrayElement; taint; manual | nodes | StoredCommand.go:11:2:11:27 | ... := ...[0] | semmle.label | ... := ...[0] | | StoredCommand.go:13:2:13:5 | rows | semmle.label | rows | +| StoredCommand.go:13:2:13:20 | []type{args} | semmle.label | []type{args} | | StoredCommand.go:13:2:13:20 | []type{args} [array] | semmle.label | []type{args} [array] | | StoredCommand.go:13:12:13:19 | &... | semmle.label | &... | | StoredCommand.go:14:22:14:28 | cmdName | semmle.label | cmdName | diff --git a/go/ql/test/query-tests/Security/CWE-079/StoredXss.expected b/go/ql/test/query-tests/Security/CWE-079/StoredXss.expected index ebeedf3d0ef7..d9bb0c2a2e2b 100644 --- a/go/ql/test/query-tests/Security/CWE-079/StoredXss.expected +++ b/go/ql/test/query-tests/Security/CWE-079/StoredXss.expected @@ -1,19 +1,30 @@ +#select +| StoredXss.go:13:21:13:36 | ...+... | StoredXss.go:13:21:13:31 | call to Name | StoredXss.go:13:21:13:36 | ...+... | Stored cross-site scripting vulnerability due to $@. | StoredXss.go:13:21:13:31 | call to Name | stored value | +| stored.go:30:22:30:25 | name | stored.go:18:3:18:28 | ... := ...[0] | stored.go:30:22:30:25 | name | Stored cross-site scripting vulnerability due to $@. | stored.go:18:3:18:28 | ... := ...[0] | stored value | +| stored.go:61:22:61:25 | path | stored.go:59:30:59:33 | definition of path | stored.go:61:22:61:25 | path | Stored cross-site scripting vulnerability due to $@. | stored.go:59:30:59:33 | definition of path | stored value | edges | StoredXss.go:13:21:13:31 | call to Name | StoredXss.go:13:21:13:36 | ...+... | provenance | | -| stored.go:18:3:18:28 | ... := ...[0] | stored.go:25:14:25:17 | rows | provenance | | -| stored.go:25:14:25:17 | rows | stored.go:25:24:25:26 | &... | provenance | FunctionModel | -| stored.go:25:14:25:17 | rows | stored.go:25:29:25:33 | &... | provenance | FunctionModel | +| stored.go:18:3:18:28 | ... := ...[0] | stored.go:25:14:25:17 | rows | provenance | Src:MaD:1 | +| stored.go:25:14:25:17 | rows | stored.go:25:14:25:34 | []type{args} | provenance | MaD:2 | +| stored.go:25:14:25:17 | rows | stored.go:25:14:25:34 | []type{args} [array] | provenance | MaD:2 | +| stored.go:25:14:25:34 | []type{args} | stored.go:25:24:25:26 | &... | provenance | | +| stored.go:25:14:25:34 | []type{args} | stored.go:25:29:25:33 | &... | provenance | | +| stored.go:25:14:25:34 | []type{args} | stored.go:30:22:30:25 | name | provenance | | | stored.go:25:14:25:34 | []type{args} [array] | stored.go:25:24:25:26 | &... | provenance | | | stored.go:25:14:25:34 | []type{args} [array] | stored.go:25:29:25:33 | &... | provenance | | | stored.go:25:24:25:26 | &... | stored.go:25:14:25:34 | []type{args} [array] | provenance | | | stored.go:25:29:25:33 | &... | stored.go:25:14:25:34 | []type{args} [array] | provenance | | | stored.go:25:29:25:33 | &... | stored.go:30:22:30:25 | name | provenance | | | stored.go:59:30:59:33 | definition of path | stored.go:61:22:61:25 | path | provenance | | +models +| 1 | Source: database/sql; DB; true; Query; ; ; ReturnValue[0]; database; manual | +| 2 | Summary: database/sql; Rows; true; Scan; ; ; Argument[receiver]; Argument[0].ArrayElement; taint; manual | nodes | StoredXss.go:13:21:13:31 | call to Name | semmle.label | call to Name | | StoredXss.go:13:21:13:36 | ...+... | semmle.label | ...+... | | stored.go:18:3:18:28 | ... := ...[0] | semmle.label | ... := ...[0] | | stored.go:25:14:25:17 | rows | semmle.label | rows | +| stored.go:25:14:25:34 | []type{args} | semmle.label | []type{args} | | stored.go:25:14:25:34 | []type{args} [array] | semmle.label | []type{args} [array] | | stored.go:25:24:25:26 | &... | semmle.label | &... | | stored.go:25:29:25:33 | &... | semmle.label | &... | @@ -21,7 +32,3 @@ nodes | stored.go:59:30:59:33 | definition of path | semmle.label | definition of path | | stored.go:61:22:61:25 | path | semmle.label | path | subpaths -#select -| StoredXss.go:13:21:13:36 | ...+... | StoredXss.go:13:21:13:31 | call to Name | StoredXss.go:13:21:13:36 | ...+... | Stored cross-site scripting vulnerability due to $@. | StoredXss.go:13:21:13:31 | call to Name | stored value | -| stored.go:30:22:30:25 | name | stored.go:18:3:18:28 | ... := ...[0] | stored.go:30:22:30:25 | name | Stored cross-site scripting vulnerability due to $@. | stored.go:18:3:18:28 | ... := ...[0] | stored value | -| stored.go:61:22:61:25 | path | stored.go:59:30:59:33 | definition of path | stored.go:61:22:61:25 | path | Stored cross-site scripting vulnerability due to $@. | stored.go:59:30:59:33 | definition of path | stored value |