Skip to content

Commit d195273

Browse files
KwstubbsKwstubbs
committed
Add mux.Vars() and url.Path sanitizers
1 parent 1f1b1b7 commit d195273

File tree

8 files changed

+345
-16
lines changed

8 files changed

+345
-16
lines changed

‎go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,35 @@ module TaintedPath {
9393
}
9494
}
9595

96+
// /**
97+
// * A call to `mux.Vars(path)`, considered to sanitize `path` against path traversal.
98+
// * Only enabled when `SkipClean` is not set true.
99+
// */
100+
class MuxVarsSanitizer extends Sanitizer {
101+
MuxVarsSanitizer() {
102+
exists(Function m |
103+
m.hasQualifiedName("github.com/gorilla/mux", "Vars") and
104+
this = m.getACall().getResult()
105+
) and
106+
not exists(CallExpr f |
107+
f.getTarget().hasQualifiedName("github.com/gorilla/mux", "SkipClean") and
108+
f.getArgument(0).toString().toLowerCase() = "true"
109+
)
110+
}
111+
}
112+
113+
// /**
114+
// * A read from `net/url` which is sanitized
115+
// */
116+
class UrlPathSanitizer extends Sanitizer {
117+
UrlPathSanitizer() {
118+
exists(DataFlow::Field fld |
119+
this = fld.getARead() and
120+
fld.hasQualifiedName("net/url", "URL", "Path")
121+
)
122+
}
123+
}
124+
96125
/**
97126
* A read from the field `Filename` of the type `mime/multipart.FileHeader`,
98127
* considered as a sanitizer for path traversal.

‎go/ql/src/change-notes/2024-10-14-gopathsanitizer.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added [github.com/gorilla/mux.Vars](https://pkg.go.dev/github.com/gorilla/mux#Vars) to path sanitizers.

‎go/ql/test/query-tests/Security/CWE-022/TaintedPath.expected

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,25 @@
11
#select
2-
| TaintedPath.go:17:29:17:40 | tainted_path | TaintedPath.go:14:18:14:22 | selection of URL | TaintedPath.go:17:29:17:40 | tainted_path | This path depends on a $@. | TaintedPath.go:14:18:14:22 | selection of URL | user-provided value |
3-
| TaintedPath.go:21:28:21:69 | call to Join | TaintedPath.go:14:18:14:22 | selection of URL | TaintedPath.go:21:28:21:69 | call to Join | This path depends on a $@. | TaintedPath.go:14:18:14:22 | selection of URL | user-provided value |
4-
| TaintedPath.go:68:28:68:57 | call to Clean | TaintedPath.go:14:18:14:22 | selection of URL | TaintedPath.go:68:28:68:57 | call to Clean | This path depends on a $@. | TaintedPath.go:14:18:14:22 | selection of URL | user-provided value |
2+
| TaintedPath.go:19:29:19:40 | tainted_path | TaintedPath.go:16:18:16:22 | selection of URL | TaintedPath.go:19:29:19:40 | tainted_path | This path depends on a $@. | TaintedPath.go:16:18:16:22 | selection of URL | user-provided value |
3+
| TaintedPath.go:23:28:23:69 | call to Join | TaintedPath.go:16:18:16:22 | selection of URL | TaintedPath.go:23:28:23:69 | call to Join | This path depends on a $@. | TaintedPath.go:16:18:16:22 | selection of URL | user-provided value |
4+
| TaintedPath.go:70:28:70:57 | call to Clean | TaintedPath.go:16:18:16:22 | selection of URL | TaintedPath.go:70:28:70:57 | call to Clean | This path depends on a $@. | TaintedPath.go:16:18:16:22 | selection of URL | user-provided value |
55
edges
6-
| TaintedPath.go:14:18:14:22 | selection of URL | TaintedPath.go:14:18:14:30 | call to Query | provenance | Src:MaD:2 MaD:3 |
7-
| TaintedPath.go:14:18:14:30 | call to Query | TaintedPath.go:17:29:17:40 | tainted_path | provenance | Sink:MaD:1 |
8-
| TaintedPath.go:14:18:14:30 | call to Query | TaintedPath.go:21:57:21:68 | tainted_path | provenance | |
9-
| TaintedPath.go:14:18:14:30 | call to Query | TaintedPath.go:68:39:68:56 | ...+... | provenance | |
10-
| TaintedPath.go:21:57:21:68 | tainted_path | TaintedPath.go:21:28:21:69 | call to Join | provenance | FunctionModel Sink:MaD:1 |
11-
| TaintedPath.go:68:39:68:56 | ...+... | TaintedPath.go:68:28:68:57 | call to Clean | provenance | MaD:4 Sink:MaD:1 |
6+
| TaintedPath.go:16:18:16:22 | selection of URL | TaintedPath.go:16:18:16:30 | call to Query | provenance | Src:MaD:2 MaD:3 |
7+
| TaintedPath.go:16:18:16:30 | call to Query | TaintedPath.go:19:29:19:40 | tainted_path | provenance | Sink:MaD:1 |
8+
| TaintedPath.go:16:18:16:30 | call to Query | TaintedPath.go:23:57:23:68 | tainted_path | provenance | |
9+
| TaintedPath.go:16:18:16:30 | call to Query | TaintedPath.go:70:39:70:56 | ...+... | provenance | |
10+
| TaintedPath.go:23:57:23:68 | tainted_path | TaintedPath.go:23:28:23:69 | call to Join | provenance | FunctionModel Sink:MaD:1 |
11+
| TaintedPath.go:70:39:70:56 | ...+... | TaintedPath.go:70:28:70:57 | call to Clean | provenance | MaD:4 Sink:MaD:1 |
1212
models
1313
| 1 | Sink: io/ioutil; ; false; ReadFile; ; ; Argument[0]; path-injection; manual |
1414
| 2 | Source: net/http; Request; true; URL; ; ; ; remote; manual |
1515
| 3 | Summary: net/url; URL; true; Query; ; ; Argument[receiver]; ReturnValue; taint; manual |
1616
| 4 | Summary: path; ; false; Clean; ; ; Argument[0]; ReturnValue; taint; manual |
1717
nodes
18-
| TaintedPath.go:14:18:14:22 | selection of URL | semmle.label | selection of URL |
19-
| TaintedPath.go:14:18:14:30 | call to Query | semmle.label | call to Query |
20-
| TaintedPath.go:17:29:17:40 | tainted_path | semmle.label | tainted_path |
21-
| TaintedPath.go:21:28:21:69 | call to Join | semmle.label | call to Join |
22-
| TaintedPath.go:21:57:21:68 | tainted_path | semmle.label | tainted_path |
23-
| TaintedPath.go:68:28:68:57 | call to Clean | semmle.label | call to Clean |
24-
| TaintedPath.go:68:39:68:56 | ...+... | semmle.label | ...+... |
18+
| TaintedPath.go:16:18:16:22 | selection of URL | semmle.label | selection of URL |
19+
| TaintedPath.go:16:18:16:30 | call to Query | semmle.label | call to Query |
20+
| TaintedPath.go:19:29:19:40 | tainted_path | semmle.label | tainted_path |
21+
| TaintedPath.go:23:28:23:69 | call to Join | semmle.label | call to Join |
22+
| TaintedPath.go:23:57:23:68 | tainted_path | semmle.label | tainted_path |
23+
| TaintedPath.go:70:28:70:57 | call to Clean | semmle.label | call to Clean |
24+
| TaintedPath.go:70:39:70:56 | ...+... | semmle.label | ...+... |
2525
subpaths

‎go/ql/test/query-tests/Security/CWE-022/TaintedPath.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"path/filepath"
99
"regexp"
1010
"strings"
11+
12+
"github.com/gorilla/mux"
1113
)
1214

1315
func handler(w http.ResponseWriter, r *http.Request) {
@@ -94,3 +96,10 @@ func handler(w http.ResponseWriter, r *http.Request) {
9496

9597
data, _ = ioutil.ReadFile(part.FileName())
9698
}
99+
100+
// GOOD: Sanitized by Gorilla's cleaner
101+
func GorillaHandler(w http.ResponseWriter, r *http.Request) {
102+
not_tainted_path := mux.Vars(r)
103+
data, _ := ioutil.ReadFile(filepath.Join("/home/user/", not_tainted_path))
104+
w.Write(data)
105+
}

‎go/ql/test/query-tests/Security/CWE-022/go.mod

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
module codeql-go-tests/frameworks/Mux
2+
3+
go 1.14
4+
5+
require github.com/gorilla/mux v1.7.4

‎go/ql/test/query-tests/Security/CWE-022/vendor/github.com/gorilla/mux/LICENSE

Lines changed: 27 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎go/ql/test/query-tests/Security/CWE-022/vendor/github.com/gorilla/mux/stub.go

Lines changed: 252 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎go/ql/test/query-tests/Security/CWE-022/vendor/modules.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# github.com/gorilla/mux v1.7.4
2+
## explicit
3+
github.com/gorilla/mux

0 commit comments

Comments
 (0)