diff --git a/python/ql/lib/semmle/python/security/dataflow/PathInjectionCustomizations.qll b/python/ql/lib/semmle/python/security/dataflow/PathInjectionCustomizations.qll index 7121faa19ffb..c1e77b05e8c5 100644 --- a/python/ql/lib/semmle/python/security/dataflow/PathInjectionCustomizations.qll +++ b/python/ql/lib/semmle/python/security/dataflow/PathInjectionCustomizations.qll @@ -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,24 @@ 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"]) + | + this = osPathModule.getMember("basename").getACall() + ) + } + } } diff --git a/python/ql/test/query-tests/Security/CWE-022-PathInjection/path_injection.py b/python/ql/test/query-tests/Security/CWE-022-PathInjection/path_injection.py index bff0d750c9fd..87e40c4d0426 100644 --- a/python/ql/test/query-tests/Security/CWE-022-PathInjection/path_injection.py +++ b/python/ql/test/query-tests/Security/CWE-022-PathInjection/path_injection.py @@ -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