Skip to content

Commit 3a512ae

Browse files
Copilothvitved
andauthored
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>
1 parent dcbdd43 commit 3a512ae

File tree

2 files changed

+39
-0
lines changed

2 files changed

+39
-0
lines changed

python/ql/lib/semmle/python/security/dataflow/PathInjectionCustomizations.qll

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ private import semmle.python.dataflow.new.DataFlow
99
private import semmle.python.Concepts
1010
private import semmle.python.dataflow.new.RemoteFlowSources
1111
private import semmle.python.dataflow.new.BarrierGuards
12+
private import semmle.python.ApiGraphs
1213

1314
/**
1415
* Provides default sources, and sinks for detecting
@@ -105,4 +106,25 @@ module PathInjection {
105106
class SanitizerFromModel extends Sanitizer {
106107
SanitizerFromModel() { ModelOutput::barrierNode(this, "path-injection") }
107108
}
109+
110+
/**
111+
* A call to `os.path.basename`, considered as a sanitizer for path injection.
112+
*
113+
* `os.path.basename` returns the final component of a path, stripping any
114+
* leading directory components. This prevents path traversal attacks since
115+
* the result cannot contain directory separators or relative path components.
116+
* See https://docs.python.org/3/library/os.path.html#os.path.basename
117+
*/
118+
private class OsPathBasenameCall extends Sanitizer, DataFlow::CallCfgNode {
119+
OsPathBasenameCall() {
120+
exists(API::Node osPathModule |
121+
(
122+
osPathModule = API::moduleImport("os").getMember("path")
123+
or
124+
osPathModule = API::moduleImport(["posixpath", "ntpath", "genericpath"])
125+
) and
126+
this = osPathModule.getMember("basename").getACall()
127+
)
128+
}
129+
}
108130
}

python/ql/test/query-tests/Security/CWE-022-PathInjection/path_injection.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,3 +150,20 @@ def safe_set_of_files():
150150
if filename in SAFE_FILES:
151151
path = os.path.join(STATIC_DIR, filename)
152152
f = open(path) # $ SPURIOUS: Alert
153+
154+
155+
@app.route("/basename-sanitizer")
156+
def basename_sanitizer():
157+
filename = request.args.get('filename', '')
158+
# Secure mitigation pattern: os.path.basename strips all directory components,
159+
# preventing path traversal attacks.
160+
path = os.path.join(STATIC_DIR, os.path.basename(filename))
161+
f = open(path) # $ result=OK
162+
163+
164+
@app.route("/basename-no-join")
165+
def basename_no_join():
166+
filename = request.args.get('filename', '')
167+
# basename alone also prevents directory traversal
168+
path = os.path.basename(filename)
169+
f = open(path) # $ result=OK

0 commit comments

Comments
 (0)