Skip to content

Commit c439fc5

Browse files
tausbntausbn
committed
Python: Replace type tracking with global data-flow
This takes care of most of the false negatives from the preceding commit. Additionally, we add models for some known wrappers of `socket.socket` from the `gevent` and `eventlet` packages.
1 parent 1ecd9e8 commit c439fc5

7 files changed

Lines changed: 43 additions & 42 deletions

File tree

‎python/ql/lib/semmle/python/frameworks/Eventlet.model.yml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/python-all
4+
extensible: typeModel
5+
data:
6+
# See https://eventlet.readthedocs.io/en/latest/patching.html
7+
- ['socket.socket', 'eventlet', 'Member[green].Member[socket].Member[socket].ReturnValue']
8+
# eventlet also re-exports as eventlet.socket for convenience
9+
- ['socket.socket', 'eventlet', 'Member[socket].Member[socket].ReturnValue']

‎python/ql/lib/semmle/python/frameworks/Gevent.model.yml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/python-all
4+
extensible: typeModel
5+
data:
6+
# See https://www.gevent.org/api/gevent.socket.html
7+
- ['socket.socket', 'gevent', 'Member[socket].Member[socket].ReturnValue']

‎python/ql/lib/semmle/python/frameworks/Stdlib.model.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ extensions:
2727
extensible: sinkModel
2828
data:
2929
- ["zipfile.ZipFile","Member[extractall].Argument[0,path:]", "path-injection"]
30+
# See https://docs.python.org/3/library/socket.html#socket.socket.bind
31+
- ["socket.socket", "Member[bind].Argument[0,address:]", "bind-socket-all-interfaces"]
3032

3133
- addsTo:
3234
pack: codeql/python-all
@@ -184,6 +186,8 @@ extensions:
184186
pack: codeql/python-all
185187
extensible: typeModel
186188
data:
189+
# See https://docs.python.org/3/library/socket.html#socket.socket
190+
- ['socket.socket', 'socket', 'Member[socket].ReturnValue']
187191
# See https://docs.python.org/3/library/urllib.parse.html#urllib.parse.urlparse
188192
- ["urllib.parse.ParseResult~Subclass", 'urllib', 'Member[parse].Member[urlparse]']
189193

‎python/ql/src/Security/CVE-2018-1281/BindToAllInterfaces.ql‎

Lines changed: 14 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@
1414

1515
import python
1616
import semmle.python.dataflow.new.DataFlow
17-
import semmle.python.ApiGraphs
17+
import semmle.python.dataflow.new.TaintTracking
18+
private import semmle.python.frameworks.data.ModelsAsData
1819

1920
/** Gets a hostname that can be used to bind to all interfaces. */
2021
private string vulnerableHostname() {
@@ -26,45 +27,21 @@ private string vulnerableHostname() {
2627
]
2728
}
2829

29-
/** Gets a reference to a hostname that can be used to bind to all interfaces. */
30-
private DataFlow::TypeTrackingNode vulnerableHostnameRef(DataFlow::TypeTracker t, string hostname) {
31-
t.start() and
32-
exists(StringLiteral allInterfacesStringLiteral | hostname = vulnerableHostname() |
33-
allInterfacesStringLiteral.getText() = hostname and
34-
result.asExpr() = allInterfacesStringLiteral
35-
)
36-
or
37-
exists(DataFlow::TypeTracker t2 | result = vulnerableHostnameRef(t2, hostname).track(t2, t))
38-
}
39-
40-
/** Gets a reference to a hostname that can be used to bind to all interfaces. */
41-
DataFlow::Node vulnerableHostnameRef(string hostname) {
42-
vulnerableHostnameRef(DataFlow::TypeTracker::end(), hostname).flowsTo(result)
43-
}
30+
private module BindToAllInterfacesConfig implements DataFlow::ConfigSig {
31+
predicate isSource(DataFlow::Node source) {
32+
source.asExpr().(StringLiteral).getText() = vulnerableHostname()
33+
}
4434

45-
/** Gets a reference to a tuple for which the first element is a hostname that can be used to bind to all interfaces. */
46-
private DataFlow::TypeTrackingNode vulnerableAddressTuple(DataFlow::TypeTracker t, string hostname) {
47-
t.start() and
48-
result.asExpr() = any(Tuple tup | tup.getElt(0) = vulnerableHostnameRef(hostname).asExpr())
49-
or
50-
exists(DataFlow::TypeTracker t2 | result = vulnerableAddressTuple(t2, hostname).track(t2, t))
35+
predicate isSink(DataFlow::Node sink) {
36+
ModelOutput::sinkNode(sink, "bind-socket-all-interfaces")
37+
}
5138
}
5239

53-
/** Gets a reference to a tuple for which the first element is a hostname that can be used to bind to all interfaces. */
54-
DataFlow::Node vulnerableAddressTuple(string hostname) {
55-
vulnerableAddressTuple(DataFlow::TypeTracker::end(), hostname).flowsTo(result)
56-
}
57-
58-
/**
59-
* Gets an instance of `socket.socket` using _some_ address family.
60-
*
61-
* See https://docs.python.org/3/library/socket.html
62-
*/
63-
API::Node socketInstance() { result = API::moduleImport("socket").getMember("socket").getReturn() }
40+
private module BindToAllInterfacesFlow = TaintTracking::Global<BindToAllInterfacesConfig>;
6441

65-
from DataFlow::CallCfgNode bindCall, DataFlow::Node addressArg, string hostname
42+
from DataFlow::Node source, DataFlow::Node sink, DataFlow::CallCfgNode bindCall, string hostname
6643
where
67-
bindCall = socketInstance().getMember("bind").getACall() and
68-
addressArg = bindCall.getArg(0) and
69-
addressArg = vulnerableAddressTuple(hostname)
44+
BindToAllInterfacesFlow::flow(source, sink) and
45+
bindCall.getArg(0) = sink and
46+
hostname = source.asExpr().(StringLiteral).getText()
7047
select bindCall.asExpr(), "'" + hostname + "' binds a socket to all interfaces."

‎python/ql/test/query-tests/Security/CVE-2018-1281/BindToAllInterfaces.expected‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,7 @@
33
| BindToAllInterfaces_test.py:17:1:17:26 | Attribute() | '0.0.0.0' binds a socket to all interfaces. |
44
| BindToAllInterfaces_test.py:21:1:21:11 | Attribute() | '0.0.0.0' binds a socket to all interfaces. |
55
| BindToAllInterfaces_test.py:26:1:26:20 | Attribute() | '::' binds a socket to all interfaces. |
6+
| BindToAllInterfaces_test.py:39:9:39:43 | Attribute() | '0.0.0.0' binds a socket to all interfaces. |
7+
| BindToAllInterfaces_test.py:48:1:48:20 | Attribute() | '0.0.0.0' binds a socket to all interfaces. |
8+
| BindToAllInterfaces_test.py:53:1:53:27 | Attribute() | '0.0.0.0' binds a socket to all interfaces. |
9+
| BindToAllInterfaces_test.py:58:1:58:27 | Attribute() | '0.0.0.0' binds a socket to all interfaces. |

‎python/ql/test/query-tests/Security/CVE-2018-1281/BindToAllInterfaces_test.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ def __init__(self):
3636

3737
def start(self):
3838
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
39-
s.bind((self.bind_addr, self.port)) # $ MISSING: Alert[py/bind-socket-all-network-interfaces]
39+
s.bind((self.bind_addr, self.port)) # $ Alert[py/bind-socket-all-network-interfaces]
4040

4141
server = Server()
4242
server.start()
@@ -45,14 +45,14 @@ def start(self):
4545
import os
4646
host = os.environ.get('APP_HOST', '0.0.0.0')
4747
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
48-
s.bind((host, 8080)) # $ MISSING: Alert[py/bind-socket-all-network-interfaces]
48+
s.bind((host, 8080)) # $ Alert[py/bind-socket-all-network-interfaces]
4949

5050
# gevent.socket (alternative socket module)
5151
from gevent import socket as gsocket
5252
gs = gsocket.socket(gsocket.AF_INET, gsocket.SOCK_STREAM)
53-
gs.bind(('0.0.0.0', 31137)) # $ MISSING: Alert[py/bind-socket-all-network-interfaces]
53+
gs.bind(('0.0.0.0', 31137)) # $ Alert[py/bind-socket-all-network-interfaces]
5454

5555
# eventlet.green.socket (another alternative socket module)
5656
from eventlet.green import socket as esocket
5757
es = esocket.socket(esocket.AF_INET, esocket.SOCK_STREAM)
58-
es.bind(('0.0.0.0', 31137)) # $ MISSING: Alert[py/bind-socket-all-network-interfaces]
58+
es.bind(('0.0.0.0', 31137)) # $ Alert[py/bind-socket-all-network-interfaces]

‎shared/mad/codeql/mad/ModelValidation.qll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ module KindValidation<KindValidationConfigSig Config> {
4848
// CPP-only currently
4949
"remote-sink",
5050
// Python-only currently, but may be shared in the future
51-
"prompt-injection"
51+
"bind-socket-all-interfaces", "prompt-injection"
5252
]
5353
or
5454
this.matches([

0 commit comments

Comments
 (0)