diff --git a/python/ql/lib/semmle/python/security/dataflow/LogInjectionCustomizations.qll b/python/ql/lib/semmle/python/security/dataflow/LogInjectionCustomizations.qll index 037ffbd0949..41990658daf 100644 --- a/python/ql/lib/semmle/python/security/dataflow/LogInjectionCustomizations.qll +++ b/python/ql/lib/semmle/python/security/dataflow/LogInjectionCustomizations.qll @@ -47,7 +47,35 @@ module LogInjection { * A logging operation, considered as a flow sink. */ class LoggingAsSink extends Sink { - LoggingAsSink() { this = any(Logging write).getAnInput() } + LoggingAsSink() { + this = any(Logging write).getAnInput() and + // since the inner implementation of the `logging.Logger.warn` function is + // ```py + // class Logger + // def warn(self, msg, *args, **kwargs): + // warnings.warn("The 'warn' method is deprecated, " + // "use 'warning' instead", DeprecationWarning, 2) + // self.warning(msg, *args, **kwargs) + // ``` + // any time we would report flow to such a logging sink, we can ALSO report + // the flow to the `self.warning` sink -- obviously we don't want that. + // + // However, simply removing taint edges out of a sink is not a good enough solution, + // since we would only flag one of the `logging.info` calls in the following example + // due to use-use flow + // ```py + // logger.warn(user_controlled) + // logger.warn(user_controlled) + // ``` + // + // The same approach is used in the command injection query. + not exists(Module loggingInit | + loggingInit.getName() = "logging.__init__" and + this.getScope().getEnclosingModule() = loggingInit and + // do allow this call if we're analyzing logging/__init__.py as part of CPython though + not exists(loggingInit.getFile().getRelativePath()) + ) + } } /** diff --git a/python/ql/test/query-tests/Security/CWE-117-LogInjection/LogInjection.expected b/python/ql/test/query-tests/Security/CWE-117-LogInjection/LogInjection.expected index 0260fbceb1c..41912469ccf 100644 --- a/python/ql/test/query-tests/Security/CWE-117-LogInjection/LogInjection.expected +++ b/python/ql/test/query-tests/Security/CWE-117-LogInjection/LogInjection.expected @@ -13,16 +13,13 @@ edges | LogInjectionBad.py:23:12:23:23 | ControlFlowNode for Attribute | LogInjectionBad.py:23:12:23:35 | ControlFlowNode for Attribute() | | LogInjectionBad.py:23:12:23:35 | ControlFlowNode for Attribute() | LogInjectionBad.py:23:5:23:8 | SSA variable name | | LogInjectionBad.py:29:5:29:8 | SSA variable name | LogInjectionBad.py:30:25:30:44 | ControlFlowNode for BinaryExpr | -| LogInjectionBad.py:29:5:29:8 | SSA variable name | LogInjectionBad.py:30:25:30:44 | ControlFlowNode for BinaryExpr | | LogInjectionBad.py:29:12:29:18 | ControlFlowNode for request | LogInjectionBad.py:29:12:29:23 | ControlFlowNode for Attribute | | LogInjectionBad.py:29:12:29:23 | ControlFlowNode for Attribute | LogInjectionBad.py:29:12:29:35 | ControlFlowNode for Attribute() | | LogInjectionBad.py:29:12:29:35 | ControlFlowNode for Attribute() | LogInjectionBad.py:29:5:29:8 | SSA variable name | -| LogInjectionBad.py:30:25:30:44 | ControlFlowNode for BinaryExpr | file:///home/rasmus/.pyenv/versions/3.9.5/lib/python3.9/logging/__init__.py:1460:20:1460:22 | ControlFlowNode for msg | | LogInjectionBad.py:35:5:35:8 | SSA variable name | LogInjectionBad.py:37:19:37:38 | ControlFlowNode for BinaryExpr | | LogInjectionBad.py:35:12:35:18 | ControlFlowNode for request | LogInjectionBad.py:35:12:35:23 | ControlFlowNode for Attribute | | LogInjectionBad.py:35:12:35:23 | ControlFlowNode for Attribute | LogInjectionBad.py:35:12:35:35 | ControlFlowNode for Attribute() | | LogInjectionBad.py:35:12:35:35 | ControlFlowNode for Attribute() | LogInjectionBad.py:35:5:35:8 | SSA variable name | -| file:///home/rasmus/.pyenv/versions/3.9.5/lib/python3.9/logging/__init__.py:1460:20:1460:22 | ControlFlowNode for msg | file:///home/rasmus/.pyenv/versions/3.9.5/lib/python3.9/logging/__init__.py:1463:22:1463:24 | ControlFlowNode for msg | nodes | LogInjectionBad.py:7:19:7:25 | ControlFlowNode for ImportMember | semmle.label | ControlFlowNode for ImportMember | | LogInjectionBad.py:7:19:7:25 | GSSA Variable request | semmle.label | GSSA Variable request | @@ -41,18 +38,14 @@ nodes | LogInjectionBad.py:29:12:29:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute | | LogInjectionBad.py:29:12:29:35 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() | | LogInjectionBad.py:30:25:30:44 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr | -| LogInjectionBad.py:30:25:30:44 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr | | LogInjectionBad.py:35:5:35:8 | SSA variable name | semmle.label | SSA variable name | | LogInjectionBad.py:35:12:35:18 | ControlFlowNode for request | semmle.label | ControlFlowNode for request | | LogInjectionBad.py:35:12:35:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute | | LogInjectionBad.py:35:12:35:35 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() | | LogInjectionBad.py:37:19:37:38 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr | -| file:///home/rasmus/.pyenv/versions/3.9.5/lib/python3.9/logging/__init__.py:1460:20:1460:22 | ControlFlowNode for msg | semmle.label | ControlFlowNode for msg | -| file:///home/rasmus/.pyenv/versions/3.9.5/lib/python3.9/logging/__init__.py:1463:22:1463:24 | ControlFlowNode for msg | semmle.label | ControlFlowNode for msg | subpaths #select | LogInjectionBad.py:18:21:18:40 | ControlFlowNode for BinaryExpr | LogInjectionBad.py:7:19:7:25 | ControlFlowNode for ImportMember | LogInjectionBad.py:18:21:18:40 | ControlFlowNode for BinaryExpr | This log entry depends on a $@. | LogInjectionBad.py:7:19:7:25 | ControlFlowNode for ImportMember | user-provided value | | LogInjectionBad.py:24:18:24:37 | ControlFlowNode for BinaryExpr | LogInjectionBad.py:7:19:7:25 | ControlFlowNode for ImportMember | LogInjectionBad.py:24:18:24:37 | ControlFlowNode for BinaryExpr | This log entry depends on a $@. | LogInjectionBad.py:7:19:7:25 | ControlFlowNode for ImportMember | user-provided value | | LogInjectionBad.py:30:25:30:44 | ControlFlowNode for BinaryExpr | LogInjectionBad.py:7:19:7:25 | ControlFlowNode for ImportMember | LogInjectionBad.py:30:25:30:44 | ControlFlowNode for BinaryExpr | This log entry depends on a $@. | LogInjectionBad.py:7:19:7:25 | ControlFlowNode for ImportMember | user-provided value | | LogInjectionBad.py:37:19:37:38 | ControlFlowNode for BinaryExpr | LogInjectionBad.py:7:19:7:25 | ControlFlowNode for ImportMember | LogInjectionBad.py:37:19:37:38 | ControlFlowNode for BinaryExpr | This log entry depends on a $@. | LogInjectionBad.py:7:19:7:25 | ControlFlowNode for ImportMember | user-provided value | -| file:///home/rasmus/.pyenv/versions/3.9.5/lib/python3.9/logging/__init__.py:1463:22:1463:24 | ControlFlowNode for msg | LogInjectionBad.py:7:19:7:25 | ControlFlowNode for ImportMember | file:///home/rasmus/.pyenv/versions/3.9.5/lib/python3.9/logging/__init__.py:1463:22:1463:24 | ControlFlowNode for msg | This log entry depends on a $@. | LogInjectionBad.py:7:19:7:25 | ControlFlowNode for ImportMember | user-provided value |