Specify vulnerable args instead of safe ones

Author: owen-mc

python/ql/lib/semmle/python/Concepts.qll

@@ -118,10 +118,14 @@ class FileSystemAccess extends DataFlow::Node instanceof FileSystemAccess::Range
   DataFlow::Node getAPathArgument() { result = super.getAPathArgument() }
 
   /**
-   * Gets an argument to this file system access that is interpreted as a path,
-   * but which is not vulnerable to path injection.
+   * Gets an argument to this file system access that is interpreted as a path
+   * which is vulnerable to path injection.
+   *
+   * By default all path arguments are considered vulnerable, but this can be overridden to
+   * exclude certain arguments that are known to be safe, for example because they are
+   * restricted to a specific directory.
    */
-  DataFlow::Node getASafePathArgument() { result = super.getASafePathArgument() }
+  DataFlow::Node getAVulnerablePathArgument() { result = super.getAVulnerablePathArgument() }
 }
 
 /** Provides a class for modeling new file system access APIs. */
@@ -138,10 +142,14 @@ module FileSystemAccess {
     abstract DataFlow::Node getAPathArgument();
 
     /**
-     * Gets an argument to this file system access that is interpreted as a path,
-     * but which is not vulnerable to path injection.
+     * Gets an argument to this file system access that is interpreted as a path
+     * which is vulnerable to path injection.
+     *
+     * By default all path arguments are considered vulnerable, but this can be overridden to
+     * exclude certain arguments that are known to be safe, for example because they are
+     * restricted to a specific directory.
      */
-    DataFlow::Node getASafePathArgument() { none() }
+    DataFlow::Node getAVulnerablePathArgument() { result = this.getAPathArgument() }
   }
 }
 

python/ql/lib/semmle/python/frameworks/Flask.qll

@@ -625,10 +625,11 @@ module Flask {
       result = this.getArgByName(["directory", "filename"])
     }
 
-    override DataFlow::Node getASafePathArgument() {
-      // as described in the docs, the `filename` argument is restrained to be within
+    override DataFlow::Node getAVulnerablePathArgument() {
+      result = this.getAPathArgument() and
+      // as described in the docs, the `filename` argument is restricted to be within
       // the provided directory, so is not exposed to path-injection.
-      result in [this.getArg(1), this.getArgByName("filename")]
+      not result in [this.getArg(1), this.getArgByName("filename")]
     }
   }
 

python/ql/lib/semmle/python/security/dataflow/PathInjectionCustomizations.qll

@@ -57,8 +57,7 @@ module PathInjection {
    */
   class FileSystemAccessAsSink extends Sink {
     FileSystemAccessAsSink() {
-      this = any(FileSystemAccess e).getAPathArgument() and
-      not this = any(FileSystemAccess e).getASafePathArgument() and
+      this = any(FileSystemAccess e).getAVulnerablePathArgument() and
       // since implementation of Path.open in pathlib.py is like
       // ```py
       // def open(self, ...):