Python: Add FileSystemWriteAccess concept

I made `FileSystemWriteAccess` be a subclass of `FileSystemAccess` (like in [JS](https://github.com/github/codeql/blob/64001cc02cd67cc5df003199954be0d4135b3eb6/javascript/ql/src/semmle/javascript/Concepts.qll#L68-L74)), but then I started wondering about how I could  give a good result for `getAPathArgument`, and what would a good result even be? The argument to the `open` call, or the object that the `write` method is called on? I can't see how doing either of these enables us to do anything useful...

So I looked closer at how JS uses `FileSystemWriteAccess`:

1. as sink for zip-slip: https://github.com/github/codeql/blob/7c51dff0f781a0ca1efe32ef449b7b1b88cf0548/javascript/ql/src/semmle/javascript/security/dataflow/ZipSlipCustomizations.qll#L121
2. as sink for downloading unsafe files (identified through their extension) through non-secure connections: https://github.com/github/codeql/blob/89ef6ea4eb1d7f606380fc9cd904ac2b540c8eae/javascript/ql/src/semmle/javascript/security/dataflow/InsecureDownloadCustomizations.qll#L134-L150
3. as sink for writing untrusted data to a local file  https://github.com/github/codeql/blob/93b1e59d62a596fe40ebce13ba2dd8e6117b25fd/javascript/ql/src/semmle/javascript/security/dataflow/HttpToFileAccessCustomizations.qll#L43-L46

for the 2 first sinks, it's important that `getAPathArgument` has a proper result... so that solves the problem, and highlights that it _can_ be important to give proper results for `getAPathArgument` (if possible).

So I'm trying to do best effort for `f = open(...); f.write(...)`, but with this current code we won't always be able to give a result (as highlighted by the tests). It will also be the case that there are multiple `FileSystemAccess` with the same path-argument, which could be a little strange.

overall, I'm not super confident about the way this new concept and implementation turned out, but it also seems like the best I could come up with right now...

The obvious alternative solution is to NOT make `FileSystemWriteAccess` a subclass of `FileSystemAccess`, but I'm not very tempted to go down this path, given the examples of this being useful above, and just the general notion that we should be able to model writes as being a specialized kind of `FileSystemAccess`.

Author: RasmusWL

python/ql/src/semmle/python/Concepts.qll

@@ -72,6 +72,39 @@ module FileSystemAccess {
   }
 }
 
+/**
+ * A data flow node that writes data to the file system access.
+ *
+ * Extend this class to refine existing API models. If you want to model new APIs,
+ * extend `FileSystemWriteAccess::Range` instead.
+ */
+class FileSystemWriteAccess extends FileSystemAccess {
+  override FileSystemWriteAccess::Range range;
+
+  /**
+   * Gets a node that represents data to be written to the file system (possibly with
+   * some transformation happening before it is written, like JSON encoding).
+   */
+  DataFlow::Node getADataNode() { result = range.getADataNode() }
+}
+
+/** Provides a class for modeling new file system writes. */
+module FileSystemWriteAccess {
+  /**
+   * A data flow node that writes data to the file system access.
+   *
+   * Extend this class to model new APIs. If you want to refine existing API models,
+   * extend `FileSystemWriteAccess` instead.
+   */
+  abstract class Range extends FileSystemAccess::Range {
+    /**
+     * Gets a node that represents data to be written to the file system (possibly with
+     * some transformation happening before it is written, like JSON encoding).
+     */
+    abstract DataFlow::Node getADataNode();
+  }
+}
+
 /** Provides classes for modeling path-related APIs. */
 module Path {
   /**

python/ql/src/semmle/python/frameworks/Stdlib.qll

@@ -383,23 +383,45 @@ private module Stdlib {
     }
   }
 
+  /** Gets a reference to the builtin `open` function */
+  private API::Node getOpenFunctionRef() {
+    result = API::builtin("open")
+    or
+    // io.open is a special case, since it is an alias for the builtin `open`
+    result = API::moduleImport("io").getMember("open")
+  }
+
   /**
    * A call to the builtin `open` function.
    * See https://docs.python.org/3/library/functions.html#open
    */
   private class OpenCall extends FileSystemAccess::Range, DataFlow::CallCfgNode {
-    OpenCall() {
-      this = API::builtin("open").getACall()
-      or
-      // io.open is a special case, since it is an alias for the builtin `open`
-      this = API::moduleImport("io").getMember("open").getACall()
-    }
+    OpenCall() { this = getOpenFunctionRef().getACall() }
 
     override DataFlow::Node getAPathArgument() {
       result in [this.getArg(0), this.getArgByName("file")]
     }
   }
 
+  /** A call to the `write` or `writelines` method on an opened file, such as `open("foo", "w").write(...)`. */
+  private class WriteOnOpenFile extends FileSystemWriteAccess::Range, DataFlow::CallCfgNode {
+    WriteOnOpenFile() {
+      this = getOpenFunctionRef().getReturn().getMember(["write", "writelines"]).getACall()
+    }
+
+    override DataFlow::Node getAPathArgument() {
+      // best effort attempt to give the path argument, that was initially given to the `open` call.
+      exists(OpenCall openCall, DataFlow::AttrRead read |
+        read.getAttributeName() in ["write", "writelines"] and
+        openCall.flowsTo(read.getObject()) and
+        read.(DataFlow::LocalSourceNode).flowsTo(this.getFunction()) and
+        result = openCall.getAPathArgument()
+      )
+    }
+
+    override DataFlow::Node getADataNode() { result in [this.getArg(0), this.getArgByName("data")] }
+  }
+
   /**
    * An exec statement (only Python 2).
    * See https://docs.python.org/2/reference/simple_stmts.html#the-exec-statement.
@@ -1001,11 +1023,14 @@ private module Stdlib {
   /** Gets a reference to a `pathlib.Path` object. */
   DataFlow::LocalSourceNode pathlibPath() { result = pathlibPath(DataFlow::TypeTracker::end()) }
 
+  /** A file system access from a `pathlib.Path` method call. */
   private class PathlibFileAccess extends FileSystemAccess::Range, DataFlow::CallCfgNode {
     DataFlow::AttrRead fileAccess;
+    string attrbuteName;
 
     PathlibFileAccess() {
-      fileAccess.getAttributeName() in [
+      attrbuteName = fileAccess.getAttributeName() and
+      attrbuteName in [
           "stat", "chmod", "exists", "expanduser", "glob", "group", "is_dir", "is_file", "is_mount",
           "is_symlink", "is_socket", "is_fifo", "is_block_device", "is_char_device", "iter_dir",
           "lchmod", "lstat", "mkdir", "open", "owner", "read_bytes", "read_text", "readlink",
@@ -1019,6 +1044,13 @@ private module Stdlib {
     override DataFlow::Node getAPathArgument() { result = fileAccess.getObject() }
   }
 
+  /** A file system write from a `pathlib.Path` method call. */
+  private class PathlibFileWrites extends PathlibFileAccess, FileSystemWriteAccess::Range {
+    PathlibFileWrites() { attrbuteName in ["write_bytes", "write_text"] }
+
+    override DataFlow::Node getADataNode() { result in [this.getArg(0), this.getArgByName("data")] }
+  }
+
   /** An additional taint steps for objects of type `pathlib.Path` */
   private class PathlibPathTaintStep extends TaintTracking::AdditionalTaintStep {
     override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {

python/ql/test/experimental/meta/ConceptsTest.qll

@@ -269,6 +269,23 @@ class FileSystemAccessTest extends InlineExpectationsTest {
   }
 }
 
+class FileSystemWriteAccessTest extends InlineExpectationsTest {
+  FileSystemWriteAccessTest() { this = "FileSystemWriteAccessTest" }
+
+  override string getARelevantTag() { result = "fileWriteData" }
+
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    exists(location.getFile().getRelativePath()) and
+    exists(FileSystemWriteAccess write, DataFlow::Node data |
+      data = write.getADataNode() and
+      location = data.getLocation() and
+      element = data.toString() and
+      value = prettyNodeForInlineTest(data) and
+      tag = "fileWriteData"
+    )
+  }
+}
+
 class PathNormalizationTest extends InlineExpectationsTest {
   PathNormalizationTest() { this = "PathNormalizationTest" }
 

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

@@ -11,11 +11,13 @@
 with p.open() as f:  # $ getAPathArgument=p
     f.read()
 
-p.write_bytes(b"hello")  # $ getAPathArgument=p
+p.write_bytes(b"hello")  # $ getAPathArgument=p fileWriteData=b"hello"
+p.write_text("hello")  # $ getAPathArgument=p fileWriteData="hello"
+p.open("wt").write("hello")  # $ getAPathArgument=p MISSING: fileWriteData="hello"
 
 name = windows.parent.name
 o = open
 o(name)  # $ getAPathArgument=name
 
 wb = p.write_bytes
-wb(b"hello")  # $ getAPathArgument=p
+wb(b"hello")  # $ getAPathArgument=p fileWriteData=b"hello"

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

@@ -16,3 +16,14 @@
 
 io.open("filepath")  # $ getAPathArgument="filepath"
 io.open(file="filepath")  # $ getAPathArgument="filepath"
+
+f = open("path") # $ getAPathArgument="path"
+f.write("foo") # $ getAPathArgument="path" fileWriteData="foo"
+lines = ["foo"]
+f.writelines(lines) # $ getAPathArgument="path" fileWriteData=lines
+
+
+def through_function(open_file):
+    open_file.write("foo") # $ fileWriteData="foo" MISSING: getAPathArgument="path"
+
+through_function(f)