Skip to content

Commit f86152d

Browse files
KwstubbsKwstubbs
committed
Add sanitizer changes and fix test
1 parent 504ae0f commit f86152d

File tree

4 files changed

+32
-20
lines changed

4 files changed

+32
-20
lines changed

‎go/ql/lib/change-notes/2025-07-15-path-injection-sanitizers.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
---
22
category: minorAnalysis
33
---
4-
* Remove model`CreateTemp` function, from the `os` package, as a path-injection sink due to proper sanitization by Go. Add check for `os.PathSeparator` in sanitizers for path-injection query.
4+
* Remove model `CreateTemp` function, from the `os` package, as a path-injection sink due to proper sanitization by Go. Add check for `os.PathSeparator` in sanitizers for path-injection query.

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,14 @@ module TaintedPath {
8787
exists(DataFlow::CallNode cleanCall, StringOps::Concatenation concatNode |
8888
cleanCall = any(Function f | f.hasQualifiedName("path/filepath", "Clean")).getACall() and
8989
concatNode = cleanCall.getArgument(0) and
90-
concatNode.getOperand(0).asExpr().(StringLit).getValue() = "/" and
90+
(
91+
concatNode.getOperand(0).asExpr().(StringLit).getValue() = "/"
92+
or
93+
exists(DeclaredConstant dc |
94+
dc.hasQualifiedName("os", "PathSeparator") and
95+
dc.getAReference() = concatNode.getOperand(0).asExpr().getAChildExpr*()
96+
)
97+
) and
9198
this = cleanCall.getResult()
9299
)
93100
}

‎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:69:28:69:57 | call to Clean | TaintedPath.go:14:18:14:22 | selection of URL | TaintedPath.go:69:28:69:57 | call to Clean | This path depends on a $@. | TaintedPath.go:14:18:14:22 | selection of URL | user-provided value |
2+
| TaintedPath.go:18:29:18:40 | tainted_path | TaintedPath.go:15:18:15:22 | selection of URL | TaintedPath.go:18:29:18:40 | tainted_path | This path depends on a $@. | TaintedPath.go:15:18:15:22 | selection of URL | user-provided value |
3+
| TaintedPath.go:22:28:22:69 | call to Join | TaintedPath.go:15:18:15:22 | selection of URL | TaintedPath.go:22:28:22:69 | call to Join | This path depends on a $@. | TaintedPath.go:15:18:15:22 | selection of URL | user-provided value |
4+
| TaintedPath.go:74:28:74:57 | call to Clean | TaintedPath.go:15:18:15:22 | selection of URL | TaintedPath.go:74:28:74:57 | call to Clean | This path depends on a $@. | TaintedPath.go:15:18:15: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:69:39:69: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:69:39:69:56 | ...+... | TaintedPath.go:69:28:69:57 | call to Clean | provenance | MaD:4 Sink:MaD:1 |
6+
| TaintedPath.go:15:18:15:22 | selection of URL | TaintedPath.go:15:18:15:30 | call to Query | provenance | Src:MaD:2 MaD:3 |
7+
| TaintedPath.go:15:18:15:30 | call to Query | TaintedPath.go:18:29:18:40 | tainted_path | provenance | Sink:MaD:1 |
8+
| TaintedPath.go:15:18:15:30 | call to Query | TaintedPath.go:22:57:22:68 | tainted_path | provenance | |
9+
| TaintedPath.go:15:18:15:30 | call to Query | TaintedPath.go:74:39:74:56 | ...+... | provenance | |
10+
| TaintedPath.go:22:57:22:68 | tainted_path | TaintedPath.go:22:28:22:69 | call to Join | provenance | FunctionModel Sink:MaD:1 |
11+
| TaintedPath.go:74:39:74:56 | ...+... | TaintedPath.go:74:28:74: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:69:28:69:57 | call to Clean | semmle.label | call to Clean |
24-
| TaintedPath.go:69:39:69:56 | ...+... | semmle.label | ...+... |
18+
| TaintedPath.go:15:18:15:22 | selection of URL | semmle.label | selection of URL |
19+
| TaintedPath.go:15:18:15:30 | call to Query | semmle.label | call to Query |
20+
| TaintedPath.go:18:29:18:40 | tainted_path | semmle.label | tainted_path |
21+
| TaintedPath.go:22:28:22:69 | call to Join | semmle.label | call to Join |
22+
| TaintedPath.go:22:57:22:68 | tainted_path | semmle.label | tainted_path |
23+
| TaintedPath.go:74:28:74:57 | call to Clean | semmle.label | call to Clean |
24+
| TaintedPath.go:74:39:74:56 | ...+... | semmle.label | ...+... |
2525
subpaths

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,13 @@ import (
44
"io/ioutil"
55
"mime/multipart"
66
"net/http"
7+
"os"
78
"path"
89
"path/filepath"
910
"regexp"
1011
"strings"
11-
"os"
1212
)
13+
1314
func handler(w http.ResponseWriter, r *http.Request) {
1415
tainted_path := r.URL.Query()["path"][0]
1516

@@ -58,9 +59,13 @@ func handler(w http.ResponseWriter, r *http.Request) {
5859
w.Write(data)
5960
}
6061

61-
// GOOD: Sanitized by filepath.Clean with a prepended '/' or os.PathSeparator forcing interpretation
62+
// GOOD: Sanitized by filepath.Clean with a prepended '/' forcing interpretation
6263
// as an absolute path, so that Clean will throw away any leading `..` components.
6364
data, _ = ioutil.ReadFile(filepath.Clean("/" + tainted_path))
65+
w.Write(data)
66+
67+
// GOOD: Sanitized by filepath.Clean with a prepended os.PathSeparator forcing interpretation
68+
// as an absolute path, so that Clean will throw away any leading `..` components.
6469
data, _ = ioutil.ReadFile(filepath.Clean(string(os.PathSeparator) + tainted_path))
6570
w.Write(data)
6671

0 commit comments

Comments
 (0)