Factor out sink definition in a class and associate location reporting to it

This avoids duplicating the sink definition
in `getQueryOfSink` and also in the `isSink` of
the configuration.

Author: jeongsoolee09

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

@@ -4,6 +4,14 @@ import advanced_security.javascript.frameworks.cap.CQL
 import advanced_security.javascript.frameworks.cap.RemoteFlowSources
 import advanced_security.javascript.frameworks.cap.dataflow.FlowSteps
 
+abstract class CqlInjectionSink extends DataFlow::Node {
+  /**
+   * Gets the data flow node that represents the query being run, for
+   * accurate reporting.
+   */
+  abstract DataFlow::Node getQuery();
+}
+
 /**
  * A CQL clause parameterized with a string concatentation expression.
  */
@@ -26,12 +34,35 @@ class CqlClauseWithStringConcatParameter instanceof CqlClause {
   string toString() { result = super.toString() }
 }
 
+class AwaitCqlClauseWithStringConcatParameter extends CqlInjectionSink {
+  DataFlow::Node queryParameter;
+  DataFlow::Node query;
+  CqlClauseWithStringConcatParameter cqlClauseWithStringConcat;
+
+  AwaitCqlClauseWithStringConcatParameter() {
+    exists(AwaitExpr await |
+      this = await.flow() and
+      await.getOperand() = cqlClauseWithStringConcat.(CqlClause).asExpr()
+    )
+  }
+
+  override DataFlow::Node getQuery() { result = cqlClauseWithStringConcat.(CqlClause).flow() }
+}
+
+class ParameterOfCqlRunMethodQueryArgument extends CqlInjectionSink {
+  CqlRunMethodCall cqlRunMethodCall;
+
+  ParameterOfCqlRunMethodQueryArgument() { this = cqlRunMethodCall.getAQueryParameter() }
+
+  override DataFlow::Node getQuery() { result = this }
+}
+
 /**
  * A CQL shortcut method call (`read`, `create`, ...) parameterized with a string
  * concatenation expression.
  */
 class CqlShortcutMethodCallWithStringConcat instanceof CqlShortcutMethodCall {
-  DataFlow::Node stringConcatParameter; 
+  DataFlow::Node stringConcatParameter;
 
   CqlShortcutMethodCallWithStringConcat() {
     stringConcatParameter = super.getAQueryParameter() and
@@ -45,6 +76,16 @@ class CqlShortcutMethodCallWithStringConcat instanceof CqlShortcutMethodCall {
   DataFlow::Node getStringConcatParameter() { result = stringConcatParameter }
 }
 
+class StringConcatParameterOfCqlShortcutMethodCall extends CqlInjectionSink {
+  CqlShortcutMethodCallWithStringConcat cqlShortcutMethodCallWithStringConcat;
+
+  StringConcatParameterOfCqlShortcutMethodCall() {
+    this = cqlShortcutMethodCallWithStringConcat.getStringConcatParameter()
+  }
+
+  override DataFlow::Node getQuery() { result = cqlShortcutMethodCallWithStringConcat }
+}
+
 /**
  * A CQL parser call (`cds.ql`, `cds.parse.cql`, ...) parameterized with a string
  * conatenation expression.
@@ -65,20 +106,7 @@ class CqlInjectionConfiguration extends TaintTracking::Configuration {
 
   override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
 
-  override predicate isSink(DataFlow::Node node) {
-    exists(CqlRunMethodCall cqlRunMethodCall |
-      node = cqlRunMethodCall.(CqlRunMethodCall).getAQueryParameter()
-    )
-    or
-    exists(CqlShortcutMethodCallWithStringConcat queryRunnerCall |
-      node = queryRunnerCall.getStringConcatParameter()
-    )
-    or
-    exists(AwaitExpr await, CqlClauseWithStringConcatParameter cqlClauseWithStringConcat |
-      node = await.flow() and
-      await.getOperand() = cqlClauseWithStringConcat.(CqlClause).asExpr()
-    )
-  }
+  override predicate isSink(DataFlow::Node node) { node instanceof CqlInjectionSink }
 
   override predicate isSanitizer(DataFlow::Node node) { node instanceof SqlInjection::Sanitizer }
 

javascript/frameworks/cap/src/cqlinjection/CqlInjection.ql

@@ -14,25 +14,7 @@ import javascript
 import DataFlow::PathGraph
 import advanced_security.javascript.frameworks.cap.CAPCqlInjectionQuery
 
-DataFlow::Node getQueryOfSink(DataFlow::Node sink) {
-  exists(CqlRunMethodCall cqlRunMethodCall |
-    sink = cqlRunMethodCall.(CqlRunMethodCall).getAQueryParameter() and
-    result = sink
-  )
-  or
-  exists(CqlShortcutMethodCallWithStringConcat shortcutCall |
-    sink = shortcutCall.(CqlQueryRunnerCall).getAQueryParameter() and
-    result = shortcutCall
-  )
-  or
-  exists(AwaitExpr await, CqlClauseWithStringConcatParameter cqlClauseWithStringConcat |
-    sink = await.flow() and
-    await.getOperand() = cqlClauseWithStringConcat.(CqlClause).asExpr() and
-    result = cqlClauseWithStringConcat.(CqlClause).flow()
-  )
-}
-
 from CqlInjectionConfiguration sql, DataFlow::PathNode source, DataFlow::PathNode sink
 where sql.hasFlowPath(source, sink)
-select getQueryOfSink(sink.getNode()), source, sink, "This CQL query depends on a $@.",
-  source.getNode(), "user-provided value"
+select sink.getNode().(CqlInjectionSink).getQuery(), source, sink,
+  "This CQL query depends on a $@.", source.getNode(), "user-provided value"

javascript/frameworks/cap/test/queries/cqlinjection/cqlinjection.expected

@@ -1,6 +1,6 @@
 WARNING: module 'PathGraph' has been deprecated and may be removed in future (CqlInjection.ql:14,8-27)
-WARNING: type 'PathNode' has been deprecated and may be removed in future (CqlInjection.ql:35,37-55)
-WARNING: type 'PathNode' has been deprecated and may be removed in future (CqlInjection.ql:35,64-82)
+WARNING: type 'PathNode' has been deprecated and may be removed in future (CqlInjection.ql:17,37-55)
+WARNING: type 'PathNode' has been deprecated and may be removed in future (CqlInjection.ql:17,64-82)
 nodes
 | srv/service1.js:6:33:6:35 | req |
 | srv/service1.js:6:33:6:35 | req |