Replace sanitizer by exclusion from sink definition

Author: owen-mc

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

@@ -116,6 +116,12 @@ module SystemCommandExecution {
 class FileSystemAccess extends DataFlow::Node instanceof FileSystemAccess::Range {
   /** Gets an argument to this file system access that is interpreted as a path. */
   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.
+   */
+  DataFlow::Node getASafePathArgument() { result = super.getASafePathArgument() }
 }
 
 /** Provides a class for modeling new file system access APIs. */
@@ -130,6 +136,12 @@ module FileSystemAccess {
   abstract class Range extends DataFlow::Node {
     /** Gets an argument to this file system access that is interpreted as a path. */
     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.
+     */
+    DataFlow::Node getASafePathArgument() { none() }
   }
 }
 

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

@@ -621,24 +621,14 @@ module Flask {
     }
 
     override DataFlow::Node getAPathArgument() {
-      result in [
-          this.getArg(0), this.getArgByName("directory"),
-          // as described in the docs, the `filename` argument is restrained to be within
-          // the provided directory, so is not exposed to path-injection. (but is still a
-          // path-argument).
-          this.getArg(1), this.getArgByName("filename")
-        ]
+      result = this.getArg([0, 1]) or
+      result = this.getArgByName(["directory", "filename"])
     }
-  }
 
-  /**
-   * To exclude `filename` argument to `flask.send_from_directory` as a path-injection sink.
-   */
-  private class FlaskSendFromDirectoryCallFilenameSanitizer extends PathInjection::Sanitizer {
-    FlaskSendFromDirectoryCallFilenameSanitizer() {
-      this = any(FlaskSendFromDirectoryCall c).getArg(1)
-      or
-      this = any(FlaskSendFromDirectoryCall c).getArgByName("filename")
+    override DataFlow::Node getASafePathArgument() {
+      // as described in the docs, the `filename` argument is restrained to be within
+      // the provided directory, so is not exposed to path-injection.
+      result in [this.getArg(1), this.getArgByName("filename")]
     }
   }
 

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

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