diff --git a/ql/src/semmle/go/security/StoredXssCustomizations.qll b/ql/src/semmle/go/security/StoredXssCustomizations.qll index fbe2029b71c..973635b754f 100644 --- a/ql/src/semmle/go/security/StoredXssCustomizations.qll +++ b/ql/src/semmle/go/security/StoredXssCustomizations.qll @@ -42,12 +42,9 @@ module StoredXss { class FileNameSource extends Source { FileNameSource() { // the second parameter to a filepath.Walk function - exists(DataFlow::ParameterNode prm, DataFlow::FunctionNode f, DataFlow::CallNode walkCall | - prm = this and - f.getParameter(0) = prm - | - walkCall.getTarget().hasQualifiedName("path/filepath", "Walk") and - walkCall.getArgument(1) = f.getASuccessor*() + exists(DataFlow::FunctionNode f, Function walkFn | this = f.getParameter(0) | + walkFn.hasQualifiedName("path/filepath", ["Walk", "WalkDir"]) and + walkFn.getACall().getArgument(1) = f.getASuccessor*() ) or // A call to os.FileInfo.Name diff --git a/ql/test/query-tests/Security/CWE-079/StoredXss.expected b/ql/test/query-tests/Security/CWE-079/StoredXss.expected index c9de068a66d..5c4a507e557 100644 --- a/ql/test/query-tests/Security/CWE-079/StoredXss.expected +++ b/ql/test/query-tests/Security/CWE-079/StoredXss.expected @@ -1,11 +1,15 @@ edges | StoredXss.go:13:21:13:31 | call to Name : string | StoredXss.go:13:21:13:36 | ...+... | -| stored.go:16:3:16:28 | ... := ...[0] : pointer type | stored.go:28:22:28:25 | name | +| stored.go:18:3:18:28 | ... := ...[0] : pointer type | stored.go:30:22:30:25 | name | +| stored.go:59:30:59:33 | definition of path : string | stored.go:61:22:61:25 | path | nodes | StoredXss.go:13:21:13:31 | call to Name : string | semmle.label | call to Name : string | | StoredXss.go:13:21:13:36 | ...+... | semmle.label | ...+... | -| stored.go:16:3:16:28 | ... := ...[0] : pointer type | semmle.label | ... := ...[0] : pointer type | -| stored.go:28:22:28:25 | name | semmle.label | name | +| stored.go:18:3:18:28 | ... := ...[0] : pointer type | semmle.label | ... := ...[0] : pointer type | +| stored.go:30:22:30:25 | name | semmle.label | name | +| stored.go:59:30:59:33 | definition of path : string | semmle.label | definition of path : string | +| stored.go:61:22:61:25 | path | semmle.label | path | #select | StoredXss.go:13:21:13:36 | ...+... | StoredXss.go:13:21:13:31 | call to Name : string | StoredXss.go:13:21:13:36 | ...+... | Stored cross-site scripting vulnerability due to $@. | StoredXss.go:13:21:13:31 | call to Name | stored value | -| stored.go:28:22:28:25 | name | stored.go:16:3:16:28 | ... := ...[0] : pointer type | stored.go:28:22:28:25 | name | Stored cross-site scripting vulnerability due to $@. | stored.go:16:3:16:28 | ... := ...[0] | stored value | +| stored.go:30:22:30:25 | name | stored.go:18:3:18:28 | ... := ...[0] : pointer type | stored.go:30:22:30:25 | name | Stored cross-site scripting vulnerability due to $@. | stored.go:18:3:18:28 | ... := ...[0] | stored value | +| stored.go:61:22:61:25 | path | stored.go:59:30:59:33 | definition of path : string | stored.go:61:22:61:25 | path | Stored cross-site scripting vulnerability due to $@. | stored.go:59:30:59:33 | definition of path | stored value | diff --git a/ql/test/query-tests/Security/CWE-079/stored.go b/ql/test/query-tests/Security/CWE-079/stored.go index 005a8e5f635..807ae7e1d44 100644 --- a/ql/test/query-tests/Security/CWE-079/stored.go +++ b/ql/test/query-tests/Security/CWE-079/stored.go @@ -3,8 +3,10 @@ package main import ( "database/sql" "io" + "io/fs" "log" "net/http" + "path/filepath" ) var db *sql.DB @@ -51,3 +53,13 @@ func storedserve2() { } }) } + +func storedserve3() { + http.HandleFunc("/user", func(w http.ResponseWriter, r *http.Request) { + filepath.WalkDir(".", func(path string, _ fs.DirEntry, err error) error { + // BAD: filenames are considered to be untrusted + io.WriteString(w, path) + return nil + }) + }) +}