Merge branch 'main' into dependabot/npm_and_yarn/javascript/frameworks/ui5/test/queries/UI5Xss/xss-book-example/npm_and_yarn-775d964dd3

Author: jeongsoolee09

.github/workflows/javascript.sarif.expected

@@ -1,6 +1,6 @@
 ---
 library: true
 name: advanced-security/javascript-sap-cap-models
-version: 2.0.0
+version: 2.2.0
 extensionTargets:
   codeql/javascript-all: "^2.4.0"

javascript/frameworks/cap/ext/qlpack.yml

@@ -166,16 +166,14 @@ class CqlClauseParserCallWithStringConcat instanceof CqlClauseParserCall {
  * instead (notice the lack of parentheses around the template literal), then the `where` call
  * becomes a parser call of the template literal following it and thus acts as a sanitizer.
  */
-class CqlInjectionConfiguration extends TaintTracking::Configuration {
-  CqlInjectionConfiguration() { this = "CQL injection from untrusted data" }
+module CqlInjectionConfiguration implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
 
-  override predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
+  predicate isSink(DataFlow::Node node) { node instanceof CqlInjectionSink }
 
-  override predicate isSink(DataFlow::Node node) { node instanceof CqlInjectionSink }
+  predicate isBarrier(DataFlow::Node node) { node instanceof SqlInjection::Sanitizer }
 
-  override predicate isSanitizer(DataFlow::Node node) { node instanceof SqlInjection::Sanitizer }
-
-  override predicate isAdditionalTaintStep(DataFlow::Node start, DataFlow::Node end) {
+  predicate isAdditionalFlowStep(DataFlow::Node start, DataFlow::Node end) {
     /*
      * 1. Given a call to a CQL parser, jump from the argument to the parser call itself.
      */

javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CAPCqlInjectionQuery.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)
@@ -43,19 +47,36 @@ class CdsLogSink extends DataFlow::Node {
   }
 }
 
-class CAPLogInjectionConfiguration extends LogInjectionConfiguration {
-  override predicate isSource(DataFlow::Node start) {
-    super.isSource(start)
+module CAPLogInjectionConfiguration implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node start) {
+    LogInjectionConfig::isSource(start)
     or
     start instanceof RemoteFlowSource
   }
 
-  override predicate isBarrier(DataFlow::Node node) {
+  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"
+      )
     )
   }
 
-  override predicate isSink(DataFlow::Node end) { end instanceof CdsLogSink }
+  predicate isSink(DataFlow::Node end) { end instanceof CdsLogSink }
 }

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

@@ -7,13 +7,21 @@
 import javascript
 import advanced_security.javascript.frameworks.cap.CDSUtils
 
-abstract class UtilsAccessedPathSink extends DataFlow::Node { }
+abstract class UtilsSink extends DataFlow::Node {
+  abstract string sinkType();
+}
 
-abstract class UtilsControlledDataSink extends DataFlow::Node { }
+abstract class UtilsAccessedPathSink extends UtilsSink {
+  override string sinkType() { result = "unrestricted file read" }
+}
 
-abstract class UtilsControlledPathSink extends DataFlow::Node { }
+abstract class UtilsControlledDataSink extends UtilsSink {
+  override string sinkType() { result = "tainted data being written to a file" }
+}
 
-abstract class UtilsExtraFlow extends DataFlow::Node { }
+abstract class UtilsControlledPathSink extends UtilsSink {
+  override string sinkType() { result = "unrestricted file operations" }
+}
 
 /**
  * This represents the data in calls as follows:
@@ -67,21 +75,3 @@ class ControlledInputPath extends UtilsControlledPathSink {
     exists(DirectoryReaders dr | dr.getPath() = this)
   }
 }
-
-/**
- * This represents calls where the taint flows through the call. e.g.
- * ```javascript
- * let dir = isdir ('app')
- * ```
- */
-class AdditionalFlowStep extends UtilsExtraFlow {
-  AdditionalFlowStep() {
-    exists(PathConverters pc | pc.getPath() = this)
-    or
-    exists(PathPredicates pr | pr.getPath() = this)
-  }
-
-  DataFlow::CallNode getOutgoingNode() { result = this }
-
-  DataFlow::Node getIngoingNode() { result = this.(DataFlow::CallNode).getAnArgument() }
-}

javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CAPPathInjectionQuery.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,14 +423,15 @@ 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`
+ * ```
+ * 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 }
 }
@@ -506,7 +574,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 +607,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 +634,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/CDS.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/lib/advanced_security/javascript/frameworks/cap/RemoteFlowSources.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)
+}