Skip to content

Commit eb67986

Browse files
committed
Python: Exlucde only command injection sinks in os and subprocess
1 parent 68eacef commit eb67986

File tree

1 file changed

+10
-9
lines changed

1 file changed

+10
-9
lines changed

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,25 +35,26 @@ class CommandInjectionConfiguration extends TaintTracking::Configuration {
3535
// proc = subprocess.Popen(cmd, ...)
3636
// ```
3737
// any time we would report flow to the `os.popen` sink, we can ALSO report the flow
38-
// from the `cmd` parameter to the `subprocess.Popen` sink -- obviously we don't want
39-
// that.
38+
// from the `cmd` parameter to the `subprocess.Popen` sink -- obviously we don't
39+
// want that.
4040
//
4141
// However, simply removing taint edges out of a sink is not a good enough solution,
42-
// since we would only flag one of the `os.system` calls in the following example due
43-
// to use-use flow
42+
// since we would only flag one of the `os.system` calls in the following example
43+
// due to use-use flow
4444
// ```py
4545
// os.system(cmd)
4646
// os.system(cmd)
4747
// ```
4848
//
49-
// Best solution I could come up with is to exclude all sinks inside the standard
50-
// library -- this does have a downside: If we have overlooked a function in the
51-
// standard library 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 `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 :|
5252
//
53-
// This does not only affect `os.popen`, but also the helper functions in `subprocess`. See
53+
// This does not only affect `os.popen`, but also the helper functions in
54+
// `subprocess`. See:
5455
// https://github.com/python/cpython/blob/fa7ce080175f65d678a7d5756c94f82887fc9803/Lib/os.py#L974
5556
// https://github.com/python/cpython/blob/fa7ce080175f65d678a7d5756c94f82887fc9803/Lib/subprocess.py#L341
56-
not sink.getLocation().getFile().inStdlib()
57+
not sink.getScope().getEnclosingModule().getName() in ["os", "subprocess"]
5758
}
5859
}
5960

0 commit comments

Comments
 (0)