Python: Fix experimental py/ip-address-spoofing

I realized the modeling was done in a non-recommended way, so I changed
the modeling. It was very nice that I could use API graphs for the flask
part, and a little sad when I couldn't for Django/Tornado.

python/ql/lib/semmle/python/frameworks/Tornado.qll

@@ -14,10 +14,12 @@ private import semmle.python.frameworks.Stdlib
 private import semmle.python.frameworks.internal.InstanceTaintStepsHelper
 
 /**
+ * INTERNAL: Do not use.
+ *
  * Provides models for the `tornado` PyPI package.
  * See https://www.tornadoweb.org/en/stable/.
  */
-private module Tornado {
+module Tornado {
   /**
    * Provides models for the `tornado.httputil.HTTPHeaders` class
    *
@@ -126,8 +128,7 @@ private module Tornado {
         abstract class InstanceSource extends DataFlow::LocalSourceNode { }
 
         /** The `self` parameter in a method on the `tornado.web.RequestHandler` class or any subclass. */
-        private class SelfParam extends InstanceSource, RemoteFlowSource::Range,
-          DataFlow::ParameterNode {
+        class SelfParam extends InstanceSource, RemoteFlowSource::Range, DataFlow::ParameterNode {
           SelfParam() {
             exists(RequestHandlerClass cls | cls.getAMethod().getArg(0) = this.getParameter())
           }

python/ql/src/experimental/Security/CWE-348/ClientSuppliedIpUsedInSecurityCheckLib.qll

@@ -1,73 +1,57 @@
 private import python
 private import semmle.python.Concepts
 private import semmle.python.ApiGraphs
-private import semmle.python.dataflow.new.RemoteFlowSources
+private import semmle.python.frameworks.Flask
+private import semmle.python.frameworks.Django
+private import semmle.python.frameworks.Tornado
 
 /**
  * A data flow source of the client ip obtained according to the remote endpoint identifier specified
  * (`X-Forwarded-For`, `X-Real-IP`, `Proxy-Client-IP`, etc.) in the header.
  *
  * For example: `request.headers.get("X-Forwarded-For")`.
+ *
+ * A call to `request.headers.get` or `request.headers.get_all` or `request.headers.getlist`.
  */
-abstract class ClientSuppliedIpUsedInSecurityCheck extends DataFlow::CallCfgNode { }
+abstract class ClientSuppliedIpUsedInSecurityCheck extends DataFlow::Node { }
 
-private class FlaskClientSuppliedIpUsedInSecurityCheck extends ClientSuppliedIpUsedInSecurityCheck {
+private class FlaskClientSuppliedIpUsedInSecurityCheck extends ClientSuppliedIpUsedInSecurityCheck,
+  DataFlow::MethodCallNode {
   FlaskClientSuppliedIpUsedInSecurityCheck() {
-    exists(RemoteFlowSource rfs, DataFlow::AttrRead get |
-      rfs.getSourceType() = "flask.request" and this.getFunction() = get
-    |
-      // `get` is a call to request.headers.get or request.headers.get_all or request.headers.getlist
-      // request.headers
-      get.getObject()
-          .(DataFlow::AttrRead)
-          // request
-          .getObject()
-          .getALocalSource() = rfs and
-      get.getAttributeName() in ["get", "get_all", "getlist"] and
-      get.getObject().(DataFlow::AttrRead).getAttributeName() = "headers" and
-      this.getArg(0).asExpr().(StrConst).getText().toLowerCase() = clientIpParameterName()
-    )
+    this = Flask::request().getMember("headers").getMember(["get", "get_all", "getlist"]).getACall() and
+    this.getArg(0).asExpr().(StrConst).getText().toLowerCase() = clientIpParameterName()
   }
 }
 
-private class DjangoClientSuppliedIpUsedInSecurityCheck extends ClientSuppliedIpUsedInSecurityCheck {
+private class DjangoClientSuppliedIpUsedInSecurityCheck extends ClientSuppliedIpUsedInSecurityCheck,
+  DataFlow::MethodCallNode {
   DjangoClientSuppliedIpUsedInSecurityCheck() {
-    exists(RemoteFlowSource rfs, DataFlow::AttrRead get |
-      rfs.getSourceType() = "django.http.request.HttpRequest" and this.getFunction() = get
-    |
-      // `get` is a call to request.headers.get or request.META.get
-      // request.headers
-      get.getObject()
-          .(DataFlow::AttrRead)
-          // request
-          .getObject()
-          .getALocalSource() = rfs and
-      get.getAttributeName() = "get" and
-      get.getObject().(DataFlow::AttrRead).getAttributeName() in ["headers", "META"] and
-      this.getArg(0).asExpr().(StrConst).getText().toLowerCase() = clientIpParameterName()
-    )
+    exists(DataFlow::Node req, DataFlow::AttrRead headers |
+      // a call to request.headers.get or request.META.get
+      req = PrivateDjango::DjangoImpl::DjangoHttp::Request::HttpRequest::instance() and
+      headers.getObject().getALocalSource() = req and
+      headers.getAttributeName() in ["headers", "META"] and
+      this.calls(headers, "get")
+    ) and
+    this.getArg(0).asExpr().(StrConst).getText().toLowerCase() = clientIpParameterName()
   }
 }
 
-private class TornadoClientSuppliedIpUsedInSecurityCheck extends ClientSuppliedIpUsedInSecurityCheck {
+private class TornadoClientSuppliedIpUsedInSecurityCheck extends ClientSuppliedIpUsedInSecurityCheck,
+  DataFlow::MethodCallNode {
   TornadoClientSuppliedIpUsedInSecurityCheck() {
-    exists(RemoteFlowSource rfs, DataFlow::AttrRead get |
-      rfs.getSourceType() = "tornado.web.RequestHandler" and this.getFunction() = get
+    // a call to self.request.headers.get or self.request.headers.get_list inside a tornado requesthandler
+    exists(
+      Tornado::TornadoModule::Web::RequestHandler::SelfParam selfParam, DataFlow::AttrRead headers,
+      DataFlow::AttrRead req
     |
-      // `get` is a call to `rfs`.request.headers.get
-      // `rfs`.request.headers
-      get.getObject()
-          .(DataFlow::AttrRead)
-          // `rfs`.request
-          .getObject()
-          .(DataFlow::AttrRead)
-          // `rfs`
-          .getObject()
-          .getALocalSource() = rfs and
-      get.getAttributeName() in ["get", "get_list"] and
-      get.getObject().(DataFlow::AttrRead).getAttributeName() = "headers" and
-      this.getArg(0).asExpr().(StrConst).getText().toLowerCase() = clientIpParameterName()
-    )
+      req.getObject().getALocalSource() = selfParam and
+      req.getAttributeName() = "request" and
+      headers.getObject().getALocalSource() = req and
+      headers.getAttributeName() = "headers" and
+      this.calls(headers, ["get", "get_list"])
+    ) and
+    this.getArg(0).asExpr().(StrConst).getText().toLowerCase() = clientIpParameterName()
   }
 }
 

python/ql/test/experimental/query-tests/Security/CWE-348/ClientSuppliedIpUsedInSecurityCheck.expected

@@ -1,8 +1,16 @@
 edges
+| flask_bad.py:13:17:13:54 | ControlFlowNode for Attribute() | flask_bad.py:14:12:14:20 | ControlFlowNode for client_ip |
+| flask_bad.py:20:17:20:54 | ControlFlowNode for Attribute() | flask_bad.py:21:12:21:20 | ControlFlowNode for client_ip |
 | tornado_bad.py:22:25:22:69 | ControlFlowNode for Attribute() | tornado_bad.py:23:16:23:24 | ControlFlowNode for client_ip |
 nodes
+| flask_bad.py:13:17:13:54 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| flask_bad.py:14:12:14:20 | ControlFlowNode for client_ip | semmle.label | ControlFlowNode for client_ip |
+| flask_bad.py:20:17:20:54 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| flask_bad.py:21:12:21:20 | ControlFlowNode for client_ip | semmle.label | ControlFlowNode for client_ip |
 | tornado_bad.py:22:25:22:69 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
 | tornado_bad.py:23:16:23:24 | ControlFlowNode for client_ip | semmle.label | ControlFlowNode for client_ip |
 subpaths
 #select
+| flask_bad.py:14:12:14:20 | ControlFlowNode for client_ip | flask_bad.py:13:17:13:54 | ControlFlowNode for Attribute() | flask_bad.py:14:12:14:20 | ControlFlowNode for client_ip | IP address spoofing might include code from $@. | flask_bad.py:13:17:13:54 | ControlFlowNode for Attribute() | this user input |
+| flask_bad.py:21:12:21:20 | ControlFlowNode for client_ip | flask_bad.py:20:17:20:54 | ControlFlowNode for Attribute() | flask_bad.py:21:12:21:20 | ControlFlowNode for client_ip | IP address spoofing might include code from $@. | flask_bad.py:20:17:20:54 | ControlFlowNode for Attribute() | this user input |
 | tornado_bad.py:23:16:23:24 | ControlFlowNode for client_ip | tornado_bad.py:22:25:22:69 | ControlFlowNode for Attribute() | tornado_bad.py:23:16:23:24 | ControlFlowNode for client_ip | IP address spoofing might include code from $@. | tornado_bad.py:22:25:22:69 | ControlFlowNode for Attribute() | this user input |