Skip to content

Commit bf9a0da

Browse files
RasmusWLRasmusWL
committed
Python: Fix stdlib sinks in LogInjection query
1 parent 7852429 commit bf9a0da

File tree

2 files changed

+29
-8
lines changed

2 files changed

+29
-8
lines changed

‎python/ql/lib/semmle/python/security/dataflow/LogInjectionCustomizations.qll

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,35 @@ module LogInjection {
4747
* A logging operation, considered as a flow sink.
4848
*/
4949
class LoggingAsSink extends Sink {
50-
LoggingAsSink() { this = any(Logging write).getAnInput() }
50+
LoggingAsSink() {
51+
this = any(Logging write).getAnInput() and
52+
// since the inner implementation of the `logging.Logger.warn` function is
53+
// ```py
54+
// class Logger
55+
// def warn(self, msg, *args, **kwargs):
56+
// warnings.warn("The 'warn' method is deprecated, "
57+
// "use 'warning' instead", DeprecationWarning, 2)
58+
// self.warning(msg, *args, **kwargs)
59+
// ```
60+
// any time we would report flow to such a logging sink, we can ALSO report
61+
// the flow to the `self.warning` sink -- obviously we don't want that.
62+
//
63+
// However, simply removing taint edges out of a sink is not a good enough solution,
64+
// since we would only flag one of the `logging.info` calls in the following example
65+
// due to use-use flow
66+
// ```py
67+
// logger.warn(user_controlled)
68+
// logger.warn(user_controlled)
69+
// ```
70+
//
71+
// The same approach is used in the command injection query.
72+
not exists(Module loggingInit |
73+
loggingInit.getName() = "logging.__init__" and
74+
this.getScope().getEnclosingModule() = loggingInit and
75+
// do allow this call if we're analyzing logging/__init__.py as part of CPython though
76+
not exists(loggingInit.getFile().getRelativePath())
77+
)
78+
}
5179
}
5280

5381
/**

‎python/ql/test/query-tests/Security/CWE-117-LogInjection/LogInjection.expected

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,13 @@ edges
1313
| LogInjectionBad.py:23:12:23:23 | ControlFlowNode for Attribute | LogInjectionBad.py:23:12:23:35 | ControlFlowNode for Attribute() |
1414
| LogInjectionBad.py:23:12:23:35 | ControlFlowNode for Attribute() | LogInjectionBad.py:23:5:23:8 | SSA variable name |
1515
| LogInjectionBad.py:29:5:29:8 | SSA variable name | LogInjectionBad.py:30:25:30:44 | ControlFlowNode for BinaryExpr |
16-
| LogInjectionBad.py:29:5:29:8 | SSA variable name | LogInjectionBad.py:30:25:30:44 | ControlFlowNode for BinaryExpr |
1716
| LogInjectionBad.py:29:12:29:18 | ControlFlowNode for request | LogInjectionBad.py:29:12:29:23 | ControlFlowNode for Attribute |
1817
| LogInjectionBad.py:29:12:29:23 | ControlFlowNode for Attribute | LogInjectionBad.py:29:12:29:35 | ControlFlowNode for Attribute() |
1918
| LogInjectionBad.py:29:12:29:35 | ControlFlowNode for Attribute() | LogInjectionBad.py:29:5:29:8 | SSA variable name |
20-
| 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 |
2119
| LogInjectionBad.py:35:5:35:8 | SSA variable name | LogInjectionBad.py:37:19:37:38 | ControlFlowNode for BinaryExpr |
2220
| LogInjectionBad.py:35:12:35:18 | ControlFlowNode for request | LogInjectionBad.py:35:12:35:23 | ControlFlowNode for Attribute |
2321
| LogInjectionBad.py:35:12:35:23 | ControlFlowNode for Attribute | LogInjectionBad.py:35:12:35:35 | ControlFlowNode for Attribute() |
2422
| LogInjectionBad.py:35:12:35:35 | ControlFlowNode for Attribute() | LogInjectionBad.py:35:5:35:8 | SSA variable name |
25-
| 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 |
2623
nodes
2724
| LogInjectionBad.py:7:19:7:25 | ControlFlowNode for ImportMember | semmle.label | ControlFlowNode for ImportMember |
2825
| LogInjectionBad.py:7:19:7:25 | GSSA Variable request | semmle.label | GSSA Variable request |
@@ -41,18 +38,14 @@ nodes
4138
| LogInjectionBad.py:29:12:29:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
4239
| LogInjectionBad.py:29:12:29:35 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
4340
| LogInjectionBad.py:30:25:30:44 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
44-
| LogInjectionBad.py:30:25:30:44 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
4541
| LogInjectionBad.py:35:5:35:8 | SSA variable name | semmle.label | SSA variable name |
4642
| LogInjectionBad.py:35:12:35:18 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
4743
| LogInjectionBad.py:35:12:35:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
4844
| LogInjectionBad.py:35:12:35:35 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
4945
| LogInjectionBad.py:37:19:37:38 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
50-
| 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 |
51-
| 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 |
5246
subpaths
5347
#select
5448
| 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 |
5549
| 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 |
5650
| 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 |
5751
| 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 |
58-
| 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 |

0 commit comments

Comments
 (0)