Merge pull request github#5243 from RasmusWL/port-bind-to-all-interfaces

Python: Port py/bind-socket-all-network-interfaces query

Author: tausbn

python/change-notes/2021-02-23-port-bind-to-all-interfaces.md

@@ -0,0 +1,3 @@
+lgtm,codescanning
+* 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.
+* 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

@@ -11,28 +11,86 @@
  */
 
 import python
+import semmle.python.dataflow.new.DataFlow
+import semmle.python.ApiGraphs
 
-Value aSocket() { result.getClass() = Value::named("socket.socket") }
+/** Gets a hostname that can be used to bind to all interfaces. */
+private string vulnerableHostname() {
+  result in [
+      // IPv4
+      "0.0.0.0", "",
+      // IPv6
+      "::", "::0"
+    ]
+}
 
-CallNode socketBindCall() {
-  result = aSocket().attr("bind").(CallableValue).getACall() and major_version() = 3
+/** Gets a reference to a hostname that can be used to bind to all interfaces. */
+private DataFlow::LocalSourceNode vulnerableHostnameRef(DataFlow::TypeTracker t, string hostname) {
+  t.start() and
+  exists(StrConst allInterfacesStrConst | hostname = vulnerableHostname() |
+    allInterfacesStrConst.getText() = hostname and
+    result.asExpr() = allInterfacesStrConst
+  )
   or
-  result.getFunction().(AttrNode).getObject("bind").pointsTo(aSocket()) and
-  major_version() = 2
+  // Due to bad performance when using normal setup with `vulnerableHostnameRef(t2, hostname).track(t2, t)`
+  // we have inlined that code and forced a join
+  exists(DataFlow::TypeTracker t2 |
+    exists(DataFlow::StepSummary summary |
+      vulnerableHostnameRef_first_join(t2, hostname, result, summary) and
+      t = t2.append(summary)
+    )
+  )
+}
+
+pragma[nomagic]
+private predicate vulnerableHostnameRef_first_join(
+  DataFlow::TypeTracker t2, string hostname, DataFlow::Node res, DataFlow::StepSummary summary
+) {
+  DataFlow::StepSummary::step(vulnerableHostnameRef(t2, hostname), res, summary)
 }
 
-string allInterfaces() { result = "0.0.0.0" or result = "" }
+/** Gets a reference to a hostname that can be used to bind to all interfaces. */
+DataFlow::Node vulnerableHostnameRef(string hostname) {
+  vulnerableHostnameRef(DataFlow::TypeTracker::end(), hostname).flowsTo(result)
+}
 
-Value getTextValue(string address) {
-  result = Value::forUnicode(address) and major_version() = 3
+/** Gets a reference to a tuple for which the first element is a hostname that can be used to bind to all interfaces. */
+private DataFlow::LocalSourceNode vulnerableAddressTuple(DataFlow::TypeTracker t, string hostname) {
+  t.start() and
+  result.asExpr() = any(Tuple tup | tup.getElt(0) = vulnerableHostnameRef(hostname).asExpr())
   or
-  result = Value::forString(address) and major_version() = 2
+  // Due to bad performance when using normal setup with `vulnerableAddressTuple(t2, hostname).track(t2, t)`
+  // we have inlined that code and forced a join
+  exists(DataFlow::TypeTracker t2 |
+    exists(DataFlow::StepSummary summary |
+      vulnerableAddressTuple_first_join(t2, hostname, result, summary) and
+      t = t2.append(summary)
+    )
+  )
+}
+
+pragma[nomagic]
+private predicate vulnerableAddressTuple_first_join(
+  DataFlow::TypeTracker t2, string hostname, DataFlow::Node res, DataFlow::StepSummary summary
+) {
+  DataFlow::StepSummary::step(vulnerableAddressTuple(t2, hostname), res, summary)
 }
 
-from CallNode call, TupleValue args, string address
+/** Gets a reference to a tuple for which the first element is a hostname that can be used to bind to all interfaces. */
+DataFlow::Node vulnerableAddressTuple(string hostname) {
+  vulnerableAddressTuple(DataFlow::TypeTracker::end(), hostname).flowsTo(result)
+}
+
+/**
+ * Gets an instance of `socket.socket` using _some_ address family.
+ *
+ * See https://docs.python.org/3/library/socket.html
+ */
+API::Node socketInstance() { result = API::moduleImport("socket").getMember("socket").getReturn() }
+
+from DataFlow::CallCfgNode bindCall, DataFlow::Node addressArg, string hostname
 where
-  call = socketBindCall() and
-  call.getArg(0).pointsTo(args) and
-  args.getItem(0) = getTextValue(address) and
-  address = allInterfaces()
-select call.getNode(), "'" + address + "' binds a socket to all interfaces."
+  bindCall = socketInstance().getMember("bind").getACall() and
+  addressArg = bindCall.getArg(0) and
+  addressArg = vulnerableAddressTuple(hostname)
+select bindCall.asExpr(), "'" + hostname + "' binds a socket to all interfaces."

python/ql/src/experimental/Security-old-dataflow/CVE-2018-1281/BindToAllInterfaces.ql

@@ -0,0 +1,33 @@
+/**
+ * @name Binding a socket to all network interfaces
+ * @description Binding a socket to all interfaces opens it up to traffic from any IPv4 address
+ * and is therefore associated with security risks.
+ * @kind problem
+ */
+
+import python
+
+Value aSocket() { result.getClass() = Value::named("socket.socket") }
+
+CallNode socketBindCall() {
+  result = aSocket().attr("bind").(CallableValue).getACall() and major_version() = 3
+  or
+  result.getFunction().(AttrNode).getObject("bind").pointsTo(aSocket()) and
+  major_version() = 2
+}
+
+string allInterfaces() { result = "0.0.0.0" or result = "" }
+
+Value getTextValue(string address) {
+  result = Value::forUnicode(address) and major_version() = 3
+  or
+  result = Value::forString(address) and major_version() = 2
+}
+
+from CallNode call, TupleValue args, string address
+where
+  call = socketBindCall() and
+  call.getArg(0).pointsTo(args) and
+  args.getItem(0) = getTextValue(address) and
+  address = allInterfaces()
+select call.getNode(), "'" + address + "' binds a socket to all interfaces."

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

@@ -1,3 +1,5 @@
 | BindToAllInterfaces_test.py:5:1:5:26 | Attribute() | '0.0.0.0' binds a socket to all interfaces. |
 | BindToAllInterfaces_test.py:9:1:9:18 | Attribute() | '' binds a socket to all interfaces. |
 | BindToAllInterfaces_test.py:17:1:17:26 | Attribute() | '0.0.0.0' binds a socket to all interfaces. |
+| BindToAllInterfaces_test.py:21:1:21:11 | Attribute() | '0.0.0.0' binds a socket to all interfaces. |
+| BindToAllInterfaces_test.py:26:1:26:20 | Attribute() | '::' binds a socket to all interfaces. |

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

@@ -15,3 +15,12 @@
 # binds to all interfaces, insecure
 ALL_LOCALS = "0.0.0.0"
 s.bind((ALL_LOCALS, 9090))
+
+# binds to all interfaces, insecure
+tup = (ALL_LOCALS, 8080)
+s.bind(tup)
+
+
+# IPv6
+s = socket.socket(socket.AF_INET6, socket.SOCK_STREAM)
+s.bind(("::", 8080)) # NOT OK