mirror of
https://github.com/github/codeql.git
synced 2026-05-14 11:19:27 +02:00
Add os.path.basename as a sanitizer for py/path-injection
- Add test cases in path_injection.py demonstrating that os.path.basename prevents path traversal attacks (false positive scenarios) - Add OsPathBasenameCall sanitizer class in PathInjectionCustomizations.qll that recognizes calls to os.path.basename (and posixpath/ntpath/genericpath variants) as barriers for the path-injection taint flow os.path.basename strips all directory components from a path, returning only the final filename. This makes it impossible for an attacker to inject path traversal sequences like ../etc/passwd - the basename of such input would just be 'passwd'. Agent-Logs-Url: https://github.com/github/codeql/sessions/6603215b-21cd-4e05-8905-550434c7b9ff Co-authored-by: hvitved <3667920+hvitved@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
dcbdd43cf9
commit
3a512ae777
@@ -9,6 +9,7 @@ private import semmle.python.dataflow.new.DataFlow
|
||||
private import semmle.python.Concepts
|
||||
private import semmle.python.dataflow.new.RemoteFlowSources
|
||||
private import semmle.python.dataflow.new.BarrierGuards
|
||||
private import semmle.python.ApiGraphs
|
||||
|
||||
/**
|
||||
* Provides default sources, and sinks for detecting
|
||||
@@ -105,4 +106,25 @@ module PathInjection {
|
||||
class SanitizerFromModel extends Sanitizer {
|
||||
SanitizerFromModel() { ModelOutput::barrierNode(this, "path-injection") }
|
||||
}
|
||||
|
||||
/**
|
||||
* A call to `os.path.basename`, considered as a sanitizer for path injection.
|
||||
*
|
||||
* `os.path.basename` returns the final component of a path, stripping any
|
||||
* leading directory components. This prevents path traversal attacks since
|
||||
* the result cannot contain directory separators or relative path components.
|
||||
* See https://docs.python.org/3/library/os.path.html#os.path.basename
|
||||
*/
|
||||
private class OsPathBasenameCall extends Sanitizer, DataFlow::CallCfgNode {
|
||||
OsPathBasenameCall() {
|
||||
exists(API::Node osPathModule |
|
||||
(
|
||||
osPathModule = API::moduleImport("os").getMember("path")
|
||||
or
|
||||
osPathModule = API::moduleImport(["posixpath", "ntpath", "genericpath"])
|
||||
) and
|
||||
this = osPathModule.getMember("basename").getACall()
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -150,3 +150,20 @@ def safe_set_of_files():
|
||||
if filename in SAFE_FILES:
|
||||
path = os.path.join(STATIC_DIR, filename)
|
||||
f = open(path) # $ SPURIOUS: Alert
|
||||
|
||||
|
||||
@app.route("/basename-sanitizer")
|
||||
def basename_sanitizer():
|
||||
filename = request.args.get('filename', '')
|
||||
# Secure mitigation pattern: os.path.basename strips all directory components,
|
||||
# preventing path traversal attacks.
|
||||
path = os.path.join(STATIC_DIR, os.path.basename(filename))
|
||||
f = open(path) # $ result=OK
|
||||
|
||||
|
||||
@app.route("/basename-no-join")
|
||||
def basename_no_join():
|
||||
filename = request.args.get('filename', '')
|
||||
# basename alone also prevents directory traversal
|
||||
path = os.path.basename(filename)
|
||||
f = open(path) # $ result=OK
|
||||
|
||||
Reference in New Issue
Block a user