Separate CAP from non-CAP alerts

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

@@ -106,7 +106,7 @@ module CDS {
    * ```
    * not sure how else to know which service is registering the handler
    */
-  class RequestSource extends EventPhaseNodeParam {
+  class RequestSource extends EventPhaseNodeParam, RemoteFlowSource {
     RequestSource() {
       // TODO : consider  - do we need to actually ever know which service the handler is associated to?
       exists(UserDefinedApplicationService svc, FunctionNode init |
@@ -119,6 +119,8 @@ module CDS {
         this.getEventPhaseNode().getEnclosingFunction() = pa.getFunction()
       )
     }
+
+    override string getSourceType() { result = "CAP request source" }
   }
 
   class ApplicationService extends API::Node {

javascript/frameworks/cap/src/sqlinjection/SqlInjection.md → javascript/frameworks/cap/src/cqlinjection/CqlInjection.md

@@ -1,4 +1,4 @@
-# SQL Injection
+# CQL query built from user-controlled sources
 
 If a database query is built from user-provided data without sufficient sanitization, a malicious user may be able to run malicious database queries.
 

javascript/frameworks/cap/src/sqlinjection/SqlInjection.ql → javascript/frameworks/cap/src/cqlinjection/CqlInjection.ql

@@ -1,6 +1,6 @@
 /**
- * @name Database query built from user-controlled sources with additional heuristic sources
- * @description Building a database query from user-controlled sources is vulnerable to insertion of
+ * @name CQL query built from user-controlled sources
+ * @description Building a CQL query from user-controlled sources is vulnerable to insertion of
  *              malicious code by the user.
  * @kind path-problem
  * @problem.severity error
@@ -13,18 +13,18 @@
 import javascript
 import DataFlow::PathGraph
 import semmle.javascript.security.dataflow.SqlInjectionCustomizations::SqlInjection
-import advanced_security.javascript.frameworks.cap.CDS
 import advanced_security.javascript.frameworks.cap.CQL
 
-class Configuration extends TaintTracking::Configuration {
-  Configuration() { this = "CapSqlInjection" }
+class CqlIConfiguration extends TaintTracking::Configuration {
+  CqlIConfiguration() { this = "CqlInjection" }
 
-  override predicate isSource(DataFlow::Node source) {
-    source instanceof Source or source instanceof CDS::RequestSource
-  }
+  override predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof CQL::CQLSink }
 
-  override predicate isSink(DataFlow::Node sink) {
-    sink instanceof Sink or sink instanceof CQL::CQLSink
+  override predicate isSanitizer(DataFlow::Node node) {
+    super.isSanitizer(node) or
+    node instanceof Sanitizer
   }
 
   override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
@@ -42,7 +42,7 @@ class Configuration extends TaintTracking::Configuration {
   }
 }
 
-from Configuration sql, DataFlow::PathNode source, DataFlow::PathNode sink
+from CqlIConfiguration sql, DataFlow::PathNode source, DataFlow::PathNode sink
 where sql.hasFlowPath(source, sink)
 select sink.getNode(), source, sink, "This query depends on a $@.", source.getNode(),
   "user-provided value"

javascript/frameworks/cap/src/loginjection/LogInjection.md

@@ -1,4 +1,4 @@
-# Log Injection
+# CAP Log Injection
 
 If unsanitized user input is written to a log entry using the CAP Node.js logging API, a malicious user may be able to forge new log entries.
 

javascript/frameworks/cap/src/loginjection/LogInjection.ql

@@ -1,10 +1,10 @@
 /**
- * @name Uncontrolled data in logging call
+ * @name CAP Log injection
  * @description Building log entries from user-controlled sources is vulnerable to
  *              insertion of forged log entries by a malicious user.
  * @kind path-problem
  * @problem.severity error
- * @security-severity 7.8
+ * @security-severity 6.1
  * @precision medium
  * @id js/cap-log-injection
  * @tags security
@@ -16,16 +16,19 @@ import semmle.javascript.security.dataflow.LogInjectionQuery
 import advanced_security.javascript.frameworks.cap.CDS
 
 /**
- * A source of remote user controlled input.
+ * A taint-tracking configuration for untrusted user input used in log entries.
  */
-class CapRemoteSource extends Source, CDS::RequestSource { }
+class CapLogIConfiguration extends TaintTracking::Configuration {
+  CapLogIConfiguration() { this = "CapLogInjection" }
 
-/**
- * An argument to a logging mechanism.
- */
-class CapLoggingSink extends Sink, CDS::CdsLogSink { }
+  override predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof CDS::CdsLogSink }
+
+  override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
+}
 
-from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
+from CapLogIConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink
 where config.hasFlowPath(source, sink)
 select sink.getNode(), source, sink, "Log entry depends on a $@.", source.getNode(),
   "user-provided value"

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

@@ -0,0 +1,84 @@
+nodes
+| cqlinjection.js:7:34:7:36 | req |
+| cqlinjection.js:7:34:7:36 | req |
+| cqlinjection.js:8:13:8:30 | { book, quantity } |
+| cqlinjection.js:8:13:8:41 | book |
+| cqlinjection.js:8:15:8:18 | book |
+| cqlinjection.js:8:34:8:36 | req |
+| cqlinjection.js:8:34:8:41 | req.data |
+| cqlinjection.js:12:11:12:56 | query |
+| cqlinjection.js:12:19:12:56 | SELECT. ... book}`) |
+| cqlinjection.js:12:44:12:55 | `ID=${book}` |
+| cqlinjection.js:12:50:12:53 | book |
+| cqlinjection.js:13:36:13:40 | query |
+| cqlinjection.js:13:36:13:40 | query |
+| cqlinjection.js:15:27:15:64 | SELECT. ... book}`) |
+| cqlinjection.js:15:27:15:64 | SELECT. ... book}`) |
+| cqlinjection.js:15:52:15:63 | `ID=${book}` |
+| cqlinjection.js:15:58:15:61 | book |
+| cqlinjection.js:17:11:17:57 | query2 |
+| cqlinjection.js:17:20:17:57 | SELECT. ... + book) |
+| cqlinjection.js:17:45:17:56 | 'ID=' + book |
+| cqlinjection.js:17:53:17:56 | book |
+| cqlinjection.js:18:37:18:42 | query2 |
+| cqlinjection.js:18:37:18:42 | query2 |
+| cqlinjection.js:20:27:20:64 | SELECT. ... + book) |
+| cqlinjection.js:20:27:20:64 | SELECT. ... + book) |
+| cqlinjection.js:20:52:20:63 | 'ID=' + book |
+| cqlinjection.js:20:60:20:63 | book |
+| cqlinjection.js:27:11:27:62 | cqn |
+| cqlinjection.js:27:17:27:62 | CQL`SEL ...  + book |
+| cqlinjection.js:27:59:27:62 | book |
+| cqlinjection.js:28:39:28:41 | cqn |
+| cqlinjection.js:28:39:28:41 | cqn |
+| cqlinjection.js:30:11:30:60 | cqn1 |
+| cqlinjection.js:30:18:30:60 | cds.par ... + book) |
+| cqlinjection.js:30:32:30:59 | `SELECT ...  + book |
+| cqlinjection.js:30:56:30:59 | book |
+| cqlinjection.js:31:39:31:42 | cqn1 |
+| cqlinjection.js:31:39:31:42 | cqn1 |
+edges
+| cqlinjection.js:7:34:7:36 | req | cqlinjection.js:8:34:8:36 | req |
+| cqlinjection.js:7:34:7:36 | req | cqlinjection.js:8:34:8:36 | req |
+| cqlinjection.js:8:13:8:30 | { book, quantity } | cqlinjection.js:8:15:8:18 | book |
+| cqlinjection.js:8:13:8:41 | book | cqlinjection.js:12:50:12:53 | book |
+| cqlinjection.js:8:13:8:41 | book | cqlinjection.js:15:58:15:61 | book |
+| cqlinjection.js:8:13:8:41 | book | cqlinjection.js:17:53:17:56 | book |
+| cqlinjection.js:8:13:8:41 | book | cqlinjection.js:20:60:20:63 | book |
+| cqlinjection.js:8:13:8:41 | book | cqlinjection.js:27:59:27:62 | book |
+| cqlinjection.js:8:13:8:41 | book | cqlinjection.js:30:56:30:59 | book |
+| cqlinjection.js:8:15:8:18 | book | cqlinjection.js:8:13:8:41 | book |
+| cqlinjection.js:8:34:8:36 | req | cqlinjection.js:8:34:8:41 | req.data |
+| cqlinjection.js:8:34:8:41 | req.data | cqlinjection.js:8:13:8:30 | { book, quantity } |
+| cqlinjection.js:12:11:12:56 | query | cqlinjection.js:13:36:13:40 | query |
+| cqlinjection.js:12:11:12:56 | query | cqlinjection.js:13:36:13:40 | query |
+| cqlinjection.js:12:19:12:56 | SELECT. ... book}`) | cqlinjection.js:12:11:12:56 | query |
+| cqlinjection.js:12:44:12:55 | `ID=${book}` | cqlinjection.js:12:19:12:56 | SELECT. ... book}`) |
+| cqlinjection.js:12:50:12:53 | book | cqlinjection.js:12:44:12:55 | `ID=${book}` |
+| cqlinjection.js:15:52:15:63 | `ID=${book}` | cqlinjection.js:15:27:15:64 | SELECT. ... book}`) |
+| cqlinjection.js:15:52:15:63 | `ID=${book}` | cqlinjection.js:15:27:15:64 | SELECT. ... book}`) |
+| cqlinjection.js:15:58:15:61 | book | cqlinjection.js:15:52:15:63 | `ID=${book}` |
+| cqlinjection.js:17:11:17:57 | query2 | cqlinjection.js:18:37:18:42 | query2 |
+| cqlinjection.js:17:11:17:57 | query2 | cqlinjection.js:18:37:18:42 | query2 |
+| cqlinjection.js:17:20:17:57 | SELECT. ... + book) | cqlinjection.js:17:11:17:57 | query2 |
+| cqlinjection.js:17:45:17:56 | 'ID=' + book | cqlinjection.js:17:20:17:57 | SELECT. ... + book) |
+| cqlinjection.js:17:53:17:56 | book | cqlinjection.js:17:45:17:56 | 'ID=' + book |
+| cqlinjection.js:20:52:20:63 | 'ID=' + book | cqlinjection.js:20:27:20:64 | SELECT. ... + book) |
+| cqlinjection.js:20:52:20:63 | 'ID=' + book | cqlinjection.js:20:27:20:64 | SELECT. ... + book) |
+| cqlinjection.js:20:60:20:63 | book | cqlinjection.js:20:52:20:63 | 'ID=' + book |
+| cqlinjection.js:27:11:27:62 | cqn | cqlinjection.js:28:39:28:41 | cqn |
+| cqlinjection.js:27:11:27:62 | cqn | cqlinjection.js:28:39:28:41 | cqn |
+| cqlinjection.js:27:17:27:62 | CQL`SEL ...  + book | cqlinjection.js:27:11:27:62 | cqn |
+| cqlinjection.js:27:59:27:62 | book | cqlinjection.js:27:17:27:62 | CQL`SEL ...  + book |
+| cqlinjection.js:30:11:30:60 | cqn1 | cqlinjection.js:31:39:31:42 | cqn1 |
+| cqlinjection.js:30:11:30:60 | cqn1 | cqlinjection.js:31:39:31:42 | cqn1 |
+| cqlinjection.js:30:18:30:60 | cds.par ... + book) | cqlinjection.js:30:11:30:60 | cqn1 |
+| cqlinjection.js:30:32:30:59 | `SELECT ...  + book | cqlinjection.js:30:18:30:60 | cds.par ... + book) |
+| cqlinjection.js:30:56:30:59 | book | cqlinjection.js:30:32:30:59 | `SELECT ...  + book |
+#select
+| cqlinjection.js:13:36:13:40 | query | cqlinjection.js:7:34:7:36 | req | cqlinjection.js:13:36:13:40 | query | This query depends on a $@. | cqlinjection.js:7:34:7:36 | req | user-provided value |
+| cqlinjection.js:15:27:15:64 | SELECT. ... book}`) | cqlinjection.js:7:34:7:36 | req | cqlinjection.js:15:27:15:64 | SELECT. ... book}`) | This query depends on a $@. | cqlinjection.js:7:34:7:36 | req | user-provided value |
+| cqlinjection.js:18:37:18:42 | query2 | cqlinjection.js:7:34:7:36 | req | cqlinjection.js:18:37:18:42 | query2 | This query depends on a $@. | cqlinjection.js:7:34:7:36 | req | user-provided value |
+| cqlinjection.js:20:27:20:64 | SELECT. ... + book) | cqlinjection.js:7:34:7:36 | req | cqlinjection.js:20:27:20:64 | SELECT. ... + book) | This query depends on a $@. | cqlinjection.js:7:34:7:36 | req | user-provided value |
+| cqlinjection.js:28:39:28:41 | cqn | cqlinjection.js:7:34:7:36 | req | cqlinjection.js:28:39:28:41 | cqn | This query depends on a $@. | cqlinjection.js:7:34:7:36 | req | user-provided value |
+| cqlinjection.js:31:39:31:42 | cqn1 | cqlinjection.js:7:34:7:36 | req | cqlinjection.js:31:39:31:42 | cqn1 | This query depends on a $@. | cqlinjection.js:7:34:7:36 | req | user-provided value |

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

@@ -0,0 +1,50 @@
+import cds from '@sap/cds'
+const { Books } = cds.entities('sap.capire.bookshop')
+
+class SampleVulnService extends cds.ApplicationService {
+  init() {
+    // contains a sample CQL injection
+    this.on('submitOrder', async req => {
+      const { book, quantity } = req.data
+
+      let { stock } = await SELECT`stock`.from(Books, book)
+
+      let query = SELECT.from`Books`.where(`ID=${book}`)
+      let books = await cds.db.run(query) // CQL injection alert
+
+      let books11 = await SELECT.from`Books`.where(`ID=${book}`) // CQL injection alert
+
+      let query2 = SELECT.from`Books`.where('ID=' + book)
+      let books2 = await cds.db.run(query2) // CQL injection alert
+
+      let books22 = await SELECT.from`Books`.where('ID=' + book) // CQL injection alert
+
+      let books3 = await SELECT.from`Books`.where`ID=${book}` //safe
+
+      let id = 2
+      let books33 = await SELECT.from`Books`.where('ID=' + id) //safe
+
+      let cqn = CQL`SELECT col1, col2, col3 from Books` + book
+      let books222 = await cds.db.run(cqn) // CQL injection alert
+
+      let cqn1 = cds.parse.cql(`SELECT * from Books` + book)
+      let books111 = await cds.db.run(cqn1) // CQL injection alert
+
+      const pg = require("pg"),
+        pool = new pg.Pool(config);
+      pool.query(req.params.category, [], function (err, results) { // non-CQL injection alert from CAP source
+        // process results
+      });
+
+      const app = require("express")();
+      app.get("search", function handler(req2, res) {
+        pool.query(req2.params.category, [], function (err, results) { // non-CQL injection alert from non-CAP source
+          // process results
+        });
+      });
+
+      return super.init()
+    })
+  }
+}
+export { SampleVulnService }

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

@@ -0,0 +1 @@
+cqlinjection/CqlInjection.ql