Skip to content

Commit 8ab4dae

Browse files
markshannonmarkshannon
authored
Merge pull request #1150 from taus-semmle/python-fix-insecure-default-protocol-fp
Python: Fix false positive for `py/insecure-default-protocol`.
2 parents d056af3 + 5eb63ae commit 8ab4dae

File tree

4 files changed

+29
-18
lines changed

4 files changed

+29
-18
lines changed

‎python/ql/src/Security/CWE-327/InsecureDefaultProtocol.ql‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,18 @@ ClassObject ssl_Context_class() {
2222

2323
CallNode unsafe_call(string method_name) {
2424
result = ssl_wrap_socket().getACall() and
25+
not exists(result.getArgByName("ssl_version")) and
2526
method_name = "deprecated method ssl.wrap_socket"
2627
or
2728
result = ssl_Context_class().getACall() and
29+
not exists(result.getArgByName("protocol")) and
30+
not exists(result.getArg(0)) and
2831
method_name = "ssl.SSLContext"
2932
}
3033

3134
from CallNode call, string method_name
3235
where
33-
call = unsafe_call(method_name) and
34-
not exists(call.getArgByName("ssl_version"))
36+
call = unsafe_call(method_name)
3537
select call, "Call to " + method_name + " does not specify a protocol, which may result in an insecure default being used."
3638

3739

‎python/ql/src/Security/CWE-327/InsecureProtocol.ql‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,12 @@ private ModuleObject the_pyOpenSSL_module() {
4545
* a protocol constant, e.g. if it has been removed in later versions of the `ssl`
4646
* library.
4747
*/
48-
predicate probable_insecure_ssl_constant(CallNode call, string insecure_version) {
49-
exists(ControlFlowNode arg | arg = call.getArgByName("ssl_version") |
48+
bindingset[named_argument]
49+
predicate probable_insecure_ssl_constant(CallNode call, string insecure_version, string named_argument) {
50+
exists(ControlFlowNode arg |
51+
arg = call.getArgByName(named_argument) or
52+
arg = call.getArg(0)
53+
|
5054
arg.(AttrNode).getObject(insecure_version).refersTo(the_ssl_module())
5155
or
5256
arg.(NameNode).getId() = insecure_version and
@@ -57,21 +61,23 @@ predicate probable_insecure_ssl_constant(CallNode call, string insecure_version)
5761
)
5862
}
5963

60-
predicate unsafe_ssl_wrap_socket_call(CallNode call, string method_name, string insecure_version) {
64+
predicate unsafe_ssl_wrap_socket_call(CallNode call, string method_name, string insecure_version, string named_argument) {
6165
(
6266
call = ssl_wrap_socket().getACall() and
63-
method_name = "deprecated method ssl.wrap_socket"
67+
method_name = "deprecated method ssl.wrap_socket" and
68+
named_argument = "ssl_version"
6469
or
6570
call = ssl_Context_class().getACall() and
71+
named_argument = "protocol" and
6672
method_name = "ssl.SSLContext"
6773
)
6874
and
6975
insecure_version = insecure_version_name()
7076
and
7177
(
72-
call.getArgByName("ssl_version").refersTo(the_ssl_module().attr(insecure_version))
78+
call.getArgByName(named_argument).refersTo(the_ssl_module().attr(insecure_version))
7379
or
74-
probable_insecure_ssl_constant(call, insecure_version)
80+
probable_insecure_ssl_constant(call, insecure_version, named_argument)
7581
)
7682
}
7783

@@ -87,7 +93,7 @@ predicate unsafe_pyOpenSSL_Context_call(CallNode call, string insecure_version)
8793

8894
from CallNode call, string method_name, string insecure_version
8995
where
90-
unsafe_ssl_wrap_socket_call(call, method_name, insecure_version)
96+
unsafe_ssl_wrap_socket_call(call, method_name, insecure_version, _)
9197
or
9298
unsafe_pyOpenSSL_Context_call(call, insecure_version) and method_name = "pyOpenSSL.SSL.Context"
9399
select call, "Insecure SSL/TLS protocol version " + insecure_version + " specified in call to " + method_name + "."

‎python/ql/test/query-tests/Security/CWE-327/InsecureProtocol.expected‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
| InsecureProtocol.py:6:1:6:47 | ControlFlowNode for Attribute() | Insecure SSL/TLS protocol version PROTOCOL_SSLv2 specified in call to deprecated method ssl.wrap_socket. |
22
| InsecureProtocol.py:7:1:7:47 | ControlFlowNode for Attribute() | Insecure SSL/TLS protocol version PROTOCOL_SSLv3 specified in call to deprecated method ssl.wrap_socket. |
33
| InsecureProtocol.py:8:1:8:47 | ControlFlowNode for Attribute() | Insecure SSL/TLS protocol version PROTOCOL_TLSv1 specified in call to deprecated method ssl.wrap_socket. |
4-
| InsecureProtocol.py:10:1:10:42 | ControlFlowNode for SSLContext() | Insecure SSL/TLS protocol version PROTOCOL_SSLv2 specified in call to ssl.SSLContext. |
5-
| InsecureProtocol.py:11:1:11:42 | ControlFlowNode for SSLContext() | Insecure SSL/TLS protocol version PROTOCOL_SSLv3 specified in call to ssl.SSLContext. |
6-
| InsecureProtocol.py:12:1:12:42 | ControlFlowNode for SSLContext() | Insecure SSL/TLS protocol version PROTOCOL_TLSv1 specified in call to ssl.SSLContext. |
4+
| InsecureProtocol.py:10:1:10:39 | ControlFlowNode for SSLContext() | Insecure SSL/TLS protocol version PROTOCOL_SSLv2 specified in call to ssl.SSLContext. |
5+
| InsecureProtocol.py:11:1:11:39 | ControlFlowNode for SSLContext() | Insecure SSL/TLS protocol version PROTOCOL_SSLv3 specified in call to ssl.SSLContext. |
6+
| InsecureProtocol.py:12:1:12:39 | ControlFlowNode for SSLContext() | Insecure SSL/TLS protocol version PROTOCOL_TLSv1 specified in call to ssl.SSLContext. |
77
| InsecureProtocol.py:14:1:14:29 | ControlFlowNode for Attribute() | Insecure SSL/TLS protocol version SSLv2_METHOD specified in call to pyOpenSSL.SSL.Context. |
88
| InsecureProtocol.py:15:1:15:30 | ControlFlowNode for Attribute() | Insecure SSL/TLS protocol version SSLv23_METHOD specified in call to pyOpenSSL.SSL.Context. |
99
| InsecureProtocol.py:16:1:16:29 | ControlFlowNode for Attribute() | Insecure SSL/TLS protocol version SSLv3_METHOD specified in call to pyOpenSSL.SSL.Context. |
1010
| InsecureProtocol.py:17:1:17:29 | ControlFlowNode for Attribute() | Insecure SSL/TLS protocol version TLSv1_METHOD specified in call to pyOpenSSL.SSL.Context. |
1111
| InsecureProtocol.py:32:1:32:19 | ControlFlowNode for Attribute() | Insecure SSL/TLS protocol version SSLv2_METHOD specified in call to pyOpenSSL.SSL.Context. |
1212
| InsecureProtocol.py:48:1:48:43 | ControlFlowNode for Attribute() | Insecure SSL/TLS protocol version PROTOCOL_SSLv2 specified in call to deprecated method ssl.wrap_socket. |
13-
| InsecureProtocol.py:49:1:49:38 | ControlFlowNode for SSLContext() | Insecure SSL/TLS protocol version PROTOCOL_SSLv2 specified in call to ssl.SSLContext. |
13+
| InsecureProtocol.py:49:1:49:35 | ControlFlowNode for SSLContext() | Insecure SSL/TLS protocol version PROTOCOL_SSLv2 specified in call to ssl.SSLContext. |
14+
| InsecureProtocol.py:52:1:52:33 | ControlFlowNode for Attribute() | Insecure SSL/TLS protocol version SSLv23_METHOD specified in call to ssl.SSLContext. |

‎python/ql/test/query-tests/Security/CWE-327/InsecureProtocol.py‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
ssl.wrap_socket(ssl_version=ssl.PROTOCOL_SSLv3)
88
ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1)
99

10-
SSLContext(ssl_version=ssl.PROTOCOL_SSLv2)
11-
SSLContext(ssl_version=ssl.PROTOCOL_SSLv3)
12-
SSLContext(ssl_version=ssl.PROTOCOL_TLSv1)
10+
SSLContext(protocol=ssl.PROTOCOL_SSLv2)
11+
SSLContext(protocol=ssl.PROTOCOL_SSLv3)
12+
SSLContext(protocol=ssl.PROTOCOL_TLSv1)
1313

1414
SSL.Context(SSL.SSLv2_METHOD)
1515
SSL.Context(SSL.SSLv23_METHOD)
@@ -34,7 +34,7 @@
3434
# secure versions
3535

3636
ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1_1)
37-
SSLContext(ssl_version=ssl.PROTOCOL_TLSv1_1)
37+
SSLContext(protocol=ssl.PROTOCOL_TLSv1_1)
3838
SSL.Context(SSL.TLSv1_1_METHOD)
3939

4040
# possibly insecure default
@@ -46,5 +46,7 @@
4646
from ssl import PROTOCOL_SSLv2
4747

4848
ssl.wrap_socket(ssl_version=PROTOCOL_SSLv2)
49-
SSLContext(ssl_version=PROTOCOL_SSLv2)
49+
SSLContext(protocol=PROTOCOL_SSLv2)
5050

51+
# FP for insecure default
52+
ssl.SSLContext(ssl.SSLv23_METHOD)

0 commit comments

Comments
 (0)