From 9f4107d211449ae9c2da974f85af0f82d0d7fee5 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Tue, 16 Nov 2021 10:27:41 +0100 Subject: [PATCH 1/2] Python: Model `posixpath`, `ntpath`, and `genericpath` modules --- python/change-notes/2021-11-16-posixpath.md | 2 ++ python/ql/lib/semmle/python/frameworks/Stdlib.qll | 12 +++++++++++- .../frameworks/stdlib/FileSystemAccess.py | 8 ++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 python/change-notes/2021-11-16-posixpath.md diff --git a/python/change-notes/2021-11-16-posixpath.md b/python/change-notes/2021-11-16-posixpath.md new file mode 100644 index 00000000000..d9103dd6115 --- /dev/null +++ b/python/change-notes/2021-11-16-posixpath.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* Added modeling of the `posixpath`, `ntpath`, and `genericpath` modules for path operations (although these are not supposed to be used), resulting in new sinks for the _Uncontrolled data used in path expression_ (`py/path-injection`) query. diff --git a/python/ql/lib/semmle/python/frameworks/Stdlib.qll b/python/ql/lib/semmle/python/frameworks/Stdlib.qll index 4884a2c3624..6601b0aecae 100644 --- a/python/ql/lib/semmle/python/frameworks/Stdlib.qll +++ b/python/ql/lib/semmle/python/frameworks/Stdlib.qll @@ -254,7 +254,17 @@ private module StdlibPrivate { /** Provides models for the `os` module. */ module os { /** Gets a reference to the `os.path` module. */ - API::Node path() { result = os().getMember("path") } + API::Node path() { + result = os().getMember("path") + or + // although the following modules should not be used directly, they certainly can. + // Each one doesn't expose the full `os.path` API, so this is an overapproximation + // that made implementation easy. See + // - https://github.com/python/cpython/blob/b567b9d74bd9e476a3027335873bb0508d6e450f/Lib/posixpath.py#L31-L38 + // - https://github.com/python/cpython/blob/b567b9d74bd9e476a3027335873bb0508d6e450f/Lib/ntpath.py#L26-L32 + // - https://github.com/python/cpython/blob/b567b9d74bd9e476a3027335873bb0508d6e450f/Lib/genericpath.py#L9-L11 + result = API::moduleImport(["posixpath", "ntpath", "genericpath"]) + } /** Provides models for the `os.path` module */ module path { diff --git a/python/ql/test/library-tests/frameworks/stdlib/FileSystemAccess.py b/python/ql/test/library-tests/frameworks/stdlib/FileSystemAccess.py index 64f8ad5010d..c727aa1dcce 100644 --- a/python/ql/test/library-tests/frameworks/stdlib/FileSystemAccess.py +++ b/python/ql/test/library-tests/frameworks/stdlib/FileSystemAccess.py @@ -34,3 +34,11 @@ path.isfile("filepath") # $ getAPathArgument="filepath" path.isdir("filepath") # $ getAPathArgument="filepath" path.islink("filepath") # $ getAPathArgument="filepath" path.ismount("filepath") # $ getAPathArgument="filepath" + +import posixpath +import ntpath +import genericpath + +posixpath.exists("filepath") # $ getAPathArgument="filepath" +ntpath.exists("filepath") # $ getAPathArgument="filepath" +genericpath.exists("filepath") # $ getAPathArgument="filepath" From a980f26fda3d81cb9abe659894592d5174976257 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Tue, 16 Nov 2021 10:40:58 +0100 Subject: [PATCH 2/2] Python: Model `os.stat` (and friends) --- python/change-notes/2021-11-16-os-stat.md | 2 ++ .../lib/semmle/python/frameworks/Stdlib.qll | 23 +++++++++++++++++++ .../frameworks/stdlib/FileSystemAccess.py | 5 ++++ 3 files changed, 30 insertions(+) create mode 100644 python/change-notes/2021-11-16-os-stat.md diff --git a/python/change-notes/2021-11-16-os-stat.md b/python/change-notes/2021-11-16-os-stat.md new file mode 100644 index 00000000000..60263d31432 --- /dev/null +++ b/python/change-notes/2021-11-16-os-stat.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* Added modeling of `os.stat`, `os.lstat`, `os.statvfs`, `os.fstat`, and `os.fstatvfs`, which are new sinks for the _Uncontrolled data used in path expression_ (`py/path-injection`) query. diff --git a/python/ql/lib/semmle/python/frameworks/Stdlib.qll b/python/ql/lib/semmle/python/frameworks/Stdlib.qll index 6601b0aecae..8b2a2d86e7d 100644 --- a/python/ql/lib/semmle/python/frameworks/Stdlib.qll +++ b/python/ql/lib/semmle/python/frameworks/Stdlib.qll @@ -273,6 +273,29 @@ private module StdlibPrivate { } } + /** + * The `os` module has multiple methods for getting the status of a file, like + * a stat() system call. + * + * Note: `os.fstat` and `os.fstatvfs` operate on file-descriptors. + * + * See: + * - https://docs.python.org/3.10/library/os.html#os.stat + * - https://docs.python.org/3.10/library/os.html#os.lstat + * - https://docs.python.org/3.10/library/os.html#os.statvfs + * - https://docs.python.org/3.10/library/os.html#os.fstat + * - https://docs.python.org/3.10/library/os.html#os.fstatvfs + */ + private class OsProbingCall extends FileSystemAccess::Range, DataFlow::CallCfgNode { + OsProbingCall() { + this = os().getMember(["stat", "lstat", "statvfs", "fstat", "fstatvfs"]).getACall() + } + + override DataFlow::Node getAPathArgument() { + result in [this.getArg(0), this.getArgByName("path")] + } + } + /** * 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. diff --git a/python/ql/test/library-tests/frameworks/stdlib/FileSystemAccess.py b/python/ql/test/library-tests/frameworks/stdlib/FileSystemAccess.py index c727aa1dcce..3992fa267b8 100644 --- a/python/ql/test/library-tests/frameworks/stdlib/FileSystemAccess.py +++ b/python/ql/test/library-tests/frameworks/stdlib/FileSystemAccess.py @@ -42,3 +42,8 @@ import genericpath posixpath.exists("filepath") # $ getAPathArgument="filepath" ntpath.exists("filepath") # $ getAPathArgument="filepath" genericpath.exists("filepath") # $ getAPathArgument="filepath" + +import os + +os.stat("filepath") # $ getAPathArgument="filepath" +os.stat(path="filepath") # $ getAPathArgument="filepath"