Skip to content

Commit a09f8c4

Browse files
committed
Python: Port bind-to-all-interfaces to type-tracking
1 parent 4026d54 commit a09f8c4

File tree

4 files changed

+116
-16
lines changed

4 files changed

+116
-16
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
lgtm,codescanning
2+
* Updated _Binding a socket to all network interfaces_ (`py/bind-socket-all-network-interfaces`) query to use the new type-tracking approach instead of points-to analysis. You may see differences in the results found by the query, but overall this change should result in a more robust and accurate analysis.
3+
* Updated _Binding a socket to all network interfaces_ (`py/bind-socket-all-network-interfaces`) to recognize binding to all interfaces in IPv6 with hostnames `::` and `::0`

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

Lines changed: 74 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,28 +11,86 @@
1111
*/
1212

1313
import python
14+
import semmle.python.dataflow.new.DataFlow
15+
import semmle.python.ApiGraphs
1416

15-
Value aSocket() { result.getClass() = Value::named("socket.socket") }
16-
17-
CallNode socketBindCall() {
18-
result = aSocket().attr("bind").(CallableValue).getACall() and major_version() = 3
17+
/** Gets a reference to a hostname that can be used to bind to all interfaces. */
18+
private DataFlow::LocalSourceNode vulnerableHostname(DataFlow::TypeTracker t, string hostname) {
19+
t.start() and
20+
exists(StrConst allInterfacesStrConst |
21+
hostname in [
22+
// IPv4
23+
"0.0.0.0", "",
24+
// IPv6
25+
"::", "::0"
26+
]
27+
|
28+
allInterfacesStrConst.getText() = hostname and
29+
result.asExpr() = allInterfacesStrConst
30+
)
1931
or
20-
result.getFunction().(AttrNode).getObject("bind").pointsTo(aSocket()) and
21-
major_version() = 2
32+
// Due to bad performance when using normal setup with `vulnerableHostname(t2, hostname).track(t2, t)`
33+
// we have inlined that code and forced a join
34+
exists(DataFlow::TypeTracker t2 |
35+
exists(DataFlow::StepSummary summary |
36+
vulnerableHostname_first_join(t2, hostname, result, summary) and
37+
t = t2.append(summary)
38+
)
39+
)
40+
}
41+
42+
pragma[nomagic]
43+
private predicate vulnerableHostname_first_join(
44+
DataFlow::TypeTracker t2, string hostname, DataFlow::Node res, DataFlow::StepSummary summary
45+
) {
46+
DataFlow::StepSummary::step(vulnerableHostname(t2, hostname), res, summary)
2247
}
2348

24-
string allInterfaces() { result = "0.0.0.0" or result = "" }
49+
/** Gets a reference to a hostname that can be used to bind to all interfaces. */
50+
DataFlow::Node vulnerableHostname(string hostname) {
51+
vulnerableHostname(DataFlow::TypeTracker::end(), hostname).flowsTo(result)
52+
}
2553

26-
Value getTextValue(string address) {
27-
result = Value::forUnicode(address) and major_version() = 3
54+
/** Gets a reference to tuple containing a hostname as the first element, that cane be used to bind to all interfaces. */
55+
private DataFlow::LocalSourceNode vulnerableAddressTuple(DataFlow::TypeTracker t, string hostname) {
56+
t.start() and
57+
exists(Tuple tup |
58+
tup.getElt(0) = vulnerableHostname(hostname).asExpr() and
59+
result.asExpr() = tup
60+
)
2861
or
29-
result = Value::forString(address) and major_version() = 2
62+
// Due to bad performance when using normal setup with `vulnerableAddressTuple(t2, hostname).track(t2, t)`
63+
// we have inlined that code and forced a join
64+
exists(DataFlow::TypeTracker t2 |
65+
exists(DataFlow::StepSummary summary |
66+
vulnerableAddressTuple_first_join(t2, hostname, result, summary) and
67+
t = t2.append(summary)
68+
)
69+
)
70+
}
71+
72+
pragma[nomagic]
73+
private predicate vulnerableAddressTuple_first_join(
74+
DataFlow::TypeTracker t2, string hostname, DataFlow::Node res, DataFlow::StepSummary summary
75+
) {
76+
DataFlow::StepSummary::step(vulnerableAddressTuple(t2, hostname), res, summary)
3077
}
3178

32-
from CallNode call, TupleValue args, string address
79+
/** Gets a reference to tuple containing a hostname as the first element, that cane be used to bind to all interfaces. */
80+
DataFlow::Node vulnerableAddressTuple(string hostname) {
81+
vulnerableAddressTuple(DataFlow::TypeTracker::end(), hostname).flowsTo(result)
82+
}
83+
84+
/**
85+
* Gets an instance of `socket.socket` using _some_ address family.
86+
*
87+
* See https://docs.python.org/3/library/socket.html
88+
*/
89+
API::Node socketInstance() { result = API::moduleImport("socket").getMember("socket").getReturn() }
90+
91+
from DataFlow::CallCfgNode bindCall, DataFlow::Node address, string hostname
3392
where
34-
call = socketBindCall() and
35-
call.getArg(0).pointsTo(args) and
36-
args.getItem(0) = getTextValue(address) and
37-
address = allInterfaces()
38-
select call.getNode(), "'" + address + "' binds a socket to all interfaces."
93+
bindCall = socketInstance().getMember("bind").getACall() and
94+
address = bindCall.getArg(0) and
95+
address = vulnerableAddressTuple(hostname)
96+
select bindCall.asExpr(), "'" + hostname + "' binds a socket to all interfaces."
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/**
2+
* @name Binding a socket to all network interfaces
3+
* @description Binding a socket to all interfaces opens it up to traffic from any IPv4 address
4+
* and is therefore associated with security risks.
5+
* @kind problem
6+
* @tags security
7+
* @problem.severity error
8+
* @sub-severity low
9+
* @precision high
10+
* @id py/bind-socket-all-network-interfaces
11+
*/
12+
13+
import python
14+
15+
Value aSocket() { result.getClass() = Value::named("socket.socket") }
16+
17+
CallNode socketBindCall() {
18+
result = aSocket().attr("bind").(CallableValue).getACall() and major_version() = 3
19+
or
20+
result.getFunction().(AttrNode).getObject("bind").pointsTo(aSocket()) and
21+
major_version() = 2
22+
}
23+
24+
string allInterfaces() { result = "0.0.0.0" or result = "" }
25+
26+
Value getTextValue(string address) {
27+
result = Value::forUnicode(address) and major_version() = 3
28+
or
29+
result = Value::forString(address) and major_version() = 2
30+
}
31+
32+
from CallNode call, TupleValue args, string address
33+
where
34+
call = socketBindCall() and
35+
call.getArg(0).pointsTo(args) and
36+
args.getItem(0) = getTextValue(address) and
37+
address = allInterfaces()
38+
select call.getNode(), "'" + address + "' binds a socket to all interfaces."

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
@@ -2,3 +2,4 @@
22
| BindToAllInterfaces_test.py:9:1:9:18 | Attribute() | '' binds a socket to all interfaces. |
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. |
5+
| BindToAllInterfaces_test.py:26:1:26:20 | Attribute() | '::' binds a socket to all interfaces. |

0 commit comments

Comments
 (0)