Skip to content

Commit 7721db2

Browse files
RasmusWLRasmusWL
committed
Python: Don't double report paths for platform.popen and popen2.*
I was a bit surprised that we hadn't double reported for popen2, but it turns out that the implementation (at least on unix) looks like: ``` def popen2(cmd, bufsize=-1, mode='t'): ... = Popen3(cmd, False, bufsize) ... ``` but since the modeling I did only considers calls to `Popen3` only if it has been imported from the `popen2` module, we don't consider that call as a sink.
1 parent 36812af commit 7721db2

File tree

2 files changed

+5
-21
lines changed

2 files changed

+5
-21
lines changed

‎python/ql/src/experimental/Security-new-dataflow/CWE-078/CommandInjection.ql‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,16 @@ class CommandInjectionConfiguration extends TaintTracking::Configuration {
4646
// os.system(cmd)
4747
// ```
4848
//
49-
// Best solution I could come up with is to exclude all sinks inside the `os` and
50-
// `subprocess` modules. This does have a downside: If we have overlooked a function
51-
// in any of these, that internally runs a command, we no longer give an alert :|
49+
// Best solution I could come up with is to exclude all sinks inside the modules of
50+
// known sinks. This does have a downside: If we have overlooked a function in any
51+
// of these, that internally runs a command, we no longer give an alert :| -- and we
52+
// need to keep them updated (which is hard to remember)
5253
//
5354
// This does not only affect `os.popen`, but also the helper functions in
5455
// `subprocess`. See:
5556
// https://github.com/python/cpython/blob/fa7ce080175f65d678a7d5756c94f82887fc9803/Lib/os.py#L974
5657
// https://github.com/python/cpython/blob/fa7ce080175f65d678a7d5756c94f82887fc9803/Lib/subprocess.py#L341
57-
not sink.getScope().getEnclosingModule().getName() in ["os", "subprocess"]
58+
not sink.getScope().getEnclosingModule().getName() in ["os", "subprocess", "platform", "popen2"]
5859
}
5960
}
6061

‎python/ql/test/experimental/query-tests/Security-new-dataflow/CWE-078-py2/CommandInjection.expected‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,36 +3,22 @@ edges
33
| command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | command_injection.py:20:15:20:27 | ControlFlowNode for BinaryExpr |
44
| command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | command_injection.py:21:15:21:27 | ControlFlowNode for BinaryExpr |
55
| command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | command_injection.py:23:20:23:32 | ControlFlowNode for BinaryExpr |
6-
| command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | command_injection.py:23:20:23:32 | ControlFlowNode for BinaryExpr |
76
| command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | command_injection.py:25:19:25:31 | ControlFlowNode for BinaryExpr |
87
| command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | command_injection.py:26:19:26:31 | ControlFlowNode for BinaryExpr |
98
| command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | command_injection.py:27:19:27:31 | ControlFlowNode for BinaryExpr |
109
| command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | command_injection.py:28:19:28:31 | ControlFlowNode for BinaryExpr |
1110
| command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | command_injection.py:29:19:29:31 | ControlFlowNode for BinaryExpr |
12-
| command_injection.py:23:20:23:32 | ControlFlowNode for BinaryExpr | file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:453:11:453:13 | SSA variable cmd |
13-
| file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:415:23:415:25 | SSA variable cmd | file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:421:19:421:41 | ControlFlowNode for BinaryExpr |
14-
| file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:453:11:453:13 | SSA variable cmd | file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:482:22:482:24 | ControlFlowNode for cmd |
15-
| file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:453:11:453:13 | SSA variable cmd | file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:482:22:482:24 | ControlFlowNode for cmd |
16-
| file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:453:11:453:13 | SSA variable cmd | file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:484:22:484:24 | ControlFlowNode for cmd |
17-
| file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:482:22:482:24 | ControlFlowNode for cmd | file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:415:23:415:25 | SSA variable cmd |
1811
nodes
1912
| command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
2013
| command_injection.py:19:15:19:27 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
2114
| command_injection.py:20:15:20:27 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
2215
| command_injection.py:21:15:21:27 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
2316
| command_injection.py:23:20:23:32 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
24-
| command_injection.py:23:20:23:32 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
2517
| command_injection.py:25:19:25:31 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
2618
| command_injection.py:26:19:26:31 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
2719
| command_injection.py:27:19:27:31 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
2820
| command_injection.py:28:19:28:31 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
2921
| command_injection.py:29:19:29:31 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
30-
| file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:415:23:415:25 | SSA variable cmd | semmle.label | SSA variable cmd |
31-
| file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:421:19:421:41 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
32-
| file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:453:11:453:13 | SSA variable cmd | semmle.label | SSA variable cmd |
33-
| file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:482:22:482:24 | ControlFlowNode for cmd | semmle.label | ControlFlowNode for cmd |
34-
| file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:482:22:482:24 | ControlFlowNode for cmd | semmle.label | ControlFlowNode for cmd |
35-
| file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:484:22:484:24 | ControlFlowNode for cmd | semmle.label | ControlFlowNode for cmd |
3622
#select
3723
| command_injection.py:19:15:19:27 | ControlFlowNode for BinaryExpr | command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | command_injection.py:19:15:19:27 | ControlFlowNode for BinaryExpr | This command depends on $@. | command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | a user-provided value |
3824
| command_injection.py:20:15:20:27 | ControlFlowNode for BinaryExpr | command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | command_injection.py:20:15:20:27 | ControlFlowNode for BinaryExpr | This command depends on $@. | command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | a user-provided value |
@@ -43,6 +29,3 @@ nodes
4329
| command_injection.py:27:19:27:31 | ControlFlowNode for BinaryExpr | command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | command_injection.py:27:19:27:31 | ControlFlowNode for BinaryExpr | This command depends on $@. | command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | a user-provided value |
4430
| command_injection.py:28:19:28:31 | ControlFlowNode for BinaryExpr | command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | command_injection.py:28:19:28:31 | ControlFlowNode for BinaryExpr | This command depends on $@. | command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | a user-provided value |
4531
| command_injection.py:29:19:29:31 | ControlFlowNode for BinaryExpr | command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | command_injection.py:29:19:29:31 | ControlFlowNode for BinaryExpr | This command depends on $@. | command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | a user-provided value |
46-
| file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:421:19:421:41 | ControlFlowNode for BinaryExpr | command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:421:19:421:41 | ControlFlowNode for BinaryExpr | This command depends on $@. | command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | a user-provided value |
47-
| file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:482:22:482:24 | ControlFlowNode for cmd | command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:482:22:482:24 | ControlFlowNode for cmd | This command depends on $@. | command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | a user-provided value |
48-
| file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:484:22:484:24 | ControlFlowNode for cmd | command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | file:///home/rasmus/.pyenv/versions/2.7.17/lib/python2.7/platform.py:484:22:484:24 | ControlFlowNode for cmd | This command depends on $@. | command_injection.py:18:13:18:24 | ControlFlowNode for Attribute | a user-provided value |

0 commit comments

Comments
 (0)