diff --git a/python/ql/lib/semmle/python/Concepts.qll b/python/ql/lib/semmle/python/Concepts.qll index 0ca8a4dbef0..7c90ff98dc0 100644 --- a/python/ql/lib/semmle/python/Concepts.qll +++ b/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() } } } diff --git a/python/ql/lib/semmle/python/frameworks/Flask.qll b/python/ql/lib/semmle/python/frameworks/Flask.qll index 8ac9f3deab3..eaafdf8ea23 100644 --- a/python/ql/lib/semmle/python/frameworks/Flask.qll +++ b/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")] } } diff --git a/python/ql/lib/semmle/python/security/dataflow/PathInjectionCustomizations.qll b/python/ql/lib/semmle/python/security/dataflow/PathInjectionCustomizations.qll index 269026b591b..10903fe0c51 100644 --- a/python/ql/lib/semmle/python/security/dataflow/PathInjectionCustomizations.qll +++ b/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, ...):