diff --git a/python/ql/lib/semmle/python/Concepts.qll b/python/ql/lib/semmle/python/Concepts.qll index 7c90ff98dc0..1c018566cbf 100644 --- a/python/ql/lib/semmle/python/Concepts.qll +++ b/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() } } } diff --git a/python/ql/lib/semmle/python/frameworks/Flask.qll b/python/ql/lib/semmle/python/frameworks/Flask.qll index eaafdf8ea23..b9bba675ac0 100644 --- a/python/ql/lib/semmle/python/frameworks/Flask.qll +++ b/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")] } } diff --git a/python/ql/lib/semmle/python/security/dataflow/PathInjectionCustomizations.qll b/python/ql/lib/semmle/python/security/dataflow/PathInjectionCustomizations.qll index 10903fe0c51..7121faa19ff 100644 --- a/python/ql/lib/semmle/python/security/dataflow/PathInjectionCustomizations.qll +++ b/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, ...):