Merge pull request #7143 from RasmusWL/path-improvements

Python: Model `posixpath` and `os.stat`

Author: yoff

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.

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.

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 {
@@ -263,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.

python/ql/test/library-tests/frameworks/stdlib/FileSystemAccess.py

@@ -34,3 +34,16 @@ def through_function(open_file):
 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"
+
+import os
+
+os.stat("filepath") # $ getAPathArgument="filepath"
+os.stat(path="filepath") # $ getAPathArgument="filepath"