Skip to content

Commit 2c518c1

Browse files
author
Porcupiney Hairs
committed
Include changes from review
1 parent ec424d7 commit 2c518c1

File tree

3 files changed

+38
-4
lines changed

3 files changed

+38
-4
lines changed

go/ql/src/experimental/CWE-134/DsnInjectionCustomizations.qll

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ private class DecodeFunctionModel extends TaintTracking::FunctionModel {
2727
DecodeFunctionModel() {
2828
// This matches any function with a name like `Decode`,`Unmarshal` or `Parse`.
2929
// This is done to allow taints stored in encoded forms, such as in toml or json to flow freely.
30-
this.getName().matches("(?i).*(parse|decode|unmarshal).*")
30+
this.getName().regexpMatch("(?i).*(parse|decode|unmarshal).*")
3131
}
3232

3333
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -38,9 +38,6 @@ private class DecodeFunctionModel extends TaintTracking::FunctionModel {
3838

3939
/** A model of `flag.Parse`, propagating tainted input passed via CLI flags to `Parse`'s result. */
4040
private class FlagSetFunctionModel extends TaintTracking::FunctionModel {
41-
FunctionInput inp;
42-
FunctionOutput outp;
43-
4441
FlagSetFunctionModel() { this.hasQualifiedName("flag", "Parse") }
4542

4643
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {

go/ql/test/experimental/CWE-134/Dsn.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,24 @@ func bad2(w http.ResponseWriter, req *http.Request) interface{} {
5151
return db
5252
}
5353

54+
type Config struct {
55+
dsn string
56+
}
57+
58+
func NewConfig() *Config { return &Config{dsn: ""} }
59+
func (Config) Parse([]string) error { return nil }
60+
61+
func RegexFuncModelTest(w http.ResponseWriter, req *http.Request) (interface{}, error) {
62+
cfg := NewConfig()
63+
err := cfg.Parse(os.Args[1:]) // This is bad. `name` can be something like `test?allowAllFiles=true&` which will allow an attacker to access local files.
64+
if err != nil {
65+
return nil, err
66+
}
67+
dbDSN := fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?charset=utf8", "username", "password", "127.0.0.1", 3306, cfg.dsn)
68+
db, _ := sql.Open("mysql", dbDSN)
69+
return db, nil
70+
}
71+
5472
func main() {
5573
bad2(nil, nil)
5674
good()
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,27 @@
11
edges
22
| Dsn.go:26:11:26:17 | selection of Args | Dsn.go:29:29:29:33 | dbDSN |
3+
| Dsn.go:62:2:62:4 | definition of cfg [pointer] | Dsn.go:63:9:63:11 | cfg [pointer] |
4+
| Dsn.go:62:2:62:4 | definition of cfg [pointer] | Dsn.go:67:102:67:104 | cfg [pointer] |
5+
| Dsn.go:63:9:63:11 | cfg [pointer] | Dsn.go:63:9:63:11 | implicit dereference |
6+
| Dsn.go:63:9:63:11 | implicit dereference | Dsn.go:62:2:62:4 | definition of cfg [pointer] |
7+
| Dsn.go:63:9:63:11 | implicit dereference | Dsn.go:63:9:63:11 | implicit dereference |
8+
| Dsn.go:63:9:63:11 | implicit dereference | Dsn.go:68:29:68:33 | dbDSN |
9+
| Dsn.go:63:19:63:25 | selection of Args | Dsn.go:63:9:63:11 | implicit dereference |
10+
| Dsn.go:63:19:63:25 | selection of Args | Dsn.go:68:29:68:33 | dbDSN |
11+
| Dsn.go:67:102:67:104 | cfg [pointer] | Dsn.go:67:102:67:104 | implicit dereference |
12+
| Dsn.go:67:102:67:104 | implicit dereference | Dsn.go:63:9:63:11 | implicit dereference |
13+
| Dsn.go:67:102:67:104 | implicit dereference | Dsn.go:68:29:68:33 | dbDSN |
314
nodes
415
| Dsn.go:26:11:26:17 | selection of Args | semmle.label | selection of Args |
516
| Dsn.go:29:29:29:33 | dbDSN | semmle.label | dbDSN |
17+
| Dsn.go:62:2:62:4 | definition of cfg [pointer] | semmle.label | definition of cfg [pointer] |
18+
| Dsn.go:63:9:63:11 | cfg [pointer] | semmle.label | cfg [pointer] |
19+
| Dsn.go:63:9:63:11 | implicit dereference | semmle.label | implicit dereference |
20+
| Dsn.go:63:19:63:25 | selection of Args | semmle.label | selection of Args |
21+
| Dsn.go:67:102:67:104 | cfg [pointer] | semmle.label | cfg [pointer] |
22+
| Dsn.go:67:102:67:104 | implicit dereference | semmle.label | implicit dereference |
23+
| Dsn.go:68:29:68:33 | dbDSN | semmle.label | dbDSN |
624
subpaths
725
#select
826
| Dsn.go:29:29:29:33 | dbDSN | Dsn.go:26:11:26:17 | selection of Args | Dsn.go:29:29:29:33 | dbDSN | This query depends on a $@. | Dsn.go:26:11:26:17 | selection of Args | user-provided value |
27+
| Dsn.go:68:29:68:33 | dbDSN | Dsn.go:63:19:63:25 | selection of Args | Dsn.go:68:29:68:33 | dbDSN | This query depends on a $@. | Dsn.go:63:19:63:25 | selection of Args | user-provided value |

0 commit comments

Comments
 (0)