hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-17 01:03:14 +01:00
Code Issues Packages Projects Releases Wiki Activity

Add sanitizer changes and fix test

Browse Source
This commit is contained in:
Kevin Stubbings
2025-07-16 21:27:33 +00:00
parent 504ae0f35a
commit f86152d3bd
4 changed files with 32 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
go/ql/lib/change-notes/2025-07-15-path-injection-sanitizers.md
View File

@@ -1,4 +1,4 @@
--- ---
category: minorAnalysis 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. * 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.

9
go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll
View File

@@ -87,7 +87,14 @@ module TaintedPath {
exists(DataFlow::CallNode cleanCall, StringOps::Concatenation concatNode | exists(DataFlow::CallNode cleanCall, StringOps::Concatenation concatNode |
cleanCall = any(Function f | f.hasQualifiedName("path/filepath", "Clean")).getACall() and cleanCall = any(Function f | f.hasQualifiedName("path/filepath", "Clean")).getACall() and
concatNode = cleanCall.getArgument(0) and concatNode = cleanCall.getArgument(0) and
concatNode.getOperand(0).asExpr().(StringLit).getValue() = "/" and (
concatNode.getOperand(0).asExpr().(StringLit).getValue() = "/"
or
exists(DeclaredConstant dc |
dc.hasQualifiedName("os", "PathSeparator") and
dc.getAReference() = concatNode.getOperand(0).asExpr().getAChildExpr*()
)
) and
this = cleanCall.getResult() this = cleanCall.getResult()
) )
} }

32
go/ql/test/query-tests/Security/CWE-022/TaintedPath.expected
View File

@@ -1,25 +1,25 @@
#select #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: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 |
| 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: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 |
| 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 | | 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 |
edges 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:15:18:15:22 | selection of URL | TaintedPath.go:15:18:15: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:15:18:15:30 | call to Query | TaintedPath.go:18:29:18: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:15:18:15:30 | call to Query | TaintedPath.go:22:57:22:68 | tainted_path | provenance | |
| TaintedPath.go:14:18:14:30 | call to Query | TaintedPath.go:69:39:69:56 | ...+... | provenance | | | TaintedPath.go:15:18:15:30 | call to Query | TaintedPath.go:74:39:74: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:22:57:22:68 | tainted_path | TaintedPath.go:22:28:22:69 | call to Join | provenance | FunctionModel Sink:MaD:1 |
| TaintedPath.go:69:39:69:56 | ...+... | TaintedPath.go:69:28:69:57 | call to Clean | provenance | MaD:4 Sink:MaD:1 | | TaintedPath.go:74:39:74:56 | ...+... | TaintedPath.go:74:28:74:57 | call to Clean | provenance | MaD:4 Sink:MaD:1 |
models models
| 1 | Sink: io/ioutil; ; false; ReadFile; ; ; Argument[0]; path-injection; manual | | 1 | Sink: io/ioutil; ; false; ReadFile; ; ; Argument[0]; path-injection; manual |
| 2 | Source: net/http; Request; true; URL; ; ; ; remote; manual | | 2 | Source: net/http; Request; true; URL; ; ; ; remote; manual |
| 3 | Summary: net/url; URL; true; Query; ; ; Argument[receiver]; ReturnValue; taint; manual | | 3 | Summary: net/url; URL; true; Query; ; ; Argument[receiver]; ReturnValue; taint; manual |
| 4 | Summary: path; ; false; Clean; ; ; Argument[0]; ReturnValue; taint; manual | | 4 | Summary: path; ; false; Clean; ; ; Argument[0]; ReturnValue; taint; manual |
nodes nodes
| TaintedPath.go:14:18:14:22 | selection of URL | semmle.label | selection of URL | | TaintedPath.go:15:18:15:22 | selection of URL | semmle.label | selection of URL |
| TaintedPath.go:14:18:14:30 | call to Query | semmle.label | call to Query | | TaintedPath.go:15:18:15:30 | call to Query | semmle.label | call to Query |
| TaintedPath.go:17:29:17:40 | tainted_path | semmle.label | tainted_path | | TaintedPath.go:18:29:18:40 | tainted_path | semmle.label | tainted_path |
| TaintedPath.go:21:28:21:69 | call to Join | semmle.label | call to Join | | TaintedPath.go:22:28:22:69 | call to Join | semmle.label | call to Join |
| TaintedPath.go:21:57:21:68 | tainted_path | semmle.label | tainted_path | | TaintedPath.go:22:57:22:68 | tainted_path | semmle.label | tainted_path |
| TaintedPath.go:69:28:69:57 | call to Clean | semmle.label | call to Clean | | TaintedPath.go:74:28:74:57 | call to Clean | semmle.label | call to Clean |
| TaintedPath.go:69:39:69:56 | ...+... | semmle.label | ...+... | | TaintedPath.go:74:39:74:56 | ...+... | semmle.label | ...+... |
subpaths subpaths

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

@@ -4,12 +4,13 @@ import (
"io/ioutil" "io/ioutil"
"mime/multipart" "mime/multipart"
"net/http" "net/http"
"os"
"path" "path"
"path/filepath" "path/filepath"
"regexp" "regexp"
"strings" "strings"
"os"
) )
func handler(w http.ResponseWriter, r *http.Request) { func handler(w http.ResponseWriter, r *http.Request) {
tainted_path := r.URL.Query()["path"][0] tainted_path := r.URL.Query()["path"][0]
@@ -58,9 +59,13 @@ func handler(w http.ResponseWriter, r *http.Request) {
w.Write(data) w.Write(data)
} }
// GOOD: Sanitized by filepath.Clean with a prepended '/' or os.PathSeparator forcing interpretation // GOOD: Sanitized by filepath.Clean with a prepended '/' forcing interpretation
// as an absolute path, so that Clean will throw away any leading `..` components. // 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("/" + tainted_path))
w.Write(data)
// GOOD: Sanitized by filepath.Clean with a prepended os.PathSeparator forcing interpretation
// as an absolute path, so that Clean will throw away any leading `..` components.
data, _ = ioutil.ReadFile(filepath.Clean(string(os.PathSeparator) + tainted_path)) data, _ = ioutil.ReadFile(filepath.Clean(string(os.PathSeparator) + tainted_path))
w.Write(data) w.Write(data)