Skip to content

Python: model os path file accesses #6741

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
codeql-ci merged 6 commits into from
Oct 13, 2021
Merged

Python: model os path file accesses #6741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
81adb7d
Python: Add tests for `os.path`-functions
yoff Sep 23, 2021
f2fbeed
Python: Model `os.path`-functions
yoff Sep 23, 2021
02e91b3
Python: Model functions that will raise
yoff Sep 30, 2021
4521a9f
Update python/ql/lib/semmle/python/frameworks/Stdlib.qll
yoff Oct 4, 2021
aa91c26
Python: Add missing taint steps
yoff Oct 4, 2021
6c108e4
Python: address review
yoff Oct 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 93 additions & 28 deletions python/ql/lib/semmle/python/frameworks/Stdlib.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,25 +196,110 @@ private module StdlibPrivate {
}

/**
* A call to `os.path.normpath`.
* See https://docs.python.org/3/library/os.path.html#os.path.normpath
* The `os.path` module offers a number of methods for checking if a file exists and/or has certain
* properties, leading to a file system access.
* A call to `os.path.exists` or `os.path.lexists` will check if a file exists on the file system.
* (Although, on some platforms, the check may return `false` due to missing permissions.)
* A call to `os.path.getatime` will raise `OSError` if the file does not exist or is inaccessible.
* See:
* - https://docs.python.org/3/library/os.path.html#os.path.exists
* - https://docs.python.org/3/library/os.path.html#os.path.lexists
* - https://docs.python.org/3/library/os.path.html#os.path.isfile
* - https://docs.python.org/3/library/os.path.html#os.path.isdir
* - https://docs.python.org/3/library/os.path.html#os.path.islink
* - https://docs.python.org/3/library/os.path.html#os.path.ismount
* - https://docs.python.org/3/library/os.path.html#os.path.getatime
* - https://docs.python.org/3/library/os.path.html#os.path.getmtime
* - https://docs.python.org/3/library/os.path.html#os.path.getctime
* - https://docs.python.org/3/library/os.path.html#os.path.getsize
* - https://docs.python.org/3/library/os.path.html#os.path.realpath
*/
private class OsPathNormpathCall extends Path::PathNormalization::Range, DataFlow::CallCfgNode {
OsPathNormpathCall() { this = os::path().getMember("normpath").getACall() }
private class OsPathProbingCall extends FileSystemAccess::Range, DataFlow::CallCfgNode {
OsPathProbingCall() {
this =
os::path()
.getMember([
// these check if the file exists
"exists", "lexists", "isfile", "isdir", "islink", "ismount",
// these raise errors if the file does not exist
"getatime", "getmtime", "getctime", "getsize"
])
.getACall()
}

DataFlow::Node getPathArg() { result in [this.getArg(0), this.getArgByName("path")] }
override DataFlow::Node getAPathArgument() {
result in [this.getArg(0), this.getArgByName("path")]
}
}

/** A call to `os.path.samefile` will raise an exception if an `os.stat()` call on either pathname fails. */
private class OsPathSamefileCall extends FileSystemAccess::Range, DataFlow::CallCfgNode {
OsPathSamefileCall() { this = os::path().getMember("samefile").getACall() }

override DataFlow::Node getAPathArgument() {
result in [
this.getArg(0), this.getArgByName("path1"), this.getArg(1), this.getArgByName("path2")
]
}
}

// Functions with non-standard arguments:
// - os.path.join(path, *paths)
// - os.path.relpath(path, start=os.curdir)
// these functions need special treatment when computing `getPathArg`.
//
// Functions that excluded because they can act as sanitizers:
// - os.path.commonpath(paths): takes a sequence
// - os.path.commonprefix(list): takes a list argument
// unless the user control all arguments, we are comparing with a known value.
private string pathComputation() {
result in [
"abspath", "basename", "commonpath", "dirname", "expanduser", "expandvars", "join",
"normcase", "normpath", "realpath", "relpath", "split", "splitdrive", "splitext"
]
}

/** An additional taint step for calls to `os.path.normpath` */
private class OsPathNormpathCallAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
/**
* The `os.path` module offers a number of methods for computing new paths from existing paths.
* These should all propagate taint.
*/
private class OsPathComputation extends DataFlow::CallCfgNode {
string methodName;

OsPathComputation() {
methodName = pathComputation() and
this = os::path().getMember(methodName).getACall()
}

DataFlow::Node getPathArg() {
result in [this.getArg(0), this.getArgByName("path")]
or
methodName = "join" and result = this.getArg(_)
or
methodName = "relpath" and result in [this.getArg(1), this.getArgByName("start")]
}
}

/** An additional taint step for path computations. */
private class OsPathComputationAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
exists(OsPathNormpathCall call |
exists(OsPathComputation call |
nodeTo = call and
nodeFrom = call.getPathArg()
)
}
}

/**
* A call to `os.path.normpath`.
* See https://docs.python.org/3/library/os.path.html#os.path.normpath
*/
private class OsPathNormpathCall extends Path::PathNormalization::Range, DataFlow::CallCfgNode {
OsPathNormpathCall() { this = os::path().getMember("normpath").getACall() }

DataFlow::Node getPathArg() { result in [this.getArg(0), this.getArgByName("path")] }
}

/**
* A call to `os.path.abspath`.
* See https://docs.python.org/3/library/os.path.html#os.path.abspath
Expand All @@ -225,16 +310,6 @@ private module StdlibPrivate {
DataFlow::Node getPathArg() { result in [this.getArg(0), this.getArgByName("path")] }
}

/** An additional taint step for calls to `os.path.abspath` */
private class OsPathAbspathCallAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
exists(OsPathAbspathCall call |
nodeTo = call and
nodeFrom = call.getPathArg()
)
}
}

/**
* A call to `os.path.realpath`.
* See https://docs.python.org/3/library/os.path.html#os.path.realpath
Expand All @@ -245,16 +320,6 @@ private module StdlibPrivate {
DataFlow::Node getPathArg() { result in [this.getArg(0), this.getArgByName("path")] }
}

/** An additional taint step for calls to `os.path.realpath` */
private class OsPathRealpathCallAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
exists(OsPathRealpathCall call |
nodeTo = call and
nodeFrom = call.getPathArg()
)
}
}

/**
* A call to `os.system`.
* See https://docs.python.org/3/library/os.html#os.system
Expand Down
7 changes: 7 additions & 0 deletions python/ql/test/library-tests/frameworks/stdlib/FileSystemAccess.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,10 @@ def through_function(open_file):
open_file.write("foo") # $ fileWriteData="foo" getAPathArgument="path"

through_function(f)

from os import path
path.exists("filepath") # $ getAPathArgument="filepath"
path.isfile("filepath") # $ getAPathArgument="filepath"
path.isdir("filepath") # $ getAPathArgument="filepath"
path.islink("filepath") # $ getAPathArgument="filepath"
path.ismount("filepath") # $ getAPathArgument="filepath"