Make property read interprocedural and make the barrier more conservative

Author: jeongsoolee09

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,21 @@ module CAPLogInjectionConfiguration implements DataFlow::ConfigSig {
   }
 
   predicate isBarrier(DataFlow::Node node) {
+    /*
+     * This predicate includes cases such as:
+     * 1. An CDS entity element lacking a type annotation.
+     *   - Possibly because it relies on a common aspect.
+     * 2. An 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. */
+      handlerParameterData.getType() != ["cds.String", "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)
+}