Skip to content

Commit 4f74d42

Browse files
committed
Python: Exclude AF_UNIX sockets from BindToAllInterfaces
Looking at the results of the the previous DCA run, there was a bunch of false positives where `bind` was being used with a `AF_UNIX` socket (a filesystem path encoded as a string), not a `(host, port)` tuple. These results should be excluded from the query, as they are not vulnerable. Ideally, we would just add `.TupleElement[0]` to the MaD sink, except we don't actually support this in Python MaD... So, instead I opted for a more low-tech solution: check that the argument in question flows from a tuple in the local scope. This eliminates a bunch of false positives on `python/cpython` leaving behind four true positive results.
1 parent c9832c3 commit 4f74d42

File tree

3 files changed

+18
-1
lines changed

3 files changed

+18
-1
lines changed

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,20 @@ private module BindToAllInterfacesFlow = TaintTracking::Global<BindToAllInterfac
4242

4343
private import BindToAllInterfacesFlow
4444

45+
/**
46+
* Holds if `sink` is the address argument of a `bind()` call on a
47+
* network socket (AF_INET or AF_INET6), as opposed to a Unix domain
48+
* socket (AF_UNIX) which takes a plain string path.
49+
*
50+
* Network socket addresses are tuples like `(host, port)`, so we check
51+
* that the sink argument is a tuple, by looking for flow from a tuple expression.
52+
*/
53+
private predicate isNetworkBind(DataFlow::Node sink) {
54+
any(DataFlow::LocalSourceNode n | n.asExpr() instanceof Tuple).flowsTo(sink)
55+
}
56+
4557
from PathNode source, PathNode sink
46-
where flowPath(source, sink)
58+
where flowPath(source, sink) and isNetworkBind(sink.getNode())
4759
select sink.getNode(), source, sink,
4860
"Binding a socket to all interfaces (using $@) is a security risk.", source.getNode(),
4961
"'" + source.getNode().asExpr().(StringLiteral).getText() + "'"

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,4 +60,5 @@ nodes
6060
| BindToAllInterfaces_test.py:53:10:53:25 | ControlFlowNode for Tuple | semmle.label | ControlFlowNode for Tuple |
6161
| BindToAllInterfaces_test.py:58:10:58:18 | ControlFlowNode for StringLiteral | semmle.label | ControlFlowNode for StringLiteral |
6262
| BindToAllInterfaces_test.py:58:10:58:25 | ControlFlowNode for Tuple | semmle.label | ControlFlowNode for Tuple |
63+
| BindToAllInterfaces_test.py:62:9:62:10 | ControlFlowNode for StringLiteral | semmle.label | ControlFlowNode for StringLiteral |
6364
subpaths

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,3 +56,7 @@ def start(self):
5656
from eventlet.green import socket as esocket
5757
es = esocket.socket(esocket.AF_INET, esocket.SOCK_STREAM)
5858
es.bind(('0.0.0.0', 31137)) # $ Alert[py/bind-socket-all-network-interfaces]
59+
60+
# AF_UNIX socket binding should not be flagged
61+
us = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
62+
us.bind('')

0 commit comments

Comments
 (0)