Make CAP Log injection query more resilient and conservative

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

@@ -22,6 +22,10 @@ class CdsLogger extends MethodCallNode {
   string getName() { result = name }
 }
 
+/**
+ * A template literal that is not interpolated. It is basically a string literal
+ * defined in backticks (\`\`).
+ */
 class ConstantOnlyTemplateLiteral extends TemplateLiteral {
   ConstantOnlyTemplateLiteral() {
     forall(Expr e | e = this.getAnElement() | e instanceof TemplateElement)
@@ -51,9 +55,26 @@ module CAPLogInjectionConfiguration implements DataFlow::ConfigSig {
   }
 
   predicate isBarrier(DataFlow::Node node) {
+    /*
+     * This predicate includes cases such as:
+     * 1. A CDS entity element lacking a type annotation.
+     *   - Possibly because it relies on a common aspect.
+     * 2. A CDS entity element annotated with a non-string type listed above.
+     *
+     * Therefore, the data held by the handler parameter data (e.g. `req.data.X`)
+     * has to be EXPLICITLY annotated as `String` or `LargeString` to be excluded
+     * from the next condition.
+     */
+
     exists(HandlerParameterData handlerParameterData |
       node = handlerParameterData and
-      not handlerParameterData.getType() = ["cds.String", "cds.LargeString"]
+      /* Note the use of `.. != ..` instead of `not .. = ..` below. */
+      exists(string handlerParameterDataType |
+        handlerParameterDataType = handlerParameterData.getType()
+      |
+        handlerParameterDataType != "cds.String" and
+        handlerParameterDataType != "cds.LargeString"
+      )
     )
   }
 

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

@@ -184,12 +184,12 @@ class ServiceInstanceFromConstructor extends ServiceInstance {
  * const cds = require("@sap/cds");
  * module.exports = class SomeService extends cds.ApplicationService {
  *   init() {
- *     this.on("SomeEvent", (req) => { ... } ) 
+ *     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');
@@ -424,13 +424,14 @@ class HandlerRegistration extends MethodCallNode {
  *   this.after("SomeEvent", "SomeEntity", (req, next) => { ... });
  * }
  * ```
- * All parameters named `req` above are captured. Also see `HandlerParameterOfExposedService`
+ * All parameters named `req` above are captured. Also see
+ * `RemoteflowSources::HandlerParameterOfExposedService`
  * for a subset of this class that is only about handlers exposed to some protocol.
  */
 class HandlerParameter extends ParameterNode {
   Handler handler;
 
-  HandlerParameter() { this = handler.getParameter(0) }
+  HandlerParameter() { this = isHandlerParameter(handler) }
 
   Handler getHandler() { result = handler }
 }

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

@@ -39,3 +39,12 @@ private SourceNode cdsApplicationServiceInstantiation(TypeTracker t) {
 SourceNode cdsApplicationServiceInstantiation() {
   result = cdsApplicationServiceInstantiation(TypeTracker::end())
 }
+
+private SourceNode isHandlerParameter(TypeTracker t, Handler handler) {
+  result = handler.getParameter(0) or
+  exists(TypeTracker t2 | result = isHandlerParameter(t, handler).track(t2, t))
+}
+
+SourceNode isHandlerParameter(Handler handler) {
+  result = isHandlerParameter(TypeTracker::end(), handler)
+}

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

@@ -1,23 +1,43 @@
 | server.js:7:32:7:34 | req |
 | server.js:11:32:11:34 | req |
-| srv/service1.js:6:29:6:31 | req |
-| srv/service1.js:12:29:12:31 | req |
-| srv/service1.js:18:29:18:31 | req |
-| srv/service1.js:24:29:24:31 | req |
-| srv/service1.js:40:29:40:31 | req |
-| srv/service1.js:46:29:46:31 | req |
-| 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/service1.js:6:30:6:32 | req |
+| srv/service1.js:12:30:12:32 | req |
+| srv/service1.js:19:30:19:32 | req |
+| srv/service1.js:25:30:25:32 | req |
+| srv/service1.js:31:30:31:32 | req |
+| srv/service1.js:37:30:37:32 | req |
+| srv/service1.js:44:29:44:31 | req |
+| srv/service1.js:60:30:60:32 | req |
+| srv/service1.js:66:30:66:32 | req |
+| srv/service1.js:72:30:72:32 | req |
+| srv/service1.js:78:30:78:32 | req |
+| srv/service1.js:84:29:84:31 | req |
+| srv/service1.js:90:29:90:31 | req |
+| srv/service1.js:96:29:96:31 | req |
+| srv/service1.js:102:30:102:32 | req |
+| srv/service1.js:110:21:110:27 | request |
+| srv/service1.js:114:23:114:29 | request |
+| srv/service1.js:118:24:118:30 | request |
+| srv/service1.js:122:19:122:25 | request |
+| srv/service1.js:126:29:126:35 | request |
 | srv/service2.js:4:27:4:29 | msg |
-| srv/service3.js:5:29:5:31 | req |
-| srv/service3.js:11:29:11:31 | req |
-| srv/service3.js:17:29:17:31 | req |
-| srv/service3.js:23:29:23:31 | req |
-| srv/service3.js:39:29:39:31 | req |
-| srv/service3.js:45:29:45:31 | req |
-| 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 |
+| srv/service3.js:5:30:5:32 | req |
+| srv/service3.js:11:30:11:32 | req |
+| srv/service3.js:18:30:18:32 | req |
+| srv/service3.js:24:30:24:32 | req |
+| srv/service3.js:30:30:30:32 | req |
+| srv/service3.js:36:30:36:32 | req |
+| srv/service3.js:43:29:43:31 | req |
+| srv/service3.js:59:30:59:32 | req |
+| srv/service3.js:65:30:65:32 | req |
+| srv/service3.js:71:30:71:32 | req |
+| srv/service3.js:77:30:77:32 | req |
+| srv/service3.js:83:29:83:31 | req |
+| srv/service3.js:89:29:89:31 | req |
+| srv/service3.js:95:29:95:31 | req |
+| srv/service3.js:101:30:101:32 | req |
+| srv/service3.js:111:21:111:27 | request |
+| srv/service3.js:115:23:115:29 | request |
+| srv/service3.js:119:24:119:30 | request |
+| srv/service3.js:123:19:123:25 | request |
+| srv/service3.js:127:29:127:35 | request |

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

@@ -1,33 +1,43 @@
 | srv/service1.js:7:33:7:40 | req.data |
-| srv/service1.js:13:33:13:42 | req.params |
-| srv/service1.js:19:29:19:39 | req.headers |
-| srv/service1.js:25:30:25:56 | req.htt ... omeProp |
-| srv/service1.js:26:30:26:55 | req.htt ... omeProp |
-| srv/service1.js:27:30:27:57 | req.htt ... omeProp |
-| srv/service1.js:28:30:28:58 | req.htt ... omeProp |
-| srv/service1.js:29:30:29:58 | req.htt ... omeProp |
-| srv/service1.js:30:30:30:53 | req.htt ... inalUrl |
-| srv/service1.js:31:30:31:50 | req.htt ... ostname |
-| srv/service1.js:32:30:32:57 | req.htt ... eProp") |
-| srv/service1.js:33:30:33:56 | req.htt ... eProp") |
-| 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/service1.js:20:33:20:42 | req.params |
+| srv/service1.js:32:29:32:39 | req.headers |
+| srv/service1.js:45:30:45:56 | req.htt ... omeProp |
+| srv/service1.js:46:30:46:55 | req.htt ... omeProp |
+| srv/service1.js:47:30:47:57 | req.htt ... omeProp |
+| srv/service1.js:48:30:48:58 | req.htt ... omeProp |
+| srv/service1.js:49:30:49:58 | req.htt ... omeProp |
+| srv/service1.js:50:30:50:53 | req.htt ... inalUrl |
+| srv/service1.js:51:30:51:50 | req.htt ... ostname |
+| srv/service1.js:52:30:52:57 | req.htt ... eProp") |
+| srv/service1.js:53:30:53:56 | req.htt ... eProp") |
+| srv/service1.js:54:31:54:61 | req.htt ... eProp") |
+| srv/service1.js:55:31:55:60 | req.htt ... eProp") |
+| srv/service1.js:61:29:61:34 | req.id |
+| srv/service1.js:73:29:73:45 | req._queryOptions |
+| srv/service1.js:111:10:111:21 | request.data |
+| srv/service1.js:115:10:115:23 | request.params |
+| srv/service1.js:119:10:119:24 | request.headers |
+| srv/service1.js:123:10:123:19 | request.id |
+| srv/service1.js:127:10:127:30 | request ... Options |
 | 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 |
-| srv/service3.js:18:29:18:39 | req.headers |
-| srv/service3.js:24:30:24:56 | req.htt ... omeProp |
-| srv/service3.js:25:30:25:55 | req.htt ... omeProp |
-| srv/service3.js:26:30:26:57 | req.htt ... omeProp |
-| srv/service3.js:27:30:27:58 | req.htt ... omeProp |
-| srv/service3.js:28:30:28:58 | req.htt ... omeProp |
-| srv/service3.js:29:30:29:53 | req.htt ... inalUrl |
-| srv/service3.js:30:30:30:50 | req.htt ... ostname |
-| srv/service3.js:31:30:31:57 | req.htt ... eProp") |
-| srv/service3.js:32:30:32:56 | req.htt ... eProp") |
-| 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 |
+| srv/service3.js:19:31:19:40 | req.params |
+| srv/service3.js:31:29:31:39 | req.headers |
+| srv/service3.js:44:30:44:56 | req.htt ... omeProp |
+| srv/service3.js:45:30:45:55 | req.htt ... omeProp |
+| srv/service3.js:46:30:46:57 | req.htt ... omeProp |
+| srv/service3.js:47:30:47:58 | req.htt ... omeProp |
+| srv/service3.js:48:30:48:58 | req.htt ... omeProp |
+| srv/service3.js:49:30:49:53 | req.htt ... inalUrl |
+| srv/service3.js:50:30:50:50 | req.htt ... ostname |
+| srv/service3.js:51:30:51:57 | req.htt ... eProp") |
+| srv/service3.js:52:30:52:56 | req.htt ... eProp") |
+| srv/service3.js:53:31:53:61 | req.htt ... eProp") |
+| srv/service3.js:54:31:54:60 | req.htt ... eProp") |
+| srv/service3.js:60:29:60:34 | req.id |
+| srv/service3.js:72:29:72:45 | req._queryOptions |
+| srv/service3.js:112:10:112:21 | request.data |
+| srv/service3.js:116:10:116:23 | request.params |
+| srv/service3.js:120:10:120:24 | request.headers |
+| srv/service3.js:124:10:124:19 | request.id |
+| srv/service3.js:128:10:128:30 | request ... Options |

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

@@ -3,24 +3,44 @@ const cds = require("@sap/cds");
 /* Emit a "Received1" event upon receiving a READ request on its entity. */
 module.exports = class Service1 extends cds.ApplicationService {
   init() {
-    this.on("send1", async (req) => {
+    this.on("send11", async (req) => {
       const { messageToPass } = req.data;  // UNSAFE: Taint source, Exposed service
       const Service2 = await cds.connect.to("service-2");
       Service2.send("send2", { messageToPass });
     });
 
-    this.on("send2", async (req) => {
+    this.on("send12", async (req) => {
+      const reqData = getReqData(req);
+      const { messageToPass } = reqData;
+      const Service2 = await cds.connect.to("service-2");
+      Service2.send("send2", { messageToPass });
+    });
+
+    this.on("send21", async (req) => {
       const [ messageToPass ] = req.params;  // UNSAFE: Taint source, Exposed service
       const Service2 = await cds.connect.to("service-2");
       Service2.send("send2", { messageToPass });
     });
 
-    this.on("send3", async (req) => {
+    this.on("send22", async (req) => {
+      const [ messageToPass ] = getReqParams(req);  // UNSAFE: Taint source, Exposed service
+      const Service2 = await cds.connect.to("service-2");
+      Service2.send("send2", { messageToPass });
+    });
+
+    this.on("send31", async (req) => {
       const messageToPass = req.headers["user-agent"];  // UNSAFE: Taint source, Exposed service
       const Service2 = await cds.connect.to("service-2");
       Service2.send("send2", { messageToPass });
     });
 
+    this.on("send32", async (req) => {
+      const reqHeaders = getReqHeaders(req);
+      const messageToPass = reqHeaders["user-agent"];  // UNSAFE: Taint source, Exposed service
+      const Service2 = await cds.connect.to("service-2");
+      Service2.send("send2", { messageToPass });
+    });
+
     this.on("send4", async (req) => {
       const messageToPass1 = req.http.req.query.someProp;  // UNSAFE: Taint source, Exposed service
       const messageToPass2 = req.http.req.body.someProp;  // UNSAFE: Taint source, Exposed service
@@ -37,18 +57,30 @@ module.exports = class Service1 extends cds.ApplicationService {
       Service2.send("send2", { messageToPass1 });
     });
 
-    this.on("send5", async (req) => {
+    this.on("send51", async (req) => {
       const messageToPass = req.id;  // UNSAFE: Taint source, Exposed service
       const Service2 = await cds.connect.to("service-2");
       Service2.send("send2", { messageToPass });
     });
 
-    this.on("send6", async (req) => {
+    this.on("send52", async (req) => {
+      const messageToPass = getReqId(req);  // UNSAFE: Taint source, Exposed service
+      const Service2 = await cds.connect.to("service-2");
+      Service2.send("send2", { messageToPass });
+    });
+
+    this.on("send61", async (req) => {
       const messageToPass = req._queryOptions;  // UNSAFE: Taint source, Exposed service
       const Service2 = await cds.connect.to("service-2");
       Service2.send("send2", { messageToPass });
     });
 
+    this.on("send62", async (req) => {
+      const messageToPass = getReqQueryOptions(req);  // 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.locale;  // SAFE: Not a taint source, Exposed service
       const Service2 = await cds.connect.to("service-2");
@@ -74,3 +106,23 @@ module.exports = class Service1 extends cds.ApplicationService {
     });
   }
 };
+
+function getReqData(request) {
+  return request.data;  // UNSAFE: Taint source, Exposed service
+}
+
+function getReqParams(request) {
+  return request.params;  // UNSAFE: Taint source, Exposed service
+}
+
+function getReqHeaders(request) {
+  return request.headers;  // UNSAFE: Taint source, Exposed service
+}
+
+function getReqId(request) {
+  return request.id;  // UNSAFE: Taint source, Exposed service
+}
+
+function getReqQueryOptions(request) {
+  return request._queryOptions;  // UNSAFE: Taint source, Exposed service
+}