Merge pull request #201 from advanced-security/knewbury01/fix-service-handler

Enhance remote flow sources for CAP

Author: jeongsoolee09

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

@@ -217,27 +217,28 @@ class CdsServeWithCall extends MethodCallNode {
  */
 class ServiceInstanceFromServeWithParameter extends ServiceInstance {
   CdsServeCall cdsServe;
+  CdsServeWithCall cdsServeWith;
 
   ServiceInstanceFromServeWithParameter() {
-    exists(CdsServeWithCall cdsServeWith |
-      /*
-       * cds.serve('./srv/some-service1').with ((srv) => {  // Parameter `srv` is captured.
-       *   srv.on ('READ','SomeEntity1', (req) => req.reply([...]))
-       * })
-       */
+    /*
+     * cds.serve('./srv/some-service1').with ((srv) => {  // Parameter `srv` is captured.
+     *   srv.on ('READ','SomeEntity1', (req) => req.reply([...]))
+     * })
+     */
 
-      this.(ThisNode).getBinder().asExpr() = cdsServeWith.getDecorator().(FunctionExpr)
-      or
-      /*
-       * cds.serve('./srv/some-service2').with (function() {  // Parameter `this` is captured.
-       *   this.on ('READ','SomeEntity2', (req) => req.reply([...]))
-       * })
-       */
+    this.(ThisNode).getBinder().asExpr() = cdsServeWith.getDecorator().(FunctionExpr)
+    or
+    /*
+     * cds.serve('./srv/some-service2').with (function() {  // Parameter `this` is captured.
+     *   this.on ('READ','SomeEntity2', (req) => req.reply([...]))
+     * })
+     */
 
-      this = cdsServeWith.getDecorator().(ArrowFunctionExpr).getParameter(0).flow()
-    )
+    this = cdsServeWith.getDecorator().(ArrowFunctionExpr).getParameter(0).flow()
   }
 
+  HandlerRegistration getAHandlerRegistration() { result = cdsServeWith.getAHandlerRegistration() }
+
   override UserDefinedApplicationService getDefinition() {
     /* 1. The argument to cds.serve is "all" */
     cdsServe.getServiceRepresentation().getStringValue() = "all" and
@@ -344,23 +345,30 @@ class HandlerRegistration extends MethodCallNode {
   predicate isAfter() { methodName = "after" }
 }
 
+/**
+ * A parameter of a handler
+ */
+class HandlerParameter instanceof ParameterNode {
+  Handler handler;
+
+  HandlerParameter() { this = handler.getParameter(0) }
+
+  Handler getHandler() { result = handler }
+
+  string toString() { result = super.toString() }
+}
+
 /**
  * The handler that implements a service's logic to deal with the incoming request or message when a certain event is fired.
  * It is the last argument to the method calls that registers the handler: either `srv.before`, `srv.on`, or `srv.after`.
  * Its first parameter is of type `cds.Event` and handles the event in an asynchronous manner,
  * or is of type `cds.Request` and handles the event synchronously.
  */
 class Handler extends FunctionNode {
-  UserDefinedApplicationService srv;
   HandlerRegistration handlerRegistration;
 
   Handler() { this = handlerRegistration.getAnArgument() }
 
-  /**
-   * Gets the service registering this handler.
-   */
-  UserDefinedApplicationService getDefiningService() { result = srv }
-
   /**
    * Gets a name of one of the event this handler is registered for.
    */
@@ -375,6 +383,8 @@ class Handler extends FunctionNode {
    * Gets the request that this handler is registered for, as represented as its first parameter.
    */
   CdsRequest getRequest() { result = this.getParameter(0) }
+
+  HandlerRegistration getHandlerRegistration() { result = handlerRegistration }
 }
 
 class CdsRequest extends ParameterNode {
@@ -822,7 +832,7 @@ class HandlerParameterData instanceof PropRead {
   string dataName;
 
   HandlerParameterData() {
-    this = handlerParameter.getAPropertyRead("data").getAPropertyRead(dataName)
+    this = handlerParameter.(SourceNode).getAPropertyRead("data").getAPropertyRead(dataName)
   }
 
   /**
@@ -841,7 +851,7 @@ class HandlerParameterData instanceof PropRead {
       CdlActionOrFunction cdlActionOrFunction, HandlerRegistration handlerRegistration
     |
       handlerRegistration = userDefinedApplicationService.getAHandlerRegistration() and
-      handlerRegistration = handlerParameter.getHandlerRegistration() and
+      handlerRegistration = handlerParameter.getHandler().getHandlerRegistration() and
       handlerRegistration.getAnEventName() = cdlActionOrFunction.getUnqualifiedName() and
       cdlActionOrFunction =
         userDefinedApplicationService.getCdsDeclaration().getAnActionOrFunction() and

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

@@ -2,65 +2,35 @@ import javascript
 import advanced_security.javascript.frameworks.cap.CDS
 
 /**
- * A parameter of a handler registered for a service on an event. e.g.
+ * Either of:
+ * a parameter of a handler registered for an (exposed) service on an event. e.g.
  * ```javascript
  * this.on("SomeEvent", "SomeEntity", (req) => { ... });
- * this.before("SomeEvent", "SomeEntity", (req, next) => { ... }); // only `req` is captured
+ * this.before("SomeEvent", "SomeEntity", (req, next) => { ... });
  * SomeService.on("SomeEvent", "SomeEntity", (msg) => { ... });
  * SomeService.after("SomeEvent", "SomeEntity", (msg) => { ... });
  * ```
- * All the parameters named `req` and `msg` are captured in the above example.
- */
-class HandlerParameter extends ParameterNode, RemoteFlowSource {
-  Handler handler;
-  HandlerRegistration handlerRegistration;
-
-  HandlerParameter() {
-    exists(UserDefinedApplicationService service |
-      handler = handlerRegistration.getHandler() and
-      this = handler.getParameter(0) and
-      service.getAHandlerRegistration() = handlerRegistration and
-      service.isExposed()
-    )
-  }
-
-  override string getSourceType() {
-    result = "Parameter of an event handler belonging to an exposed service"
-  }
-
-  /**
-   * Gets the handler this is a parameter of.
-   */
-  Handler getHandler() { result = handler }
-
-  /**
-   * Gets the handler registration registering the handler it is a parameter of.
-   */
-  HandlerRegistration getHandlerRegistration() { result = handlerRegistration }
-}
-
-/**
- * A service may be described only in a CDS file, but event handlers may still be registered in a format such as:
+ * OR
+ * a handler parameter that is not connected to a service
+ * possibly due to cds compilation failure
+ * or non explicit service references in source. e.g.
  * ```javascript
- * module.exports = srv => {
- *   srv.before('CREATE', 'Media', req => { // an entity name is used to describe which to register this handler to.
- *     ...
- *   });
- * }
+ * cds.serve('./test-service').with((srv) => {
+ *    srv.after('READ', req => req.target.data) //req
+ * })
  * ```
- * parameters named `req` are captured in the above example.
  */
-class ServiceinCDSHandlerParameter extends ParameterNode, RemoteFlowSource {
-  ServiceinCDSHandlerParameter() {
-    exists(MethodCallNode m, CdlEntity entity, string entityName |
-      entity.getName().regexpReplaceAll(".*\\.", "") = entityName and
-      m.getArgument(1).asExpr().getStringValue().regexpReplaceAll("'", "") = entityName and
-      this = m.getArgument(m.getNumArgument() - 1).(FunctionNode).getParameter(0) and
-      m.getMethodName() in ["on", "before", "after"]
-    )
+class HandlerParameterOfExposedService extends RemoteFlowSource, HandlerParameter {
+  HandlerParameterOfExposedService() {
+    this.getHandler().getHandlerRegistration().getService().getDefinition().isExposed()
+    or
+    /* no precise service definition is known */
+    not exists(this.getHandler().getHandlerRegistration().getService().getDefinition())
   }
 
+  override string toString() { result = HandlerParameter.super.toString() }
+
   override string getSourceType() {
-    result = "Parameter of an event handler belonging to an exposed service defined in a cds file"
+    result = "Parameter of an event handler belonging to an exposed service"
   }
 }

javascript/frameworks/cap/test/models/cds/remoteflowsources/db/schema.cds

@@ -8,4 +8,8 @@ entity Entity1 {
 entity Entity2 {
   Attribute3 : String(100);
   Attribute4 : String(100)
+}
+
+entity Entity4 {
+  Attribute4 : String(100)
 }

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

@@ -1,2 +1,12 @@
 | srv/service1.js:6:29:6:31 | req |
 | srv/service2.js:5:27:5:29 | msg |
+| srv/service3nocds.js:6:43:6:45 | req |
+| srv/service3nocds.js:7:34:7:36 | req |
+| srv/service3nocds.js:11:28:11:30 | req |
+| srv/service3nocds.js:12:26:12:28 | req |
+| srv/service3nocds.js:19:34:19:36 | req |
+| srv/service4withcds.js:5:38:5:40 | req |
+| srv/service4withcds.js:6:43:6:45 | req |
+| srv/service4withcds.js:14:33:14:35 | req |
+| srv/service4withcds.js:15:38:15:40 | req |
+| srv/service4withcds.js:16:23:16:25 | req |

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

@@ -2,4 +2,4 @@ import javascript
 import advanced_security.javascript.frameworks.cap.RemoteFlowSources
 
 from RemoteFlowSource source
-select source
+select source

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

@@ -0,0 +1,20 @@
+//this service unit test is a replica of requesthandler.js 
+const cds = require("@sap/cds");
+class BooksService extends cds.ApplicationService {
+    init() {
+        const { Books, Authors } = this.entities
+        this.on('READ', [Books, Authors], req => req.target.data) //req
+        this.on('UPDATE', Books, req => { //req
+            let [ID] = req.params
+            return Object.assign(Books.data[ID], req.data)
+        })
+        this.after('READ', req => req.target.data) //req
+        this.before('*', req => req.target.data) //req
+        return super.init()
+    }
+}
+module.exports = BooksService
+
+cds.serve('./test-service').with((srv) => {
+    srv.before('READ', 'Books', (req) => req.reply([])) //req
+})

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

@@ -0,0 +1,11 @@
+using { advanced_security.log_injection.sample_entities as db_schema } from '../db/schema';
+
+service Service4 @(path: '/service-4') {
+  /* Entity to send READ/GET about. */
+  entity Service4Entity as projection on db_schema.Entity4 excluding { Attribute4 }
+
+  /* API to talk to other services. */
+  action send4 (
+    messageToPass: String
+  ) returns String;
+}

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

@@ -0,0 +1,17 @@
+const cds = require("@sap/cds");
+
+class TestService extends cds.ApplicationService {
+    init() {
+        this.before('READ', 'Test', (req) => req.reply([])) //req
+        this.after('READ', this.entities, req => req.target.data) //req
+        return super.init()
+    }
+}
+module.exports = TestService
+
+cds.serve('./test-service').with((srv) => {
+    const { Test, Service4 } = this.entities
+    srv.before('READ', 'Test', (req) => req.reply([])) //req
+    srv.on('READ', [Test, Service4], req => req.target.data) //req
+    srv.after('READ', req => req.target.data) //req
+})

javascript/frameworks/cap/test/models/cds/requesthandler/requesthandler.expected

@@ -1,3 +1,5 @@
 | requesthandler.js:5:43:5:45 | req |
 | requesthandler.js:6:34:6:36 | req |
-| requesthandler.js:16:34:16:36 | req |
+| requesthandler.js:10:28:10:30 | req |
+| requesthandler.js:11:26:11:28 | req |
+| requesthandler.js:18:34:18:36 | req |

javascript/frameworks/cap/test/models/cds/requesthandler/requesthandler.js

@@ -7,6 +7,8 @@ class BooksService extends cds.ApplicationService {
             let [ID] = req.params
             return Object.assign(Books.data[ID], req.data)
         })
+        this.after('READ', req => req.target.data)
+        this.before('*', req => req.target.data)
         return super.init()
     }
 }