Skip to content

Commit 504ae0f

Browse files
committed
Update go path sanitizers and sinks
1 parent 391e9f7 commit 504ae0f

File tree

4 files changed

+12
-8
lines changed

4 files changed

+12
-8
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
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/ext/os.model.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ extensions:
2828
- ["os", "", False, "ReadDir", "", "", "Argument[0]", "path-injection", "manual"]
2929
- ["os", "", False, "ReadFile", "", "", "Argument[0]", "path-injection", "manual"]
3030
- ["os", "", False, "MkdirTemp", "", "", "Argument[0..1]", "path-injection", "manual"]
31-
- ["os", "", False, "CreateTemp", "", "", "Argument[0..1]", "path-injection", "manual"]
3231
- ["os", "", False, "WriteFile", "", "", "Argument[0]", "path-injection", "manual"]
3332
# command-injection
3433
- ["os", "", False, "StartProcess", "", "", "Argument[0]", "command-injection", "manual"]
Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
#select
22
| 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 |
33
| 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 |
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 |
55
edges
66
| TaintedPath.go:14:18:14:22 | selection of URL | TaintedPath.go:14:18:14:30 | call to Query | provenance | Src:MaD:2 MaD:3 |
77
| TaintedPath.go:14:18:14:30 | call to Query | TaintedPath.go:17:29:17:40 | tainted_path | provenance | Sink:MaD:1 |
88
| 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 | |
9+
| TaintedPath.go:14:18:14:30 | call to Query | TaintedPath.go:69:39:69:56 | ...+... | provenance | |
1010
| 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 |
11+
| TaintedPath.go:69:39:69:56 | ...+... | TaintedPath.go:69:28:69: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 |
@@ -20,6 +20,6 @@ nodes
2020
| TaintedPath.go:17:29:17:40 | tainted_path | semmle.label | tainted_path |
2121
| TaintedPath.go:21:28:21:69 | call to Join | semmle.label | call to Join |
2222
| 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 | ...+... |
23+
| TaintedPath.go:69:28:69:57 | call to Clean | semmle.label | call to Clean |
24+
| TaintedPath.go:69:39:69:56 | ...+... | semmle.label | ...+... |
2525
subpaths

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import (
88
"path/filepath"
99
"regexp"
1010
"strings"
11+
"os"
1112
)
12-
1313
func handler(w http.ResponseWriter, r *http.Request) {
1414
tainted_path := r.URL.Query()["path"][0]
1515

@@ -58,9 +58,10 @@ func handler(w http.ResponseWriter, r *http.Request) {
5858
w.Write(data)
5959
}
6060

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

6667
// BAD: Sanitized by path.Clean with a prepended '/' forcing interpretation

0 commit comments

Comments
 (0)