Skip to content

Update Go Path Injection Sanitizer and Sink #20064

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
owen-mc merged 4 commits into from
Sep 3, 2025
Merged

Update Go Path Injection Sanitizer and Sink #20064

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
504ae0f
Update go path sanitizers and sinks
Kwstubbs Jul 16, 2025
f86152d
Add sanitizer changes and fix test
Kwstubbs Jul 16, 2025
b4b848a
Fix tests and simplify sanitizer
Kwstubbs Jul 21, 2025
e2f3c9d
Reword change note
owen-mc Jul 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions go/ql/lib/change-notes/2025-07-15-path-injection-sanitizers.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* 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.
1 change: 0 additions & 1 deletion go/ql/lib/ext/os.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ extensions:
- ["os", "", False, "ReadDir", "", "", "Argument[0]", "path-injection", "manual"]
- ["os", "", False, "ReadFile", "", "", "Argument[0]", "path-injection", "manual"]
- ["os", "", False, "MkdirTemp", "", "", "Argument[0..1]", "path-injection", "manual"]
- ["os", "", False, "CreateTemp", "", "", "Argument[0..1]", "path-injection", "manual"]
- ["os", "", False, "WriteFile", "", "", "Argument[0]", "path-injection", "manual"]
# command-injection
- ["os", "", False, "StartProcess", "", "", "Argument[0]", "command-injection", "manual"]
Expand Down
10 changes: 5 additions & 5 deletions go/ql/test/query-tests/Security/CWE-022/TaintedPath.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
#select
| 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 |
| 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 |
| 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 |
| 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 |
edges
| TaintedPath.go:14:18:14:22 | selection of URL | TaintedPath.go:14:18:14:30 | call to Query | provenance | Src:MaD:2 MaD:3 |
| TaintedPath.go:14:18:14:30 | call to Query | TaintedPath.go:17:29:17:40 | tainted_path | provenance | Sink:MaD:1 |
| TaintedPath.go:14:18:14:30 | call to Query | TaintedPath.go:21:57:21:68 | tainted_path | provenance | |
| TaintedPath.go:14:18:14:30 | call to Query | TaintedPath.go:68:39:68:56 | ...+... | provenance | |
| TaintedPath.go:14:18:14:30 | call to Query | TaintedPath.go:69:39:69:56 | ...+... | provenance | |
| TaintedPath.go:21:57:21:68 | tainted_path | TaintedPath.go:21:28:21:69 | call to Join | provenance | FunctionModel Sink:MaD:1 |
| TaintedPath.go:68:39:68:56 | ...+... | TaintedPath.go:68:28:68:57 | call to Clean | provenance | MaD:4 Sink:MaD:1 |
| TaintedPath.go:69:39:69:56 | ...+... | TaintedPath.go:69:28:69:57 | call to Clean | provenance | MaD:4 Sink:MaD:1 |
models
| 1 | Sink: io/ioutil; ; false; ReadFile; ; ; Argument[0]; path-injection; manual |
| 2 | Source: net/http; Request; true; URL; ; ; ; remote; manual |
Expand All @@ -20,6 +20,6 @@ nodes
| TaintedPath.go:17:29:17:40 | tainted_path | semmle.label | tainted_path |
| TaintedPath.go:21:28:21:69 | call to Join | semmle.label | call to Join |
| TaintedPath.go:21:57:21:68 | tainted_path | semmle.label | tainted_path |
| TaintedPath.go:68:28:68:57 | call to Clean | semmle.label | call to Clean |
| TaintedPath.go:68:39:68:56 | ...+... | semmle.label | ...+... |
| TaintedPath.go:69:28:69:57 | call to Clean | semmle.label | call to Clean |
| TaintedPath.go:69:39:69:56 | ...+... | semmle.label | ...+... |
subpaths
5 changes: 3 additions & 2 deletions go/ql/test/query-tests/Security/CWE-022/TaintedPath.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
"path/filepath"
"regexp"
"strings"
"os"
)

func handler(w http.ResponseWriter, r *http.Request) {
tainted_path := r.URL.Query()["path"][0]

Expand Down Expand Up @@ -58,9 +58,10 @@ func handler(w http.ResponseWriter, r *http.Request) {
w.Write(data)
}

// GOOD: Sanitized by filepath.Clean with a prepended '/' forcing interpretation
// GOOD: Sanitized by filepath.Clean with a prepended '/' or os.PathSeparator forcing interpretation
// as an absolute path, so that Clean will throw away any leading `..` components.
data, _ = ioutil.ReadFile(filepath.Clean("/" + tainted_path))
data, _ = ioutil.ReadFile(filepath.Clean(string(os.PathSeparator) + tainted_path))
w.Write(data)

// BAD: Sanitized by path.Clean with a prepended '/' forcing interpretation
Expand Down
Loading