Merge pull request #17759 from Kwstubbs/path-sanitizers

Go: Add Tainted Path sanitizers

Author: owen-mc

go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll

@@ -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.

go/ql/src/change-notes/2024-10-14-gopathsanitizer.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added [github.com/gorilla/mux.Vars](https://pkg.go.dev/github.com/gorilla/mux#Vars) to path sanitizers (disabled if [github.com/gorilla/mix.Router.SkipClean](https://pkg.go.dev/github.com/gorilla/mux#Router.SkipClean) has been called).

go/ql/test/query-tests/Security/CWE-022/GorillaMuxDefault/MuxClean.go

@@ -0,0 +1,22 @@
+package main
+
+import (
+	"io/ioutil"
+	"net/http"
+	"path/filepath"
+
+	"github.com/gorilla/mux"
+)
+
+// GOOD: Sanitized by Gorilla's cleaner
+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)
+}

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

@@ -0,0 +1,4 @@
+edges
+nodes
+subpaths
+#select

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

@@ -0,0 +1,2 @@
+query: Security/CWE-022/TaintedPath.ql
+postprocess: utils/test//PrettyPrintModels.ql

go/ql/test/query-tests/Security/CWE-022/GorillaMuxDefault/go.mod

@@ -0,0 +1,5 @@
+module codeql-go-tests/frameworks/Mux
+
+go 1.14
+
+require github.com/gorilla/mux v1.7.4

go/ql/test/query-tests/Security/CWE-022/GorillaMuxDefault/vendor/github.com/gorilla/mux/LICENSE

@@ -0,0 +1,3 @@
+# github.com/gorilla/mux v1.7.4
+## explicit
+github.com/gorilla/mux

go/ql/test/query-tests/Security/CWE-022/GorillaMuxDefault/vendor/github.com/gorilla/mux/stub.go

@@ -0,0 +1,22 @@
+package main
+
+import (
+	"io/ioutil"
+	"net/http"
+	"path/filepath"
+
+	"github.com/gorilla/mux"
+)
+
+// BAD: Gorilla's `Vars` is not a sanitizer as `Router.SkipClean` has been called
+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)
+}