Skip to content

Commit fa07f16

Browse files
committed
Revert "Convert database/sql sql-injection sinks to MaD"
This reverts commit 501bb3e.
1 parent b3326ba commit fa07f16

File tree

3 files changed

+23
-34
lines changed

3 files changed

+23
-34
lines changed

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

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,4 @@
11
extensions:
2-
- addsTo:
3-
pack: codeql/go-all
4-
extensible: sinkModel
5-
data:
6-
- ["database/sql", "Conn", False, "Exec", "", "", "Argument[0]", "sql-injection", "manual"]
7-
- ["database/sql", "Conn", False, "ExecContext", "", "", "Argument[1]", "sql-injection", "manual"]
8-
- ["database/sql", "Conn", False, "Prepare", "", "", "Argument[0]", "sql-injection", "manual"]
9-
- ["database/sql", "Conn", False, "PrepareContext", "", "", "Argument[1]", "sql-injection", "manual"]
10-
- ["database/sql", "Conn", False, "Query", "", "", "Argument[0]", "sql-injection", "manual"]
11-
- ["database/sql", "Conn", False, "QueryContext", "", "", "Argument[1]", "sql-injection", "manual"]
12-
- ["database/sql", "Conn", False, "QueryRow", "", "", "Argument[0]", "sql-injection", "manual"]
13-
- ["database/sql", "Conn", False, "QueryRowContext", "", "", "Argument[1]", "sql-injection", "manual"]
14-
- ["database/sql", "DB", False, "Exec", "", "", "Argument[0]", "sql-injection", "manual"]
15-
- ["database/sql", "DB", False, "ExecContext", "", "", "Argument[1]", "sql-injection", "manual"]
16-
- ["database/sql", "DB", False, "Prepare", "", "", "Argument[0]", "sql-injection", "manual"]
17-
- ["database/sql", "DB", False, "PrepareContext", "", "", "Argument[1]", "sql-injection", "manual"]
18-
- ["database/sql", "DB", False, "Query", "", "", "Argument[0]", "sql-injection", "manual"]
19-
- ["database/sql", "DB", False, "QueryContext", "", "", "Argument[1]", "sql-injection", "manual"]
20-
- ["database/sql", "DB", False, "QueryRow", "", "", "Argument[0]", "sql-injection", "manual"]
21-
- ["database/sql", "DB", False, "QueryRowContext", "", "", "Argument[1]", "sql-injection", "manual"]
22-
- ["database/sql", "Tx", False, "Exec", "", "", "Argument[0]", "sql-injection", "manual"]
23-
- ["database/sql", "Tx", False, "ExecContext", "", "", "Argument[1]", "sql-injection", "manual"]
24-
- ["database/sql", "Tx", False, "Prepare", "", "", "Argument[0]", "sql-injection", "manual"]
25-
- ["database/sql", "Tx", False, "PrepareContext", "", "", "Argument[1]", "sql-injection", "manual"]
26-
- ["database/sql", "Tx", False, "Query", "", "", "Argument[0]", "sql-injection", "manual"]
27-
- ["database/sql", "Tx", False, "QueryContext", "", "", "Argument[1]", "sql-injection", "manual"]
28-
- ["database/sql", "Tx", False, "QueryRow", "", "", "Argument[0]", "sql-injection", "manual"]
29-
- ["database/sql", "Tx", False, "QueryRowContext", "", "", "Argument[1]", "sql-injection", "manual"]
302
- addsTo:
313
pack: codeql/go-all
324
extensible: summaryModel

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,32 @@ module DatabaseSql {
2626
override DataFlow::Node getAResult() { result = this.getResult(0) }
2727

2828
override SQL::QueryString getAQueryString() {
29-
result = this.getASyntacticArgument()
29+
result = this.getAnArgument()
3030
or
3131
// attempt to resolve a `QueryString` for `Stmt`s using local data flow.
3232
t = "Stmt" and
3333
result = this.getReceiver().getAPredecessor*().(DataFlow::MethodCallNode).getAnArgument()
3434
}
3535
}
3636

37+
/** A query string used in an API function of the `database/sql` package. */
38+
private class QueryString extends SQL::QueryString::Range {
39+
QueryString() {
40+
exists(Method meth, string base, string t, string m, int n |
41+
t = ["DB", "Tx", "Conn"] and
42+
meth.hasQualifiedName("database/sql", t, m) and
43+
this = meth.getACall().getArgument(n)
44+
|
45+
base = ["Exec", "Prepare", "Query", "QueryRow"] and
46+
(
47+
m = base and n = 0
48+
or
49+
m = base + "Context" and n = 1
50+
)
51+
)
52+
}
53+
}
54+
3755
/** A query in the standard `database/sql/driver` package. */
3856
private class DriverQuery extends SQL::Query::Range, DataFlow::MethodCallNode {
3957
DriverQuery() {

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
#select
22
| 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 |
33
edges
4-
| test.go:56:2:56:42 | ... := ...[0] | test.go:57:29:57:40 | selection of Value | provenance | Src:MaD:3 |
5-
| test.go:57:29:57:40 | selection of Value | test.go:57:11:57:41 | call to EscapeString | provenance | MaD:2 Sink:MaD:1 |
4+
| test.go:56:2:56:42 | ... := ...[0] | test.go:57:29:57:40 | selection of Value | provenance | Src:MaD:2 |
5+
| test.go:57:29:57:40 | selection of Value | test.go:57:11:57:41 | call to EscapeString | provenance | MaD:1 |
66
models
7-
| 1 | Sink: database/sql; DB; false; Query; ; ; Argument[0]; sql-injection; manual |
8-
| 2 | Summary: golang.org/x/net/html; ; false; EscapeString; ; ; Argument[0]; ReturnValue; taint; manual |
9-
| 3 | Source: net/http; Request; true; Cookie; ; ; ReturnValue[0]; remote; manual |
7+
| 1 | Summary: golang.org/x/net/html; ; false; EscapeString; ; ; Argument[0]; ReturnValue; taint; manual |
8+
| 2 | Source: net/http; Request; true; Cookie; ; ; ReturnValue[0]; remote; manual |
109
nodes
1110
| test.go:56:2:56:42 | ... := ...[0] | semmle.label | ... := ...[0] |
1211
| test.go:57:11:57:41 | call to EscapeString | semmle.label | call to EscapeString |

0 commit comments

Comments
 (0)