Skip to content
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.

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

Go: Add Tainted Path sanitizers #17759

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
19 changes: 19 additions & 0 deletions go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,25 @@ module TaintedPath {
}
}

/**
* A call to `mux.Vars(path)`, considered to sanitize `path` against path traversal.
* Only enabled when `SkipClean` is not set true.
*/
class MuxVarsSanitizer extends Sanitizer {
MuxVarsSanitizer() {
exists(Function m |
m.hasQualifiedName(package("github.com/gorilla/mux", ""), "Vars") and
this = m.getACall().getResult()
) and
not exists(CallExpr f |
f.getTarget()
.(Method)
.hasQualifiedName(package("github.com/gorilla/mux", ""), "Router", "SkipClean") and
f.getArgument(0).getBoolValue() = true
)
}
}

/**
* A read from the field `Filename` of the type `mime/multipart.FileHeader`,
* considered as a sanitizer for path traversal.
Expand Down
4 changes: 4 additions & 0 deletions go/ql/src/change-notes/2024-10-14-gopathsanitizer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Added [github.com/gorilla/mux.Vars](https://pkg.go.dev/github.com/gorilla/mux#Vars) to path sanitizers.
Kwstubbs marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// GOOD: Sanitized by Gorilla's cleaner
package main

import (
"io/ioutil"
"net/http"
"path/filepath"

"github.com/gorilla/mux"
)

Kwstubbs marked this conversation as resolved.
Show resolved Hide resolved
func GorillaHandler(w http.ResponseWriter, r *http.Request) {
not_tainted_path := mux.Vars(r)["id"]
data, _ := ioutil.ReadFile(filepath.Join("/home/user/", not_tainted_path))
w.Write(data)
}

func main() {
var router = mux.NewRouter()
router.SkipClean(false)
router.HandleFunc("/{category}", GorillaHandler)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
edges
nodes
subpaths
#select
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
query: Security/CWE-022/TaintedPath.ql
postprocess: TestUtilities/PrettyPrintModels.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module codeql-go-tests/frameworks/Mux

go 1.14

require github.com/gorilla/mux v1.7.4

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# github.com/gorilla/mux v1.7.4
## explicit
github.com/gorilla/mux
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// GOOD: Sanitized by Gorilla's cleaner
package main

import (
"io/ioutil"
"net/http"
"path/filepath"

"github.com/gorilla/mux"
)

Kwstubbs marked this conversation as resolved.
Show resolved Hide resolved
func GorillaHandler(w http.ResponseWriter, r *http.Request) {
not_tainted_path := mux.Vars(r)["id"]
data, _ := ioutil.ReadFile(filepath.Join("/home/user/", not_tainted_path))
w.Write(data)
}

func main() {
var router = mux.NewRouter()
router.SkipClean(true)
router.HandleFunc("/{category}", GorillaHandler)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#select
| MuxClean.go:14:29:14:74 | call to Join | MuxClean.go:13:22:13:32 | call to Vars | MuxClean.go:14:29:14:74 | call to Join | This path depends on a $@. | MuxClean.go:13:22:13:32 | call to Vars | user-provided value |
edges
| MuxClean.go:13:22:13:32 | call to Vars | MuxClean.go:14:58:14:73 | not_tainted_path | provenance | Src:MaD:2 |
| MuxClean.go:14:58:14:73 | not_tainted_path | MuxClean.go:14:29:14:74 | call to Join | provenance | FunctionModel Sink:MaD:1 |
models
| 1 | Sink: io/ioutil; ; false; ReadFile; ; ; Argument[0]; path-injection; manual |
| 2 | Source: github.com/gorilla/mux; ; true; Vars; ; ; ReturnValue; remote; manual |
nodes
| MuxClean.go:13:22:13:32 | call to Vars | semmle.label | call to Vars |
| MuxClean.go:14:29:14:74 | call to Join | semmle.label | call to Join |
| MuxClean.go:14:58:14:73 | not_tainted_path | semmle.label | not_tainted_path |
subpaths
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
query: Security/CWE-022/TaintedPath.ql
postprocess: TestUtilities/PrettyPrintModels.ql
Loading
Loading