Add CDS Utils path injection query

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

@@ -7,11 +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 UtilsAccessedPathSink extends UtilsSink {
+  override string sinkType() { result = "unrestricted file read" }
+}
 
-abstract class UtilsControlledDataSink extends DataFlow::Node { }
+abstract class UtilsControlledDataSink extends UtilsSink {
+  override string sinkType() { result = "tainted data being written to a file" }
+}
 
-abstract class UtilsControlledPathSink extends DataFlow::Node { }
+abstract class UtilsControlledPathSink extends UtilsSink {
+  override string sinkType() { result = "unrestricted file operations" }
+}
 
 abstract class UtilsExtraFlow extends DataFlow::Node { }
 
@@ -74,14 +84,16 @@ class ControlledInputPath extends UtilsControlledPathSink {
  * let dir = isdir ('app')
  * ```
  */
-class AdditionalFlowStep extends UtilsExtraFlow {
-  AdditionalFlowStep() {
-    exists(PathConverters pc | pc.getPath() = this)
+class CDSAdditionalFlowStep extends UtilsExtraFlow {
+  DataFlow::CallNode outNode;
+
+  CDSAdditionalFlowStep() {
+    exists(PathConverters pc | pc.getPath() = this and outNode = pc)
     or
-    exists(PathPredicates pr | pr.getPath() = this)
+    exists(PathPredicates pr | pr.getPath() = this and outNode = pr)
   }
 
-  DataFlow::CallNode getOutgoingNode() { result = this }
+  DataFlow::CallNode getOutgoingNode() { result = outNode }
 
-  DataFlow::Node getIngoingNode() { result = this.(DataFlow::CallNode).getAnArgument() }
+  DataFlow::Node getIngoingNode() { result = this }
 }

javascript/frameworks/cap/src/path-traversal/PathInjection.md

@@ -0,0 +1,51 @@
+# CAP CDS Utils used with user-controlled sources
+
+If a path is constructed from user-provided input without sufficient sanitization, a malicious user may be able to manipulate the contents of the filesystem without proper authorization.
+
+Additionally if user-provided input is used to create file contents this can also result in a malicious user manipulating the filesystem in an unchecked way.
+
+## Recommendation
+
+CAP applications using CDS Utils should not use user-provided input without sanitization.
+
+## Examples
+
+This CAP service directly uses user-provided input to construct a path.
+
+``` javascript
+const cds = require("@sap/cds");
+const { rm } = cds.utils
+
+module.exports = class Service1 extends cds.ApplicationService {
+
+    init() {
+        this.on("send1", async (req) => {
+            let userinput = req.data
+            await rm(userinput, 'db', 'data') // Path injection alert
+        }
+    }
+}
+```
+
+This CAP service directly uses user-provided input to add content to a file.
+
+``` javascript
+const cds = require("@sap/cds");
+const { rm } = cds.utils
+
+module.exports = class Service1 extends cds.ApplicationService {
+
+    init() {
+        this.on("send1", async (req) => {
+            let userinput = req.data
+            await write(userinput).to('db/data') // Path injection alert
+        }
+    }
+}
+
+```
+
+## References
+
+- OWASP 2021: [Injection](https://owasp.org/Top10/A03_2021-Injection/).
+- SAP CAP CDS Utils : [Documentation](https://cap.cloud.sap/docs/node.js/cds-utils).

javascript/frameworks/cap/src/path-traversal/PathInjection.ql

@@ -0,0 +1,41 @@
+/**
+ * @name Use of user controlled input in CAP CDS file system utilies
+ * @description Using unchecked user controlled values can allow an
+ *              attacker to affect paths constructed and accessed in
+ *              the filesystem.
+ * @kind path-problem
+ * @problem.severity warning
+ * @security-severity 7.5
+ * @precision medium
+ * @id js/cap-path-injection
+ * @tags security
+ *       external/cwe/cwe-020
+ *       external/cwe/cwe-022
+ */
+
+import javascript
+import advanced_security.javascript.frameworks.cap.CAPPathInjectionQuery
+import advanced_security.javascript.frameworks.cap.RemoteFlowSources
+
+module PathInjectionConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof UtilsSink }
+
+  predicate isAdditionalFlowStep(DataFlow::Node nodein, DataFlow::Node nodeout) {
+    exists(CDSAdditionalFlowStep step |
+      step.getIngoingNode() = nodein and
+      step.getOutgoingNode() = nodeout
+    )
+  }
+}
+
+module PathInjectionConfigFlow = TaintTracking::Global<PathInjectionConfig>;
+
+import PathInjectionConfigFlow::PathGraph
+
+from PathInjectionConfigFlow::PathNode source, PathInjectionConfigFlow::PathNode sink
+where PathInjectionConfigFlow::flowPath(source, sink)
+select sink, source, sink,
+  "This CDS utils usage relies on user-provided value and can result in " +
+    sink.getNode().(UtilsSink).sinkType() + "."

javascript/frameworks/cap/test/queries/path-traversal/pathinjection.expected

@@ -0,0 +1,100 @@
+edges
+| pathinjection.js:8:19:8:38 | userinput | pathinjection.js:31:26:31:34 | userinput | provenance |  |
+| pathinjection.js:8:19:8:38 | userinput | pathinjection.js:33:38:33:46 | userinput | provenance |  |
+| pathinjection.js:8:19:8:38 | userinput | pathinjection.js:34:24:34:32 | userinput | provenance |  |
+| pathinjection.js:8:19:8:38 | userinput | pathinjection.js:36:44:36:52 | userinput | provenance |  |
+| pathinjection.js:8:19:8:38 | userinput | pathinjection.js:38:25:38:33 | userinput | provenance |  |
+| pathinjection.js:8:19:8:38 | userinput | pathinjection.js:40:26:40:34 | userinput | provenance |  |
+| pathinjection.js:8:19:8:38 | userinput | pathinjection.js:41:26:41:34 | userinput | provenance |  |
+| pathinjection.js:8:19:8:38 | userinput | pathinjection.js:43:25:43:33 | userinput | provenance |  |
+| pathinjection.js:8:19:8:38 | userinput | pathinjection.js:44:25:44:33 | userinput | provenance |  |
+| pathinjection.js:8:19:8:38 | userinput | pathinjection.js:46:26:46:34 | userinput | provenance |  |
+| pathinjection.js:8:19:8:38 | userinput | pathinjection.js:47:26:47:34 | userinput | provenance |  |
+| pathinjection.js:8:19:8:38 | userinput | pathinjection.js:49:22:49:30 | userinput | provenance |  |
+| pathinjection.js:8:19:8:38 | userinput | pathinjection.js:50:22:50:30 | userinput | provenance |  |
+| pathinjection.js:8:31:8:38 | req.data | pathinjection.js:8:19:8:38 | userinput | provenance |  |
+| pathinjection.js:9:19:9:44 | userinputtwo | pathinjection.js:37:25:37:36 | userinputtwo | provenance |  |
+| pathinjection.js:9:34:9:44 | req.headers | pathinjection.js:9:19:9:44 | userinputtwo | provenance |  |
+| pathinjection.js:10:19:10:45 | userinputthree | pathinjection.js:12:38:12:51 | userinputthree | provenance |  |
+| pathinjection.js:10:19:10:45 | userinputthree | pathinjection.js:14:47:14:60 | userinputthree | provenance |  |
+| pathinjection.js:10:19:10:45 | userinputthree | pathinjection.js:16:34:16:47 | userinputthree | provenance |  |
+| pathinjection.js:10:19:10:45 | userinputthree | pathinjection.js:18:34:18:47 | userinputthree | provenance |  |
+| pathinjection.js:10:19:10:45 | userinputthree | pathinjection.js:20:35:20:48 | userinputthree | provenance |  |
+| pathinjection.js:10:36:10:45 | req.params | pathinjection.js:10:19:10:45 | userinputthree | provenance |  |
+| pathinjection.js:12:19:12:52 | taint1 | pathinjection.js:22:36:22:41 | taint1 | provenance |  |
+| pathinjection.js:12:28:12:52 | decodeU ... tthree) | pathinjection.js:12:19:12:52 | taint1 | provenance |  |
+| pathinjection.js:12:38:12:51 | userinputthree | pathinjection.js:12:28:12:52 | decodeU ... tthree) | provenance | Config |
+| pathinjection.js:14:19:14:61 | taint2 | pathinjection.js:24:40:24:45 | taint2 | provenance |  |
+| pathinjection.js:14:28:14:61 | decodeU ... tthree) | pathinjection.js:14:19:14:61 | taint2 | provenance |  |
+| pathinjection.js:14:47:14:60 | userinputthree | pathinjection.js:14:28:14:61 | decodeU ... tthree) | provenance | Config |
+| pathinjection.js:16:19:16:48 | taint3 | pathinjection.js:26:34:26:39 | taint3 | provenance |  |
+| pathinjection.js:16:28:16:48 | local(u ... tthree) | pathinjection.js:16:19:16:48 | taint3 | provenance |  |
+| pathinjection.js:16:34:16:47 | userinputthree | pathinjection.js:16:28:16:48 | local(u ... tthree) | provenance | Config |
+| pathinjection.js:18:19:18:48 | taint4 | pathinjection.js:28:34:28:39 | taint4 | provenance |  |
+| pathinjection.js:18:28:18:48 | isdir(u ... tthree) | pathinjection.js:18:19:18:48 | taint4 | provenance |  |
+| pathinjection.js:18:34:18:47 | userinputthree | pathinjection.js:18:28:18:48 | isdir(u ... tthree) | provenance | Config |
+| pathinjection.js:20:19:20:49 | taint5 | pathinjection.js:30:40:30:45 | taint5 | provenance |  |
+| pathinjection.js:20:28:20:49 | isfile( ... tthree) | pathinjection.js:20:19:20:49 | taint5 | provenance |  |
+| pathinjection.js:20:35:20:48 | userinputthree | pathinjection.js:20:28:20:49 | isfile( ... tthree) | provenance | Config |
+nodes
+| pathinjection.js:8:19:8:38 | userinput | semmle.label | userinput |
+| pathinjection.js:8:31:8:38 | req.data | semmle.label | req.data |
+| pathinjection.js:9:19:9:44 | userinputtwo | semmle.label | userinputtwo |
+| pathinjection.js:9:34:9:44 | req.headers | semmle.label | req.headers |
+| pathinjection.js:10:19:10:45 | userinputthree | semmle.label | userinputthree |
+| pathinjection.js:10:36:10:45 | req.params | semmle.label | req.params |
+| pathinjection.js:12:19:12:52 | taint1 | semmle.label | taint1 |
+| pathinjection.js:12:28:12:52 | decodeU ... tthree) | semmle.label | decodeU ... tthree) |
+| pathinjection.js:12:38:12:51 | userinputthree | semmle.label | userinputthree |
+| pathinjection.js:14:19:14:61 | taint2 | semmle.label | taint2 |
+| pathinjection.js:14:28:14:61 | decodeU ... tthree) | semmle.label | decodeU ... tthree) |
+| pathinjection.js:14:47:14:60 | userinputthree | semmle.label | userinputthree |
+| pathinjection.js:16:19:16:48 | taint3 | semmle.label | taint3 |
+| pathinjection.js:16:28:16:48 | local(u ... tthree) | semmle.label | local(u ... tthree) |
+| pathinjection.js:16:34:16:47 | userinputthree | semmle.label | userinputthree |
+| pathinjection.js:18:19:18:48 | taint4 | semmle.label | taint4 |
+| pathinjection.js:18:28:18:48 | isdir(u ... tthree) | semmle.label | isdir(u ... tthree) |
+| pathinjection.js:18:34:18:47 | userinputthree | semmle.label | userinputthree |
+| pathinjection.js:20:19:20:49 | taint5 | semmle.label | taint5 |
+| pathinjection.js:20:28:20:49 | isfile( ... tthree) | semmle.label | isfile( ... tthree) |
+| pathinjection.js:20:35:20:48 | userinputthree | semmle.label | userinputthree |
+| pathinjection.js:22:36:22:41 | taint1 | semmle.label | taint1 |
+| pathinjection.js:24:40:24:45 | taint2 | semmle.label | taint2 |
+| pathinjection.js:26:34:26:39 | taint3 | semmle.label | taint3 |
+| pathinjection.js:28:34:28:39 | taint4 | semmle.label | taint4 |
+| pathinjection.js:30:40:30:45 | taint5 | semmle.label | taint5 |
+| pathinjection.js:31:26:31:34 | userinput | semmle.label | userinput |
+| pathinjection.js:33:38:33:46 | userinput | semmle.label | userinput |
+| pathinjection.js:34:24:34:32 | userinput | semmle.label | userinput |
+| pathinjection.js:36:44:36:52 | userinput | semmle.label | userinput |
+| pathinjection.js:37:25:37:36 | userinputtwo | semmle.label | userinputtwo |
+| pathinjection.js:38:25:38:33 | userinput | semmle.label | userinput |
+| pathinjection.js:40:26:40:34 | userinput | semmle.label | userinput |
+| pathinjection.js:41:26:41:34 | userinput | semmle.label | userinput |
+| pathinjection.js:43:25:43:33 | userinput | semmle.label | userinput |
+| pathinjection.js:44:25:44:33 | userinput | semmle.label | userinput |
+| pathinjection.js:46:26:46:34 | userinput | semmle.label | userinput |
+| pathinjection.js:47:26:47:34 | userinput | semmle.label | userinput |
+| pathinjection.js:49:22:49:30 | userinput | semmle.label | userinput |
+| pathinjection.js:50:22:50:30 | userinput | semmle.label | userinput |
+subpaths
+#select
+| pathinjection.js:22:36:22:41 | taint1 | pathinjection.js:10:36:10:45 | req.params | pathinjection.js:22:36:22:41 | taint1 | This CDS utils usage relies on user-provided value and can result in unrestricted file operations. |
+| pathinjection.js:24:40:24:45 | taint2 | pathinjection.js:10:36:10:45 | req.params | pathinjection.js:24:40:24:45 | taint2 | This CDS utils usage relies on user-provided value and can result in unrestricted file operations. |
+| pathinjection.js:26:34:26:39 | taint3 | pathinjection.js:10:36:10:45 | req.params | pathinjection.js:26:34:26:39 | taint3 | This CDS utils usage relies on user-provided value and can result in unrestricted file operations. |
+| pathinjection.js:28:34:28:39 | taint4 | pathinjection.js:10:36:10:45 | req.params | pathinjection.js:28:34:28:39 | taint4 | This CDS utils usage relies on user-provided value and can result in unrestricted file operations. |
+| pathinjection.js:30:40:30:45 | taint5 | pathinjection.js:10:36:10:45 | req.params | pathinjection.js:30:40:30:45 | taint5 | This CDS utils usage relies on user-provided value and can result in unrestricted file operations. |
+| pathinjection.js:31:26:31:34 | userinput | pathinjection.js:8:31:8:38 | req.data | pathinjection.js:31:26:31:34 | userinput | This CDS utils usage relies on user-provided value and can result in unrestricted file operations. |
+| pathinjection.js:33:38:33:46 | userinput | pathinjection.js:8:31:8:38 | req.data | pathinjection.js:33:38:33:46 | userinput | This CDS utils usage relies on user-provided value and can result in unrestricted file operations. |
+| pathinjection.js:34:24:34:32 | userinput | pathinjection.js:8:31:8:38 | req.data | pathinjection.js:34:24:34:32 | userinput | This CDS utils usage relies on user-provided value and can result in unrestricted file read. |
+| pathinjection.js:36:44:36:52 | userinput | pathinjection.js:8:31:8:38 | req.data | pathinjection.js:36:44:36:52 | userinput | This CDS utils usage relies on user-provided value and can result in unrestricted file operations. |
+| pathinjection.js:37:25:37:36 | userinputtwo | pathinjection.js:9:34:9:44 | req.headers | pathinjection.js:37:25:37:36 | userinputtwo | This CDS utils usage relies on user-provided value and can result in tainted data being written to a file. |
+| pathinjection.js:38:25:38:33 | userinput | pathinjection.js:8:31:8:38 | req.data | pathinjection.js:38:25:38:33 | userinput | This CDS utils usage relies on user-provided value and can result in unrestricted file operations. |
+| pathinjection.js:40:26:40:34 | userinput | pathinjection.js:8:31:8:38 | req.data | pathinjection.js:40:26:40:34 | userinput | This CDS utils usage relies on user-provided value and can result in unrestricted file operations. |
+| pathinjection.js:41:26:41:34 | userinput | pathinjection.js:8:31:8:38 | req.data | pathinjection.js:41:26:41:34 | userinput | This CDS utils usage relies on user-provided value and can result in unrestricted file operations. |
+| pathinjection.js:43:25:43:33 | userinput | pathinjection.js:8:31:8:38 | req.data | pathinjection.js:43:25:43:33 | userinput | This CDS utils usage relies on user-provided value and can result in unrestricted file operations. |
+| pathinjection.js:44:25:44:33 | userinput | pathinjection.js:8:31:8:38 | req.data | pathinjection.js:44:25:44:33 | userinput | This CDS utils usage relies on user-provided value and can result in unrestricted file operations. |
+| pathinjection.js:46:26:46:34 | userinput | pathinjection.js:8:31:8:38 | req.data | pathinjection.js:46:26:46:34 | userinput | This CDS utils usage relies on user-provided value and can result in unrestricted file operations. |
+| pathinjection.js:47:26:47:34 | userinput | pathinjection.js:8:31:8:38 | req.data | pathinjection.js:47:26:47:34 | userinput | This CDS utils usage relies on user-provided value and can result in unrestricted file operations. |
+| pathinjection.js:49:22:49:30 | userinput | pathinjection.js:8:31:8:38 | req.data | pathinjection.js:49:22:49:30 | userinput | This CDS utils usage relies on user-provided value and can result in unrestricted file operations. |
+| pathinjection.js:50:22:50:30 | userinput | pathinjection.js:8:31:8:38 | req.data | pathinjection.js:50:22:50:30 | userinput | This CDS utils usage relies on user-provided value and can result in unrestricted file operations. |

javascript/frameworks/cap/test/queries/path-traversal/pathinjection.js

@@ -0,0 +1,55 @@
+const cds = require("@sap/cds");
+const { decodeURI, decodeURIComponent, local, isdir, isfile, read, readdir, append, write, copy, stat, find, mkdirp, rmdir, rimraf, rm } = cds.utils
+
+module.exports = class Service1 extends cds.ApplicationService {
+
+    init() {
+        this.on("send1", async (req) => {
+            const userinput = req.data
+            const userinputtwo = req.headers
+            const userinputthree = req.params
+
+            const taint1 = decodeURI(userinputthree) // taint step
+
+            const taint2 = decodeURIComponent(userinputthree) // taint step
+
+            const taint3 = local(userinputthree) // taint step
+
+            const taint4 = isdir(userinputthree) // taint step
+
+            const taint5 = isfile(userinputthree) // taint step
+
+            const pkg = await read(taint1) // sink
+
+            const pdir = await readdir(taint2) // sink
+
+            const s = await stat(taint3) // sink
+
+            const f = await find(taint4) // sink
+
+            await append('db/data').to(taint5)  // sink
+            await append(userinput, 'dist/db/data')  // sink
+
+            await copy('db/data').to(userinput)  // sink
+            await copy(userinput, 'dist/db/data')  // sink
+
+            await write({ foo: 'bar' }).to(userinput) // sink
+            await write(userinputtwo).to('db/data') // sink
+            await write(userinput, { foo: 'bar' }) // sink
+
+            await mkdirp(userinput, 'db', 'data') // sink
+            await mkdirp(userinput) // sink
+
+            await rmdir(userinput, 'db', 'data') // sink
+            await rmdir(userinput) // sink
+
+            await rimraf(userinput, 'db', 'data') // sink
+            await rimraf(userinput) // sink
+
+            await rm(userinput, 'db', 'data') // sink
+            await rm(userinput) // sink
+        });
+
+        super.init();
+    }
+};

javascript/frameworks/cap/test/queries/path-traversal/pathinjection.qlref

@@ -0,0 +1 @@
+path-traversal/PathInjection.ql