Merge pull request #678 from smowton/smowton/feature/note-filepath-clean-sanitizer

Note that `filepath.Clean("/" + e)` is a sanitizer against path traversal attacks

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

@@ -76,6 +76,20 @@ module TaintedPath {
     }
   }
 
+  /**
+   * A call to `filepath.Clean("/" + e)`, considered to sanitize `e` against path traversal.
+   */
+  class FilepathCleanSanitizer extends Sanitizer {
+    FilepathCleanSanitizer() {
+      exists(DataFlow::CallNode cleanCall, StringOps::Concatenation concatNode |
+        cleanCall = any(Function f | f.hasQualifiedName("path/filepath", "Clean")).getACall() and
+        concatNode = cleanCall.getArgument(0) and
+        concatNode.getOperand(0).asExpr().(StringLit).getValue() = "/" and
+        this = cleanCall.getResult()
+      )
+    }
+  }
+
   /**
    * A check of the form `!strings.Contains(nd, "..")`, considered as a sanitizer guard for
    * path traversal.

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

@@ -51,4 +51,9 @@ func handler(w http.ResponseWriter, r *http.Request) {
 		data, _ = ioutil.ReadFile(filepath.Join("/home/user/", path))
 		w.Write(data)
 	}
+
+	// GOOD: Sanitized by filepath.Clean with a prepended '/' forcing interpretation
+	// as an absolute path, so that Clean will throw away any leading `..` components.
+	data, _ = ioutil.ReadFile(filepath.Clean("/" + path))
+	w.Write(data)
 }