Skip to content

Commit 5cc34a1

Browse files
owen-mcowen-mc
authored
Merge pull request #18439 from egregius313/egregius313/go/mad/database-sql/revert-varargs
Go: Revert MaD models for `database/sql` to use QL instead
2 parents 3bf2416 + bc68e44 commit 5cc34a1

4 files changed

Lines changed: 23 additions & 15 deletions

File tree

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,6 @@ extensions:
5353
- ["database/sql", "Conn", True, "PrepareContext", "", "", "Argument[1]", "ReturnValue[0]", "taint", "manual"]
5454
- ["database/sql", "DB", True, "Prepare", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
5555
- ["database/sql", "DB", True, "PrepareContext", "", "", "Argument[1]", "ReturnValue[0]", "taint", "manual"]
56-
- ["database/sql", "Row", True, "Scan", "", "", "Argument[receiver]", "Argument[0].ArrayElement", "taint", "manual"]
57-
- ["database/sql", "Rows", True, "Scan", "", "", "Argument[receiver]", "Argument[0].ArrayElement", "taint", "manual"]
5856
- ["database/sql", "Scanner", True, "Scan", "", "", "Argument[0]", "Argument[receiver]", "taint", "manual"]
5957
- ["database/sql", "Tx", True, "Prepare", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
6058
- ["database/sql", "Tx", True, "PrepareContext", "", "", "Argument[1]", "ReturnValue[0]", "taint", "manual"]

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,24 @@ module DatabaseSql {
6666
result = this.getReceiver().getAPredecessor*().(DataFlow::MethodCallNode).getAnArgument()
6767
}
6868
}
69+
70+
// These are expressed using TaintTracking::FunctionModel because varargs functions don't work with Models-as-Data sumamries yet.
71+
private class SqlMethodModels extends TaintTracking::FunctionModel, Method {
72+
FunctionInput inp;
73+
FunctionOutput outp;
74+
75+
SqlMethodModels() {
76+
// signature: func (*Row) Scan(dest ...interface{}) error
77+
this.hasQualifiedName("database/sql", "Row", "Scan") and
78+
(inp.isReceiver() and outp.isParameter(_))
79+
or
80+
// signature: func (*Rows) Scan(dest ...interface{}) error
81+
this.hasQualifiedName("database/sql", "Rows", "Scan") and
82+
(inp.isReceiver() and outp.isParameter(_))
83+
}
84+
85+
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
86+
input = inp and output = outp
87+
}
88+
}
6989
}

‎go/ql/test/query-tests/Security/CWE-078/StoredCommand.expected‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,16 @@
22
| 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 |
33
edges
44
| StoredCommand.go:11:2:11:27 | ... := ...[0] | StoredCommand.go:13:2:13:5 | rows | provenance | Src:MaD:2 |
5-
| StoredCommand.go:13:2:13:5 | rows | StoredCommand.go:13:2:13:20 | []type{args} | provenance | MaD:3 |
6-
| StoredCommand.go:13:2:13:5 | rows | StoredCommand.go:13:2:13:20 | []type{args} [array] | provenance | MaD:3 |
7-
| StoredCommand.go:13:2:13:20 | []type{args} | StoredCommand.go:13:12:13:19 | &... | provenance | |
8-
| StoredCommand.go:13:2:13:20 | []type{args} | StoredCommand.go:14:22:14:28 | cmdName | provenance | Sink:MaD:1 |
5+
| StoredCommand.go:13:2:13:5 | rows | StoredCommand.go:13:12:13:19 | &... | provenance | FunctionModel |
96
| StoredCommand.go:13:2:13:20 | []type{args} [array] | StoredCommand.go:13:12:13:19 | &... | provenance | |
107
| StoredCommand.go:13:12:13:19 | &... | StoredCommand.go:13:2:13:20 | []type{args} [array] | provenance | |
118
| StoredCommand.go:13:12:13:19 | &... | StoredCommand.go:14:22:14:28 | cmdName | provenance | Sink:MaD:1 |
129
models
1310
| 1 | Sink: os/exec; ; false; Command; ; ; Argument[0]; command-injection; manual |
1411
| 2 | Source: database/sql; DB; true; Query; ; ; ReturnValue[0]; database; manual |
15-
| 3 | Summary: database/sql; Rows; true; Scan; ; ; Argument[receiver]; Argument[0].ArrayElement; taint; manual |
1612
nodes
1713
| StoredCommand.go:11:2:11:27 | ... := ...[0] | semmle.label | ... := ...[0] |
1814
| StoredCommand.go:13:2:13:5 | rows | semmle.label | rows |
19-
| StoredCommand.go:13:2:13:20 | []type{args} | semmle.label | []type{args} |
2015
| StoredCommand.go:13:2:13:20 | []type{args} [array] | semmle.label | []type{args} [array] |
2116
| StoredCommand.go:13:12:13:19 | &... | semmle.label | &... |
2217
| StoredCommand.go:14:22:14:28 | cmdName | semmle.label | cmdName |

‎go/ql/test/query-tests/Security/CWE-079/StoredXss.expected‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,8 @@
55
edges
66
| StoredXss.go:13:21:13:31 | call to Name | StoredXss.go:13:21:13:36 | ...+... | provenance | |
77
| stored.go:18:3:18:28 | ... := ...[0] | stored.go:25:14:25:17 | rows | provenance | Src:MaD:1 |
8-
| stored.go:25:14:25:17 | rows | stored.go:25:14:25:34 | []type{args} | provenance | MaD:2 |
9-
| stored.go:25:14:25:17 | rows | stored.go:25:14:25:34 | []type{args} [array] | provenance | MaD:2 |
10-
| stored.go:25:14:25:34 | []type{args} | stored.go:25:24:25:26 | &... | provenance | |
11-
| stored.go:25:14:25:34 | []type{args} | stored.go:25:29:25:33 | &... | provenance | |
12-
| stored.go:25:14:25:34 | []type{args} | stored.go:30:22:30:25 | name | provenance | |
8+
| stored.go:25:14:25:17 | rows | stored.go:25:24:25:26 | &... | provenance | FunctionModel |
9+
| stored.go:25:14:25:17 | rows | stored.go:25:29:25:33 | &... | provenance | FunctionModel |
1310
| stored.go:25:14:25:34 | []type{args} [array] | stored.go:25:24:25:26 | &... | provenance | |
1411
| stored.go:25:14:25:34 | []type{args} [array] | stored.go:25:29:25:33 | &... | provenance | |
1512
| stored.go:25:24:25:26 | &... | stored.go:25:14:25:34 | []type{args} [array] | provenance | |
@@ -18,13 +15,11 @@ edges
1815
| stored.go:59:30:59:33 | definition of path | stored.go:61:22:61:25 | path | provenance | |
1916
models
2017
| 1 | Source: database/sql; DB; true; Query; ; ; ReturnValue[0]; database; manual |
21-
| 2 | Summary: database/sql; Rows; true; Scan; ; ; Argument[receiver]; Argument[0].ArrayElement; taint; manual |
2218
nodes
2319
| StoredXss.go:13:21:13:31 | call to Name | semmle.label | call to Name |
2420
| StoredXss.go:13:21:13:36 | ...+... | semmle.label | ...+... |
2521
| stored.go:18:3:18:28 | ... := ...[0] | semmle.label | ... := ...[0] |
2622
| stored.go:25:14:25:17 | rows | semmle.label | rows |
27-
| stored.go:25:14:25:34 | []type{args} | semmle.label | []type{args} |
2823
| stored.go:25:14:25:34 | []type{args} [array] | semmle.label | []type{args} [array] |
2924
| stored.go:25:24:25:26 | &... | semmle.label | &... |
3025
| stored.go:25:29:25:33 | &... | semmle.label | &... |

0 commit comments

Comments
 (0)