From 8093d5778105097424934ab50359e2c235ff45d5 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Fri, 3 Jan 2025 12:48:08 -0500 Subject: [PATCH 1/8] `database/sql` and `database/sql/driver` source models --- go/ql/lib/ext/database.sql.driver.model.yml | 8 ++++++++ go/ql/lib/ext/database.sql.model.yml | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/go/ql/lib/ext/database.sql.driver.model.yml b/go/ql/lib/ext/database.sql.driver.model.yml index 10cfba9388a6..f4e46eaa3434 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 diff --git a/go/ql/lib/ext/database.sql.model.yml b/go/ql/lib/ext/database.sql.model.yml index 5854ced527d5..8d67dd921423 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 From 80ad349a481317248da1989f9bbfda7822fdaf39 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Fri, 3 Jan 2025 12:49:07 -0500 Subject: [PATCH 2/8] `database/sql` summary models for `Row` types --- go/ql/lib/ext/database.sql.driver.model.yml | 1 + go/ql/lib/ext/database.sql.model.yml | 2 ++ .../go/frameworks/stdlib/DatabaseSql.qll | 20 ------------------- 3 files changed, 3 insertions(+), 20 deletions(-) diff --git a/go/ql/lib/ext/database.sql.driver.model.yml b/go/ql/lib/ext/database.sql.driver.model.yml index f4e46eaa3434..0f33a6e14b8c 100644 --- a/go/ql/lib/ext/database.sql.driver.model.yml +++ b/go/ql/lib/ext/database.sql.driver.model.yml @@ -23,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 8d67dd921423..34042a397744 100644 --- a/go/ql/lib/ext/database.sql.model.yml +++ b/go/ql/lib/ext/database.sql.model.yml @@ -53,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 - } - } } From e9fdc8a34cd894035362cae278aa289f7b517daa Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Fri, 3 Jan 2025 21:52:26 -0500 Subject: [PATCH 3/8] database source tests --- .../flowsources/local/database/sink.go | 5 +++++ .../local/database/source.expected | 2 ++ .../flowsources/local/database/source.ext.yml | 6 ++++++ .../flowsources/local/database/source.ql | 19 +++++++++++++++++++ .../flowsources/local/database/test.expected | 2 ++ .../flowsources/local/database/test.ext.yml | 7 +++++++ .../flowsources/local/database/test.ql | 15 +++++++++++++++ 7 files changed, 56 insertions(+) create mode 100644 go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/sink.go create mode 100644 go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/source.expected create mode 100644 go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/source.ext.yml create mode 100644 go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/source.ql create mode 100644 go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test.expected create mode 100644 go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test.ext.yml create mode 100644 go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test.ql 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 From 3e65c8de36e7e4f220418985077d47de0eb8b379 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Fri, 3 Jan 2025 22:07:33 -0500 Subject: [PATCH 4/8] `database/sql` tests --- .../local/database/test_database_sql.go | 134 ++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test_database_sql.go 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..20ee84288005 --- /dev/null +++ b/go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test_database_sql.go @@ -0,0 +1,134 @@ +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) + + 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) + + 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) + + 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) + + 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) + + 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) + + sink(id, name) // $ hasTaintFlow="id" hasTaintFlow="name" + + dog := tx.QueryRow("SELECT * FROM dogs WHERE id = 1") // $ source + ignore(dog) +} From 128c02b488882a63d5adeb0872c40655b3cf3f55 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Fri, 3 Jan 2025 22:08:08 -0500 Subject: [PATCH 5/8] `database/sql/driver` tests --- .../database/test_database_sql_driver.go | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/test_database_sql_driver.go 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) +} From 0f06ddcff0b7fcf3c9ea62780763dd3643b0fe75 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Fri, 3 Jan 2025 22:12:22 -0500 Subject: [PATCH 6/8] Change note --- ...1-03-database-sql-and-database-sql-driver-source-models.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 go/ql/lib/change-notes/2025-01-03-database-sql-and-database-sql-driver-source-models.md 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. From 60cf1eccae2d94e268899c304bbb0b579bf19ac2 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Sun, 5 Jan 2025 22:22:18 -0500 Subject: [PATCH 7/8] Update test results --- .../Security/CWE-078/StoredCommand.expected | 10 +++++++-- .../Security/CWE-079/StoredXss.expected | 21 ++++++++++++------- 2 files changed, 22 insertions(+), 9 deletions(-) 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 | From b3d8c6b2e8a6edece26f2f6efd3053b140b0f130 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Tue, 7 Jan 2025 06:46:54 -0500 Subject: [PATCH 8/8] Add error handling to test --- .../local/database/test_database_sql.go | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) 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 index 20ee84288005..beb8b14f9bd3 100644 --- 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 @@ -18,6 +18,9 @@ func testConnQuery(conn *sql.Conn) { var id int var name string err = rows.Scan(&id, &name) + if err != nil { + return + } sink(id, name) // $ hasTaintFlow="id" hasTaintFlow="name" } @@ -50,6 +53,10 @@ func testDBQuery(db *sql.DB) { var name string err = rows.Scan(&id, &name) + if err != nil { + return + } + sink(id, name) // $ hasTaintFlow="id" hasTaintFlow="name" } @@ -84,6 +91,10 @@ func testStmtQuery(stmt *sql.Stmt) { var name string err = rows.Scan(&id, &name) + if err != nil { + return + } + sink(id, name) // $ hasTaintFlow="id" hasTaintFlow="name" } @@ -94,6 +105,10 @@ func testStmtQuery(stmt *sql.Stmt) { 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 @@ -117,6 +132,10 @@ func testTxQuery(tx *sql.Tx) { var name string err = rows.Scan(&id, &name) + if err != nil { + return + } + sink(id, name) // $ hasTaintFlow="id" hasTaintFlow="name" } @@ -127,6 +146,10 @@ func testTxQuery(tx *sql.Tx) { 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