Merge pull request #8138 from github/ruby/file-write

Ruby: Implement `FileSystemWriteAccess` concept

ruby/ql/lib/change-notes/2022-02-20-file-system-write-access.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added `FileSystemWriteAccess` concept to model data written to the filesystem.

ruby/ql/lib/codeql/ruby/Concepts.qll

@@ -92,6 +92,35 @@ module FileSystemReadAccess {
   }
 }
 
+/**
+ * A data flow node that writes data to the file system.
+ *
+ * Extend this class to refine existing API models. If you want to model new APIs,
+ * extend `FileSystemWriteAccess::Range` instead.
+ */
+class FileSystemWriteAccess extends FileSystemAccess instanceof FileSystemWriteAccess::Range {
+  /**
+   * Gets a node that represents data written to the file system by this access.
+   */
+  DataFlow::Node getADataNode() { result = FileSystemWriteAccess::Range.super.getADataNode() }
+}
+
+/** Provides a class for modeling new file system writes. */
+module FileSystemWriteAccess {
+  /**
+   * A data flow node that writes data to the file system.
+   *
+   * 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 written to the file system by this access.
+     */
+    abstract DataFlow::Node getADataNode();
+  }
+}
+
 /**
  * A data flow node that sets the permissions for one or more files.
  *

ruby/ql/lib/codeql/ruby/frameworks/Files.qll

@@ -52,20 +52,110 @@ private DataFlow::Node fileInstance() {
   )
 }
 
-private string ioReaderClassMethodName() { result = ["binread", "foreach", "read", "readlines"] }
+abstract private class IOOrFileMethodCall extends DataFlow::CallNode {
+  // TODO: Currently this only handles class method calls.
+  // Can we infer a path argument for instance method calls?
+  // e.g. by tracing back to the instantiation of that instance
+  DataFlow::Node getAPathArgumentImpl() {
+    result = this.getArgument(0) and this.getReceiverKind() = "class"
+  }
 
-private string ioReaderInstanceMethodName() {
-  result =
-    [
-      "getbyte", "getc", "gets", "pread", "read", "read_nonblock", "readbyte", "readchar",
-      "readline", "readlines", "readpartial", "sysread"
-    ]
+  /**
+   * Holds if this call appears to read/write from/to a spawned subprocess,
+   * rather than to/from a file.
+   */
+  predicate spawnsSubprocess() {
+    pathArgSpawnsSubprocess(this.getAPathArgumentImpl().asExpr().getExpr())
+  }
+
+  /** Gets the API used to perform this call, either "IO" or "File" */
+  abstract string getAPI();
+
+  /** Gets a node representing the data read or written by this call */
+  abstract DataFlow::Node getADataNodeImpl();
+
+  /** Gets a string representation of the receiver kind, either "class" or "instance". */
+  abstract string getReceiverKind();
 }
 
-private string ioReaderMethodName(string receiverKind) {
-  receiverKind = "class" and result = ioReaderClassMethodName()
-  or
-  receiverKind = "instance" and result = ioReaderInstanceMethodName()
+/**
+ * A method call that performs a read using either the `IO` or `File` classes.
+ */
+private class IOOrFileReadMethodCall extends IOOrFileMethodCall {
+  private string api;
+  private string receiverKind;
+
+  IOOrFileReadMethodCall() {
+    exists(string methodName | methodName = this.getMethodName() |
+      // e.g. `{IO,File}.readlines("foo.txt")`
+      receiverKind = "class" and
+      methodName = ["binread", "foreach", "read", "readlines"] and
+      api = ["IO", "File"] and
+      this = API::getTopLevelMember(api).getAMethodCall(methodName)
+      or
+      // e.g. `{IO,File}.new("foo.txt", "r").getc`
+      receiverKind = "interface" and
+      (
+        methodName =
+          [
+            "getbyte", "getc", "gets", "pread", "read", "read_nonblock", "readbyte", "readchar",
+            "readline", "readlines", "readpartial", "sysread"
+          ] and
+        (
+          this.getReceiver() = ioInstance() and api = "IO"
+          or
+          this.getReceiver() = fileInstance() and api = "File"
+        )
+      )
+    )
+  }
+
+  override string getAPI() { result = api }
+
+  override DataFlow::Node getADataNodeImpl() { result = this }
+
+  override string getReceiverKind() { result = receiverKind }
+}
+
+/**
+ * A method call that performs a write using either the `IO` or `File` classes.
+ */
+private class IOOrFileWriteMethodCall extends IOOrFileMethodCall {
+  private string api;
+  private string receiverKind;
+  private DataFlow::Node dataNode;
+
+  IOOrFileWriteMethodCall() {
+    exists(string methodName | methodName = this.getMethodName() |
+      // e.g. `{IO,File}.write("foo.txt", "hello\n")`
+      receiverKind = "class" and
+      api = ["IO", "File"] and
+      this = API::getTopLevelMember(api).getAMethodCall(methodName) and
+      methodName = ["binwrite", "write"] and
+      dataNode = this.getArgument(1)
+      or
+      // e.g. `{IO,File}.new("foo.txt", "a+).puts("hello")`
+      receiverKind = "interface" and
+      (
+        this.getReceiver() = ioInstance() and api = "IO"
+        or
+        this.getReceiver() = fileInstance() and api = "File"
+      ) and
+      (
+        methodName = ["<<", "print", "putc", "puts", "syswrite", "pwrite", "write_nonblock"] and
+        dataNode = this.getArgument(0)
+        or
+        // Any argument to these methods may be written as data
+        methodName = ["printf", "write"] and dataNode = this.getArgument(_)
+      )
+    )
+  }
+
+  override string getAPI() { result = api }
+
+  override DataFlow::Node getADataNodeImpl() { result = dataNode }
+
+  override string getReceiverKind() { result = receiverKind }
 }
 
 /**
@@ -111,31 +201,31 @@ module IO {
    * This class includes only reads that use the `IO` class directly, not those
    * that use a subclass of `IO` such as `File`.
    */
-  class IOReader extends DataFlow::CallNode {
-    private string receiverKind;
-
-    IOReader() {
-      // `IO` class method calls
-      receiverKind = "class" and
-      this = API::getTopLevelMember("IO").getAMethodCall(ioReaderMethodName(receiverKind))
-      or
-      // `IO` instance method calls
-      receiverKind = "instance" and
-      exists(IOInstanceStrict ii |
-        this.getReceiver() = ii and
-        this.getMethodName() = ioReaderMethodName(receiverKind)
-      )
-      // TODO: enumeration style methods such as `each`, `foreach`, etc.
-    }
-
-    /**
-     * Gets a string representation of the receiver kind, either "class" or "instance".
-     */
-    string getReceiverKind() { result = receiverKind }
+  class IOReader extends IOOrFileReadMethodCall {
+    IOReader() { this.getAPI() = "IO" }
   }
 
   /**
-   * A `DataFlow::CallNode` that reads data from the filesystem using the `IO`
+   * A `DataFlow::CallNode` that writes data using the `IO` class. For example,
+   * the `write` and `puts` calls in:
+   *
+   * ```rb
+   * # writes the string `hello world` to the file `foo.txt`
+   * IO.write("foo.txt", "hello world")
+   *
+   * # appends the string `hello again\n` to the file `foo.txt`
+   * IO.new(IO.sysopen("foo.txt", "a")).puts("hello again")
+   * ```
+   *
+   * This class includes only writes that use the `IO` class directly, not those
+   * that use a subclass of `IO` such as `File`.
+   */
+  class IOWriter extends IOOrFileWriteMethodCall {
+    IOWriter() { this.getAPI() = "IO" }
+  }
+
+  /**
+   * A `DataFlow::CallNode` that reads data to the filesystem using the `IO`
    * or `File` classes. For example, the `IO.read` and `File#readline` calls in:
    *
    * ```rb
@@ -146,46 +236,32 @@ module IO {
    * File.new("foo.txt").readline
    * ```
    */
-  class FileReader extends DataFlow::CallNode, FileSystemReadAccess::Range {
-    private string receiverKind;
-    private string api;
+  class FileReader extends IOOrFileReadMethodCall, FileSystemReadAccess::Range {
+    FileReader() { not this.spawnsSubprocess() }
 
-    FileReader() {
-      // A viable `IOReader` that could feasibly read from the filesystem
-      api = "IO" and
-      receiverKind = this.(IOReader).getReceiverKind() and
-      not pathArgSpawnsSubprocess(this.getArgument(0).asExpr().getExpr())
-      or
-      api = "File" and
-      (
-        // `File` class method calls
-        receiverKind = "class" and
-        this = API::getTopLevelMember(api).getAMethodCall(ioReaderMethodName(receiverKind))
-        or
-        // `File` instance method calls
-        receiverKind = "instance" and
-        exists(File::FileInstance fi |
-          this.getReceiver() = fi and
-          this.getMethodName() = ioReaderMethodName(receiverKind)
-        )
-      )
-      // TODO: enumeration style methods such as `each`, `foreach`, etc.
-    }
+    override DataFlow::Node getADataNode() { result = this.getADataNodeImpl() }
 
-    // TODO: Currently this only handles class method calls.
-    // Can we infer a path argument for instance method calls?
-    // e.g. by tracing back to the instantiation of that instance
-    override DataFlow::Node getAPathArgument() {
-      result = this.getArgument(0) and receiverKind = "class"
-    }
+    override DataFlow::Node getAPathArgument() { result = this.getAPathArgumentImpl() }
+  }
 
-    // This class represents calls that return data
-    override DataFlow::Node getADataNode() { result = this }
+  /**
+   * A `DataFlow::CallNode` that reads data from the filesystem using the `IO`
+   * or `File` classes. For example, the `write` and `puts` calls in:
+   *
+   * ```rb
+   * # writes the string `hello world` to the file `foo.txt`
+   * IO.write("foo.txt", "hello world")
+   *
+   * # appends the string `hello again\n` to the file `foo.txt`
+   * File.new("foo.txt", "a").puts("hello again")
+   * ```
+   */
+  class FileWriter extends IOOrFileWriteMethodCall, FileSystemWriteAccess::Range {
+    FileWriter() { not this.spawnsSubprocess() }
 
-    /**
-     * Returns the most specific core class used for this read, `IO` or `File`
-     */
-    string getAPI() { result = api }
+    override DataFlow::Node getADataNode() { result = this.getADataNodeImpl() }
+
+    override DataFlow::Node getAPathArgument() { result = this.getAPathArgumentImpl() }
   }
 }
 
@@ -231,6 +307,10 @@ module File {
    */
   class FileModuleReader extends IO::FileReader {
     FileModuleReader() { this.getAPI() = "File" }
+
+    override DataFlow::Node getADataNode() { result = this.getADataNodeImpl() }
+
+    override DataFlow::Node getAPathArgument() { result = this.getAPathArgumentImpl() }
   }
 
   /**

ruby/ql/test/library-tests/frameworks/files/Files.expected

@@ -1,7 +1,7 @@
 fileInstances
-| Files.rb:2:1:2:30 | ... = ... |
-| Files.rb:2:1:2:30 | ... = ... |
-| Files.rb:2:12:2:30 | call to new |
+| Files.rb:2:1:2:36 | ... = ... |
+| Files.rb:2:1:2:36 | ... = ... |
+| Files.rb:2:12:2:36 | call to new |
 | Files.rb:3:1:3:21 | ... = ... |
 | Files.rb:3:1:3:21 | ... = ... |
 | Files.rb:3:14:3:21 | foo_file |
@@ -15,10 +15,12 @@ fileInstances
 | Files.rb:24:19:24:40 | call to open |
 | Files.rb:37:1:37:33 | ... = ... |
 | Files.rb:37:14:37:33 | call to open |
+| Files.rb:40:1:40:8 | foo_file |
+| Files.rb:41:1:41:26 | call to open |
 ioInstances
-| Files.rb:2:1:2:30 | ... = ... |
-| Files.rb:2:1:2:30 | ... = ... |
-| Files.rb:2:12:2:30 | call to new |
+| Files.rb:2:1:2:36 | ... = ... |
+| Files.rb:2:1:2:36 | ... = ... |
+| Files.rb:2:12:2:36 | call to new |
 | Files.rb:3:1:3:21 | ... = ... |
 | Files.rb:3:1:3:21 | ... = ... |
 | Files.rb:3:14:3:21 | foo_file |
@@ -40,6 +42,12 @@ ioInstances
 | Files.rb:35:13:35:56 | call to open |
 | Files.rb:37:1:37:33 | ... = ... |
 | Files.rb:37:14:37:33 | call to open |
+| Files.rb:40:1:40:8 | foo_file |
+| Files.rb:41:1:41:26 | call to open |
+| Files.rb:44:1:44:45 | ... = ... |
+| Files.rb:44:1:44:45 | ... = ... |
+| Files.rb:44:11:44:45 | call to open |
+| Files.rb:48:1:48:7 | io_file |
 fileModuleReaders
 | Files.rb:7:13:7:32 | call to readlines |
 ioReaders
@@ -64,7 +72,21 @@ fileSystemAccesses
 | Files.rb:20:13:20:25 | call to read |
 | Files.rb:29:12:29:29 | call to read |
 | Files.rb:37:14:37:33 | call to open |
+| Files.rb:40:1:40:22 | call to puts |
+| Files.rb:41:1:41:26 | call to open |
+| Files.rb:41:1:41:43 | call to write |
+| Files.rb:48:1:48:40 | call to printf |
 fileNameSources
 | Files.rb:10:6:10:18 | call to path |
 | Files.rb:11:6:11:21 | call to to_path |
 | Files.rb:14:8:14:43 | call to makedirs |
+ioWriters
+| Files.rb:48:1:48:40 | call to printf |
+fileWriters
+| Files.rb:40:1:40:22 | call to puts |
+| Files.rb:41:1:41:43 | call to write |
+| Files.rb:48:1:48:40 | call to printf |
+fileSystemWriteAccesses
+| Files.rb:40:1:40:22 | call to puts |
+| Files.rb:41:1:41:43 | call to write |
+| Files.rb:48:1:48:40 | call to printf |

ruby/ql/test/library-tests/frameworks/files/Files.ql

@@ -21,3 +21,9 @@ query predicate fileSystemReadAccesses(FileSystemReadAccess a) { any() }
 query predicate fileSystemAccesses(FileSystemAccess a) { any() }
 
 query predicate fileNameSources(FileNameSource s) { any() }
+
+query predicate ioWriters(IO::IOWriter r) { any() }
+
+query predicate fileWriters(IO::FileWriter r) { any() }
+
+query predicate fileSystemWriteAccesses(FileSystemWriteAccess a) { any() }

ruby/ql/test/library-tests/frameworks/files/Files.rb

@@ -1,5 +1,5 @@
 # `foo_file` is a `File` instance
-foo_file = File.new("foo.txt")
+foo_file = File.new("foo.txt", "a+")
 foo_file_2 = foo_file
 foo_file
 
@@ -34,4 +34,15 @@ date = IO.read("|date")
 # `rand_open` is an `IO` instance
 rand_open = IO.open(IO.sysopen("/dev/random", "r"), "r")
 
-foo_file_3 = File.open("foo.txt")
+foo_file_3 = File.open("foo.txt")
+
+# File write accesses
+foo_file.puts("hello")
+File.open("foo.txt", "a+").write("world\n")
+
+# IO instance
+io_file = IO.open(IO.sysopen("foo.txt", "w"))
+str_1 = "hello"
+int_1 = 123
+# File/IO write
+io_file.printf("%s: %d\n", str_1, int_1)