mirror of
https://github.com/github/codeql.git
synced 2026-05-05 13:45:19 +02:00
Add New Sanitizers and Modify Old Ones
This commit is contained in:
Kevin Stubbings
@@ -74,19 +74,46 @@ module TaintedPath {
|
||||
}
|
||||
|
||||
/**
|
||||
* A call to `[file]path.Clean("/" + e)`, considered to sanitize `e` against path traversal.
|
||||
* 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", "path/filepath"], "Clean")).getACall() and
|
||||
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 call to `filepath.Base(e)`, considered to sanitize `e` against path traversal.
|
||||
*/
|
||||
class FilepathBaseSanitizer extends Sanitizer {
|
||||
FilepathBaseSanitizer() {
|
||||
exists(Function f, FunctionOutput outp |
|
||||
f.hasQualifiedName("path/filepath", "Base") and
|
||||
outp.isResult(0) and
|
||||
this = outp.getNode(f.getACall())
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/** An call to ParseMultipartForm creates multipart.Form and cleans mutlpart.Form.FileHeader.Filename using path.Base() */
|
||||
class MultipartClean extends Sanitizer {
|
||||
MultipartClean() {
|
||||
exists(DataFlow::FieldReadNode frn, ControlFlow::Node node, DataFlow::CallNode cleanCall, Method get |
|
||||
get.hasQualifiedName("net/http","Request", "ParseMultipartForm") and
|
||||
cleanCall = get.getACall() and
|
||||
cleanCall.asInstruction() = node and
|
||||
frn.getField().hasQualifiedName("mime/multipart", "FileHeader", "Filename") and
|
||||
node.getASuccessor*() = frn.asInstruction()
|
||||
|
|
||||
this = frn.getBase()
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A check of the form `!strings.Contains(nd, "..")`, considered as a sanitizer guard for
|
||||
@@ -105,6 +132,21 @@ module TaintedPath {
|
||||
branch = false
|
||||
}
|
||||
}
|
||||
/**
|
||||
* A replacement of the form `!strings.ReplaceAll(nd, "..")` or `!strings.ReplaceAll(nd, ".")`, considered as a sanitizer guard for
|
||||
* path traversal.
|
||||
*/
|
||||
class DotDotReplace extends Sanitizer {
|
||||
DotDotReplace() {
|
||||
exists(DataFlow::CallNode cleanCall, DataFlow::Node valueNode |
|
||||
cleanCall =
|
||||
any(Function f | f.hasQualifiedName("strings", "ReplaceAll")).getACall() and
|
||||
valueNode = cleanCall.getArgument(1) and
|
||||
(valueNode.asExpr().(StringLit).getValue() = ".." or valueNode.asExpr().(StringLit).getValue() = ".") and
|
||||
this = cleanCall.getResult()
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A node `nd` guarded by a check that ensures it is contained within some root folder,
|
||||
|
||||
@@ -1,19 +1,35 @@
|
||||
edges
|
||||
<<<<<<< HEAD
|
||||
| TaintedPath.go:13:18:13:22 | selection of URL | TaintedPath.go:13:18:13:30 | call to Query | provenance | |
|
||||
| TaintedPath.go:13:18:13:30 | call to Query | TaintedPath.go:16:29:16:40 | tainted_path | provenance | |
|
||||
| TaintedPath.go:13:18:13:30 | call to Query | TaintedPath.go:20:57:20:68 | tainted_path | provenance | |
|
||||
| TaintedPath.go:20:57:20:68 | tainted_path | TaintedPath.go:20:28:20:69 | call to Join | provenance | |
|
||||
| tst.go:14:2:14:39 | ... := ...[1] | tst.go:17:41:17:56 | selection of Filename | provenance | |
|
||||
=======
|
||||
| TaintedPath.go:13:18:13:22 | selection of URL : pointer type | TaintedPath.go:16:29:16:40 | tainted_path |
|
||||
| TaintedPath.go:13:18:13:22 | selection of URL : pointer type | TaintedPath.go:20:28:20:69 | call to Join |
|
||||
| TaintedPath.go:13:18:13:22 | selection of URL : pointer type | TaintedPath.go:67:28:67:57 | call to Clean |
|
||||
| TaintedPath.go:13:18:13:22 | selection of URL : pointer type | TaintedPath.go:77:28:77:56 | call to Base |
|
||||
| tst.go:14:2:14:39 | ... := ...[1] : pointer type | tst.go:17:41:17:56 | selection of Filename |
|
||||
>>>>>>> a45343fb6c (Add New Sanitizers and Modify Old Ones)
|
||||
nodes
|
||||
| TaintedPath.go:13:18:13:22 | selection of URL | semmle.label | selection of URL |
|
||||
| TaintedPath.go:13:18:13:30 | call to Query | semmle.label | call to Query |
|
||||
| TaintedPath.go:16:29:16:40 | tainted_path | semmle.label | tainted_path |
|
||||
| TaintedPath.go:20:28:20:69 | call to Join | semmle.label | call to Join |
|
||||
<<<<<<< HEAD
|
||||
| TaintedPath.go:20:57:20:68 | tainted_path | semmle.label | tainted_path |
|
||||
| tst.go:14:2:14:39 | ... := ...[1] | semmle.label | ... := ...[1] |
|
||||
=======
|
||||
| TaintedPath.go:67:28:67:57 | call to Clean | semmle.label | call to Clean |
|
||||
| TaintedPath.go:77:28:77:56 | call to Base | semmle.label | call to Base |
|
||||
| tst.go:14:2:14:39 | ... := ...[1] : pointer type | semmle.label | ... := ...[1] : pointer type |
|
||||
>>>>>>> a45343fb6c (Add New Sanitizers and Modify Old Ones)
|
||||
| tst.go:17:41:17:56 | selection of Filename | semmle.label | selection of Filename |
|
||||
subpaths
|
||||
#select
|
||||
| TaintedPath.go:16:29:16:40 | tainted_path | TaintedPath.go:13:18:13:22 | selection of URL | TaintedPath.go:16:29:16:40 | tainted_path | This path depends on a $@. | TaintedPath.go:13:18:13:22 | selection of URL | user-provided value |
|
||||
| TaintedPath.go:20:28:20:69 | call to Join | TaintedPath.go:13:18:13:22 | selection of URL | TaintedPath.go:20:28:20:69 | call to Join | This path depends on a $@. | TaintedPath.go:13:18:13:22 | selection of URL | user-provided value |
|
||||
| tst.go:17:41:17:56 | selection of Filename | tst.go:14:2:14:39 | ... := ...[1] | tst.go:17:41:17:56 | selection of Filename | This path depends on a $@. | tst.go:14:2:14:39 | ... := ...[1] | user-provided value |
|
||||
| TaintedPath.go:16:29:16:40 | tainted_path | TaintedPath.go:13:18:13:22 | selection of URL : pointer type | TaintedPath.go:16:29:16:40 | tainted_path | This path depends on a $@. | TaintedPath.go:13:18:13:22 | selection of URL | user-provided value |
|
||||
| TaintedPath.go:20:28:20:69 | call to Join | TaintedPath.go:13:18:13:22 | selection of URL : pointer type | TaintedPath.go:20:28:20:69 | call to Join | This path depends on a $@. | TaintedPath.go:13:18:13:22 | selection of URL | user-provided value |
|
||||
| TaintedPath.go:67:28:67:57 | call to Clean | TaintedPath.go:13:18:13:22 | selection of URL : pointer type | TaintedPath.go:67:28:67:57 | call to Clean | This path depends on a $@. | TaintedPath.go:13:18:13:22 | selection of URL | user-provided value |
|
||||
| TaintedPath.go:77:28:77:56 | call to Base | TaintedPath.go:13:18:13:22 | selection of URL : pointer type | TaintedPath.go:77:28:77:56 | call to Base | This path depends on a $@. | TaintedPath.go:13:18:13:22 | selection of URL | user-provided value |
|
||||
| tst.go:17:41:17:56 | selection of Filename | tst.go:14:2:14:39 | ... := ...[1] : pointer type | tst.go:17:41:17:56 | selection of Filename | This path depends on a $@. | tst.go:14:2:14:39 | ... := ...[1] | user-provided value |
|
||||
|
||||
@@ -31,6 +31,10 @@ func handler(w http.ResponseWriter, r *http.Request) {
|
||||
w.Write(data)
|
||||
}
|
||||
|
||||
// GOOD: Sanitized by strings.ReplaceAll and replaces all .. with empty string
|
||||
data, _ = ioutil.ReadFile(strings.ReplaceAll(tainted_path, "..", ""))
|
||||
w.Write(data)
|
||||
|
||||
// GOOD: This can only read inside the provided safe path
|
||||
_, err := filepath.Rel("/home/user/safepath", tainted_path)
|
||||
if err == nil {
|
||||
@@ -53,10 +57,33 @@ func handler(w http.ResponseWriter, r *http.Request) {
|
||||
w.Write(data)
|
||||
}
|
||||
|
||||
// GOOD: Sanitized by [file]path.Clean with a prepended '/' 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.
|
||||
data, _ = ioutil.ReadFile(filepath.Clean("/" + tainted_path))
|
||||
w.Write(data)
|
||||
|
||||
// BAD: Sanitized by path.Clean with a prepended '/' forcing interpretation
|
||||
// as an absolute path, however is not sufficient for Windows paths.
|
||||
data, _ = ioutil.ReadFile(path.Clean("/" + tainted_path))
|
||||
w.Write(data)
|
||||
|
||||
// GOOD: Sanitized by filepath.Base with a prepended '/' forcing interpretation
|
||||
// as an absolute path, so that Base will throw away any leading `..` components.
|
||||
data, _ = ioutil.ReadFile(filepath.Base("/" + tainted_path))
|
||||
w.Write(data)
|
||||
|
||||
// BAD: Sanitized by path.Base with a prepended '/' forcing interpretation
|
||||
// as an absolute path, however is not sufficient for Windows paths.
|
||||
data, _ = ioutil.ReadFile(path.Base("/" + tainted_path))
|
||||
w.Write(data)
|
||||
|
||||
// GOOD: Multipart.Form.FileHeader.Filename sanitized by filepath.Base when calling ParseMultipartForm
|
||||
r.ParseMultipartForm(32 << 20)
|
||||
form := r.MultipartForm
|
||||
files, ok := form.File["files"]
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
data, _ = ioutil.ReadFile(files[0].Filename)
|
||||
w.Write(data)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user