Merge pull request #222 from advanced-security/jeongsoolee09/debug-remoteflowsources-properties

Address FN involving CAP remote flow sources

Author: jeongsoolee09

javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CDS.qll

@@ -177,7 +177,27 @@ class ServiceInstanceFromConstructor extends ServiceInstance {
 }
 
 /**
- * A read to `this` variable which represents the service whose definition encloses this variable access.
+ * A read to `this` variable which represents the service whose definition encloses
+ * this variable access.
+ * e.g.1. Given this code:
+ * ``` javascript
+ * const cds = require("@sap/cds");
+ * module.exports = class SomeService extends cds.ApplicationService {
+ *   init() {
+ *     this.on("SomeEvent", (req) => { ... } ) 
+ *   }
+ * }
+ * ```
+ * This class captures the access to the `this` variable as in `this.on(...)`.
+ * 
+ * e.g.2. Given this code:
+ * ``` javascript
+ * const cds = require('@sap/cds');
+ * module.exports = cds.service.impl (function() {
+ *   this.on("SomeEvent", (req) => { ... })
+ * })
+ * ```
+ * This class captures the access to the `this` variable as in `this.on(...)`.
  */
 class ServiceInstanceFromThisNode extends ServiceInstance, ThisNode {
   UserDefinedApplicationService userDefinedApplicationService;
@@ -294,12 +314,56 @@ class DbServiceInstanceFromCdsConnectTo extends ServiceInstanceFromCdsConnectTo,
   override UserDefinedApplicationService getDefinition() { none() }
 }
 
+/**
+ * The 0-th parameter of an exported closure that represents the service being implemented. e.g.
+ * ``` javascript
+ * const cds = require('@sap/cds')
+ * module.exports = (srv) => {
+ *   srv.on("SomeEvent1", (req) => { ... })
+ * }
+ * ```
+ * This class captures the `srv` parameter of the exported arrow function. Also see
+ * `ServiceInstanceFromImplMethodCallClosureParameter` which is similar to this.
+ */
+class ServiceInstanceFromExportedClosureParameter extends ServiceInstance, ParameterNode {
+  ExportedClosureApplicationServiceDefinition exportedClosure;
+
+  ServiceInstanceFromExportedClosureParameter() { this = exportedClosure.getParameter(0) }
+
+  override UserDefinedApplicationService getDefinition() { result = exportedClosure }
+}
+
+/**
+ * The 0-th parameter of a callback (usually an arrow function) passed to `cds.service.impl` that
+ * represents the service being implemented. e.g.
+ * ``` javascript
+ * const cds = require('@sap/cds')
+ * module.exports = cds.service.impl((srv) => {
+ *   srv.on("SomeEvent1", (req) => { ... })
+ * })
+ * ```
+ * This class captures the `srv` parameter of the exported arrow function. Also see
+ * `ServiceInstanceFromExportedClosureParameter` which is similar to this.
+ */
+class ServiceInstanceFromImplMethodCallClosureParameter extends ServiceInstance, ParameterNode {
+  ImplMethodCallApplicationServiceDefinition implMethodCallApplicationServiceDefinition;
+
+  ServiceInstanceFromImplMethodCallClosureParameter() {
+    this = implMethodCallApplicationServiceDefinition.getInitFunction().getParameter(0)
+  }
+
+  override UserDefinedApplicationService getDefinition() {
+    result = implMethodCallApplicationServiceDefinition
+  }
+}
+
 /**
  * A call to `before`, `on`, or `after` on an `cds.ApplicationService`.
  * It registers an handler to be executed when an event is fired,
  * to do something with the incoming request or event as its parameter.
  */
 class HandlerRegistration extends MethodCallNode {
+  /** The instance of the service a handler is registered on. */
   ServiceInstance srv;
   string methodName;
 
@@ -308,6 +372,9 @@ class HandlerRegistration extends MethodCallNode {
     methodName = ["before", "on", "after"]
   }
 
+  /**
+   * Gets the instance of the service a handler is registered on.
+   */
   ServiceInstance getService() { result = srv }
 
   /**
@@ -347,7 +414,7 @@ class HandlerRegistration extends MethodCallNode {
 
 /**
  * The first parameter of a handler, representing the request object received either directly
- * from a user, or from another service that may be internal (defined in the same application) 
+ * from a user, or from another service that may be internal (defined in the same application)
  * or external (defined in another application, or even served from a different server).
  * e.g.
  * ``` javascript
@@ -356,7 +423,7 @@ class HandlerRegistration extends MethodCallNode {
  *   this.before("SomeEvent", "SomeEntity", (req, next) => { ... });
  *   this.after("SomeEvent", "SomeEntity", (req, next) => { ... });
  * }
- * ``` 
+ * ```
  * All parameters named `req` above are captured. Also see `HandlerParameterOfExposedService`
  * for a subset of this class that is only about handlers exposed to some protocol.
  */
@@ -506,7 +573,7 @@ abstract class UserDefinedApplicationService extends UserDefinedService {
   /**
    * Holds if this service supports access from the outside through any kind of protocol.
    */
-  predicate isExposed() { not this.isInternal() }
+  predicate isExposed() { exists(this.getCdsDeclaration()) and not this.isInternal() }
 
   /**
    * Holds if this service does not support access from the outside through any kind of protocol, thus being internal only.
@@ -539,9 +606,21 @@ class ES6ApplicationServiceDefinition extends ClassNode, UserDefinedApplicationS
 
 /**
  * Subclassing `cds.ApplicationService` via a call to `cds.service.impl`.
- * ```js
+ * e.g.1. Given this code:
+ * ``` javascript
+ * const cds = require('@sap/cds')
+ * module.exports = cds.service.impl (function() {
+ *   this.on("SomeEvent1", (req) => { ... })
+ * })
+ * ```
+ * This class captures the call `cds.service.impl (function() { ... })`.
+ *
+ * e.g.2. Given this code:
+ * ``` javascript
  * const cds = require('@sap/cds')
- * module.exports = cds.service.impl (function() { ... })
+ * module.exports = cds.service.impl ((srv) => {
+ *   srv.on("SomeEvent1", (req) => { ... })
+ * })
  * ```
  */
 class ImplMethodCallApplicationServiceDefinition extends MethodCallNode,
@@ -554,6 +633,40 @@ class ImplMethodCallApplicationServiceDefinition extends MethodCallNode,
   override FunctionNode getInitFunction() { result = this.getArgument(0) }
 }
 
+/**
+ * A user-defined application service that comes in a form of an exported
+ * closure. e.g. Given the below code,
+ * ``` javascript
+ * const cds = require("@sap/cds");
+ *
+ * module.exports = (srv) => {
+ *   srv.before("SomeEvent1", "SomeEntity", (req, res) => { ... })
+ *   srv.on("SomeEvent2", (req) => { ... } )
+ *   srv.after("SomeEvent3", (req) => { ... } )
+ * }
+ * ```
+ * This class captures the entire `(srv) => { ... }` function that is
+ * exported.
+ */
+class ExportedClosureApplicationServiceDefinition extends FunctionNode,
+  UserDefinedApplicationService
+{
+  ExportedClosureApplicationServiceDefinition() {
+    /*
+     * ==================== HACK ====================
+     * See issue #221.
+     */
+
+    exists(PropWrite moduleExports |
+      moduleExports.getBase().asExpr().(VarAccess).getName() = "module" and
+      moduleExports.getPropertyName() = "exports" and
+      this = moduleExports.getRhs()
+    )
+  }
+
+  override FunctionNode getInitFunction() { result = this }
+}
+
 abstract class InterServiceCommunicationMethodCall extends MethodCallNode {
   string name;
   ServiceInstance recipient;

javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/RemoteFlowSources.qll

@@ -30,7 +30,9 @@ class HandlerParameterOfExposedService extends HandlerParameter {
      * is known.
      */
 
-    not exists(this.getHandler().getHandlerRegistration().getService().getDefinition())
+    not exists(
+      this.getHandler().getHandlerRegistration().getService().getDefinition().getCdsDeclaration()
+    )
   }
 }
 
@@ -54,7 +56,9 @@ class UserProvidedPropertyReadOfHandlerParameterOfExposedService extends RemoteF
 
   UserProvidedPropertyReadOfHandlerParameterOfExposedService() {
     /* 1. `req.(data|params|headers|id)` */
-    this = handlerParameterOfExposedService.getAPropertyRead(["data", "params", "headers", "id"])
+    this =
+      handlerParameterOfExposedService
+          .getAPropertyRead(["data", "params", "headers", "id", "_queryOptions"])
     or
     /* 2. APIs stemming from `req.http.req`: Defined by Express.js */
     exists(PropRead reqHttpReq |

javascript/frameworks/cap/test/models/cds/remoteflowsources/ExposedServices.expected renamed to javascript/frameworks/cap/test/models/cds/remoteflowsources/HandlerParameterOfExposedService.expected

@@ -9,6 +9,7 @@
 | srv/service1.js:52:29:52:31 | req |
 | srv/service1.js:58:29:58:31 | req |
 | srv/service1.js:64:29:64:31 | req |
+| srv/service1.js:70:30:70:32 | req |
 | srv/service2.js:4:27:4:29 | msg |
 | srv/service3.js:5:29:5:31 | req |
 | srv/service3.js:11:29:11:31 | req |
@@ -19,3 +20,4 @@
 | srv/service3.js:51:29:51:31 | req |
 | srv/service3.js:57:29:57:31 | req |
 | srv/service3.js:63:29:63:31 | req |
+| srv/service3.js:69:30:69:32 | req |

javascript/frameworks/cap/test/models/cds/remoteflowsources/ExposedServices.ql renamed to javascript/frameworks/cap/test/models/cds/remoteflowsources/HandlerParameterOfExposedService.ql

@@ -13,6 +13,7 @@
 | srv/service1.js:34:31:34:61 | req.htt ... eProp") |
 | srv/service1.js:35:31:35:60 | req.htt ... eProp") |
 | srv/service1.js:41:29:41:34 | req.id |
+| srv/service1.js:47:29:47:45 | req._queryOptions |
 | srv/service2.js:5:31:5:38 | msg.data |
 | srv/service3.js:6:33:6:40 | req.data |
 | srv/service3.js:12:33:12:42 | req.params |
@@ -29,3 +30,4 @@
 | srv/service3.js:33:31:33:61 | req.htt ... eProp") |
 | srv/service3.js:34:31:34:60 | req.htt ... eProp") |
 | srv/service3.js:40:29:40:34 | req.id |
+| srv/service3.js:46:29:46:45 | req._queryOptions |

javascript/frameworks/cap/test/models/cds/remoteflowsources/remoteflowsource.expected

@@ -44,24 +44,30 @@ module.exports = class Service1 extends cds.ApplicationService {
     });
 
     this.on("send6", async (req) => {
-      const messageToPass = req.locale;  // SAFE: Not a taint source, Exposed service
+      const messageToPass = req._queryOptions;  // UNSAFE: Taint source, Exposed service
       const Service2 = await cds.connect.to("service-2");
       Service2.send("send2", { messageToPass });
     });
 
     this.on("send7", async (req) => {
-      const messageToPass = req.tenant;  // SAFE: Not a taint source, Exposed service
+      const messageToPass = req.locale;  // SAFE: Not a taint source, Exposed service
       const Service2 = await cds.connect.to("service-2");
       Service2.send("send2", { messageToPass });
     });
 
     this.on("send8", async (req) => {
-      const messageToPass = req.timestamp;  // SAFE: Not a taint source, Exposed service
+      const messageToPass = req.tenant;  // SAFE: Not a taint source, Exposed service
       const Service2 = await cds.connect.to("service-2");
       Service2.send("send2", { messageToPass });
     });
 
     this.on("send9", async (req) => {
+      const messageToPass = req.timestamp;  // SAFE: Not a taint source, Exposed service
+      const Service2 = await cds.connect.to("service-2");
+      Service2.send("send2", { messageToPass });
+    });
+
+    this.on("send10", async (req) => {
       const messageToPass = req.user;  // SAFE: Not a taint source, Exposed service
       const Service2 = await cds.connect.to("service-2");
       Service2.send("send2", { messageToPass });

javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service1.js

@@ -43,24 +43,30 @@ class Service3 extends cds.ApplicationService {
     });
 
     this.on("send6", async (req) => {
-      const messageToPass = req.locale;  // SAFE: Not a taint source, Exposed service (fallback)
+      const messageToPass = req._queryOptions;  // UNSAFE: Taint source, Exposed service
       const Service2 = await cds.connect.to("service-2");
       Service2.send("send2", { messageToPass });
     });
 
     this.on("send7", async (req) => {
-      const messageToPass = req.tenant;  // SAFE: Not a taint source, Exposed service (fallback)
+      const messageToPass = req.locale;  // SAFE: Not a taint source, Exposed service (fallback)
       const Service2 = await cds.connect.to("service-2");
       Service2.send("send2", { messageToPass });
     });
 
     this.on("send8", async (req) => {
-      const messageToPass = req.timestamp;  // SAFE: Not a taint source, Exposed service (fallback)
+      const messageToPass = req.tenant;  // SAFE: Not a taint source, Exposed service (fallback)
       const Service2 = await cds.connect.to("service-2");
       Service2.send("send2", { messageToPass });
     });
 
     this.on("send9", async (req) => {
+      const messageToPass = req.timestamp;  // SAFE: Not a taint source, Exposed service (fallback)
+      const Service2 = await cds.connect.to("service-2");
+      Service2.send("send2", { messageToPass });
+    });
+
+    this.on("send10", async (req) => {
       const messageToPass = req.user;  // SAFE: Not a taint source, Exposed service (fallback)
       const Service2 = await cds.connect.to("service-2");
       Service2.send("send2", { messageToPass });

javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service3.js

@@ -41,5 +41,11 @@ module.exports = class Service4 extends cds.ApplicationService {
       const Service2 = await cds.connect.to("service-2");
       Service2.send("send2", { messageToPass });
     });
+
+    this.on("send6", async (req) => {
+      const messageToPass = req._queryOptions;  // UNSAFE: Taint source, Exposed service
+      const Service2 = await cds.connect.to("service-2");
+      Service2.send("send2", { messageToPass });
+    });
   }
 };

javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service4.js

@@ -0,0 +1,9 @@
+const cds = require("@sap/cds");
+
+class Service1 extends cds.ApplicationService {  // ES6ApplicationServiceDefinition
+  init() {
+    return super.init()
+  }
+}
+
+module.exports = Service1

javascript/frameworks/cap/test/models/cds/userdefinedservice/service1.js

@@ -0,0 +1,7 @@
+const cds = require("@sap/cds");
+
+module.exports = class LogService extends cds.Service {  // UserDefinedService
+  init() {
+    return super.init()
+  }
+}