Create query for vulnerability not specific to webapp security

javascript/frameworks/ui5/ext/ui5.model.yml

@@ -57,6 +57,13 @@ extensions:
       - ["UI5Control", "sap/ui/richtexteditor/RichTextEditor", ""]
       - ["UI5CodeEditor", "UI5CodeEditor", "Instance"]
       - ["UI5CodeEditor", "sap/ui/codeeditor/CodeEditor", ""]
+      # Classes that provide Path injection APIs
+      - ["UI5ClientStorage", "sap/ui/util/Storage", ""]
+      - ["UI5ClientStorage", "global", "Member[sap].Member[ui].Member[util].Member[Storage]"]
+      - ["UI5ClientStorage", "global", "Member[jQuery].Member[sap].Member[storage]"]
+      - ["UI5ClientStorage", "sap/ui/core/util/File", ""]
+      - ["UI5ClientStorage", "global", "Member[sap].Member[ui].Member[core].Member[util].Member[File]"]
+
 
   - addsTo:
       pack: codeql/javascript-all
@@ -94,6 +101,8 @@ extensions:
       - ["ResourceBundle", "Member[create].Argument[0]", "ui5-path-injection"]
       - ["Properties", "Member[create].Argument[0]", "ui5-path-injection"]
       - ["LoaderExtensions", "Member[registerResourcePath].Argument[1]", "ui5-path-injection"]
+      - ["UI5ClientStorage", "Member[put].Argument[0]", "ui5-path-injection"]
+      - ["UI5ClientStorage", "Member[save].Argument[1]", "ui5-path-injection"]
 
   - addsTo:
       pack: codeql/javascript-all

javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5FormulaInjectionQuery.qll

@@ -0,0 +1,132 @@
+import javascript
+import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow
+
+/**
+ * Call to [`sap.ui.util.Storage.put`](https://sapui5.hana.ondemand.com/sdk/#/api/module:sap/ui/util/Storage%23methods/put)
+ *  or its jQuery counterpart, [`jQuery.sap.Storage.put`](https://sapui5.hana.ondemand.com/sdk/#/api/jQuery.sap.storage%23methods/jQuery.sap.storage.put).
+ */
+private class StoragePutCall extends CallNode {
+  StoragePutCall() {
+    /* 1. This is a call to `sap.ui.util.Storage.put` */
+    // 1-1. Required from `sap/ui/util/Storage`
+    exists(RequiredObject storageClass |
+      this.getReceiver().getALocalSource() = storageClass and
+      this.getCalleeName() = "put"
+    )
+    or
+    // 1-2. Direct call to `sap.ui.util.Storage.put`
+    this =
+      globalVarRef("sap")
+          .getAPropertyRead("ui")
+          .getAPropertyRead("util")
+          .getAPropertyRead("Storage")
+          .getAMemberCall("put")
+    or
+    /* 2. This is a call to `jQuery.sap.storage.put` */
+    this =
+      globalVarRef("jQuery")
+          .getAPropertyRead("sap")
+          .getAPropertyRead("storage")
+          .getAMemberCall("put")
+  }
+
+  string getKeyName() {
+    result = this.getArgument(0).getALocalSource().asExpr().(StringLiteral).getValue()
+  }
+
+  string getContentToBeSaved() {
+    result = this.getArgument(1).getALocalSource().asExpr().(StringLiteral).getValue()
+  }
+}
+
+/**
+ * Call to [`sap.ui.core.util.File.save`](https://sapui5.hana.ondemand.com/sdk/#/api/sap.ui.core.util.File%23methods/sap.ui.core.util.File.save).
+ */
+private class FileSaveCall extends CallNode {
+  FileSaveCall() {
+    /* 1. Required from `sap/ui/core/util/File` */
+    exists(RequiredObject fileClass |
+      this.getReceiver().getALocalSource() = fileClass and
+      this.getCalleeName() = "save"
+    )
+    or
+    /* 2. Direct call to `sap.ui.core.util.File.save` */
+    this =
+      globalVarRef("sap")
+          .getAPropertyRead("ui")
+          .getAPropertyRead("core")
+          .getAPropertyRead("util")
+          .getAPropertyRead("File")
+          .getAMemberCall("save")
+  }
+
+  /**
+   * Gets the MIME type the file will saved under.
+   */
+  string getMimeType() {
+    result = this.getArgument(3).getALocalSource().asExpr().(StringLiteral).getValue()
+  }
+
+  /**
+   * Gets the file extension to be attached to the filename.
+   */
+  string getExtension() {
+    result = this.getArgument(2).getALocalSource().asExpr().(StringLiteral).getValue()
+  }
+
+  /**
+   * Holds if the file MIME type is `"text/csv"`.
+   */
+  predicate mimeTypeIsCsv() { this.getMimeType() = "text/csv" }
+
+  /**
+   * Holds if the file MIME type is `"application/json"`.
+   */
+  predicate mimeTypeIsJson() { this.getMimeType() = "application/json" }
+
+  /**
+   * Holds if the file extension is `"csv"`. It can be used as a fallback
+   * to detect a CSV data being written if `this.mimeTypeIsCsv()` fails.
+   */
+  predicate extensionIsCsv() { this.getExtension() = "csv" }
+
+  /**
+   * Holds if the file extension is `"json"`. It can be used as a fallback
+   * to detect a JSON data being written if `this.mimeTypeIsJson()` fails.
+   */
+  predicate extensionIsJson() { this.getExtension() = "json" }
+
+  /**
+   * Gets the content object to be saved into the file.
+   */
+  DataFlow::Node getContentToBeSaved() { result = this.getArgument(0) }
+
+  /**
+   * Gets the path the file will be saved under.
+   */
+  string getPathToBeSavedUnder() {
+    result = this.getArgument(1).getALocalSource().asExpr().(StringLiteral).getValue()
+  }
+}
+
+class UI5FormulaInjectionConfiguration extends TaintTracking::Configuration {
+  UI5FormulaInjectionConfiguration() { this = "UI5 Formula Injection" }
+
+  override predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node node) {
+    exists(StoragePutCall storagePutCall | node = storagePutCall.getArgument(1))
+    or
+    exists(FileSaveCall fileSaveCall |
+      node = fileSaveCall.getArgument(0) and
+      (
+        /* 1. Primary check: match on the MIME type */
+        fileSaveCall.mimeTypeIsCsv() or
+        fileSaveCall.mimeTypeIsJson() or
+        /* 2. Fallback check: match on the file extension */
+        fileSaveCall.extensionIsCsv() or
+        fileSaveCall.extensionIsJson()
+      )
+    )
+  }
+}

javascript/frameworks/ui5/src/UI5FormulaInjection/UI5FormulaInjection.md

@@ -0,0 +1,39 @@
+# Formula injection
+
+UI5 applications that save local data, fetched from an uncontrolled remote source, into a CSV file format using generic APIs such as [`sap.ui.core.util.File.save`](https://sapui5.hana.ondemand.com/sdk/#/api/sap.ui.core.util.File%23methods/sap.ui.core.util.File.save) are vulnerable to formula injection, or CSV injection.
+
+## Recommendation
+
+### Escape the leading special characters
+
+CSV cells containing a leading special characters such as the equal sign (`=`) may be interpreted as spreadsheet formulas. Therefore, these prefixes should be escaped with surrounding single quotes in order to have them interpreted as literal strings.
+
+### Use a dedicated API function
+
+As in other injection attacks, it is not recommended to use string concatenation to assemble data passed into a generic file-saving function, even if sanitizers are to be used. Instead, a dedicated library function should be used. For example, if the target being exported is a [sap.m.Table](https://sapui5.hana.ondemand.com/sdk/#/api/sap.m.Table) and the resulting file is intended to be opened using a spreadsheet program, then using one of the API functions provided by [`sap.ui.export.Spreadsheet`](https://sapui5.hana.ondemand.com/#/entity/sap.ui.export.Spreadsheet) is the preferred method of achieving the same exporting functionality.
+
+## Example
+
+The following controller is exporting a CSV file obtained from an event parameter by surrounding it in a pair of semicolons (`;`) as CSV separators.
+
+``` javascript
+sap.ui.define([
+    "sap/ui/core/Controller",
+    "sap/ui/core/util/File"
+  ], function(Controller, File) {
+    return Controller.extend("vulnerable.controller.app", {
+      onSomeEvent: function(oEvent) {
+         let response = oEvent.getProperty("someProperty").someField;
+         let csvRow = ";" + response + ";";
+         File.save(csvRow, "someFile", "csv", "text/csv", "utf-8");
+      }
+    });
+  });
+```
+
+## References
+
+- OWASP: [CSV Injection](https://owasp.org/www-community/attacks/CSV_Injection)
+- [CWE-1236](https://cwe.mitre.org/data/definitions/1236.html): Improper Neutralization of Formula Elements in a CSV File
+- API Documentation: [`sap.ui.export.Spreadsheet`](https://sapui5.hana.ondemand.com/#/entity/sap.ui.export.Spreadsheet)
+- API Documentation: [`sap.ui.core.util.File.save`](https://sapui5.hana.ondemand.com/sdk/#/api/sap.ui.core.util.File%23methods/sap.ui.core.util.File.save)

javascript/frameworks/ui5/src/UI5FormulaInjection/UI5FormulaInjection.ql

@@ -0,0 +1,26 @@
+/**
+ * @name UI5 Formula Injection
+ * @description Saving data from an uncontrolled remote source using filesystem or local storage
+ *              leads to disclosure of sensitive information or forgery of entry.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 7.8
+ * @precision medium
+ * @id js/ui5-formula-injection
+ * @tags security
+ *       external/cwe/cwe-1236
+ */
+
+import javascript
+import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow
+import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow::UI5PathGraph
+import advanced_security.javascript.frameworks.ui5.UI5FormulaInjectionQuery
+
+from
+  UI5FormulaInjectionConfiguration config, UI5PathNode source, UI5PathNode sink,
+  UI5PathNode primarySource
+where
+  config.hasFlowPath(source.getPathNode(), sink.getPathNode()) and
+  primarySource = source.getAPrimarySource()
+select sink, primarySource, sink, "The content of a saved file depends on a $@.", primarySource,
+  "user-provided value"

javascript/frameworks/ui5/src/UI5PathInjection/UI5PathInjection.md

@@ -0,0 +1,67 @@
+# Client-side path injection
+
+UI5 applications that access files using a dynamically configured path are vulnerable to injection attacks that allow an attacker to manipulate the file location.
+
+## Recommendation
+
+### Make path argument independent of the user input
+
+If possible, do not parameterize the path on a user input. Either hardcode the path string in the source, or use only strings that are created within the application.
+
+### Keep an allow-list of safe paths
+
+Keep a strict allow-list of safe paths to load from or send requests to. Before loading a script from a location outside the application or making an API request to a location, check if the path is contained in the list of safe paths. Also, make sure that the allow-list is kept up to date.
+
+### Check the script into the repository or use package managers
+
+Since the URL of the script may be pointing to a web server vulnerable to being hijacked, it may be a good idea to check a stable version of the script into the repository to increase the degree of control. If not possible, use a trusted package manager such as `npm`.
+
+## Example
+
+### Including scripts from an untrusted domain
+
+``` javascript
+sap.ui.require([
+    "sap/ui/dom/includeScript"
+  ],
+  function(includeScript) {
+    includeScript("http://some.vulnerable.domain/some-script.js");
+  }
+);
+```
+
+If the vulnerable domain is outside the organization and controlled by an untrusted third party, this may result in arbitrary code execution in the user's browser.
+
+### Using user input as a name of a file to be saved
+
+Suppose a controller is configured to receive a response from a server as follows.
+
+``` javascript
+sap.ui.define([
+   "sap/ui/core/mvc/Controller",
+   "sap/ui/core/util/File"
+  ],
+  function(Controller, File) {
+    return Controller.extend("vulnerable.controller.app", {
+      onInit: function() {
+        let oDataV2Model = this.getOwnerComponent().getModel("some-ODatav2-model");
+        this.getView().setModel(oDataV2Model);
+      },
+
+      onSomeEvent: function() {
+        let remoteResponse = this.getView().getModel().getProperty("someProperty");
+        File.save("some-content", remoteResponse, "txt", "text/plain", "utf-8");
+      }
+    });
+  });
+```
+
+Even if the server which updates the OData V2 model is in a trusted domain such as within the organization, the server may still contain tainted information if the UI5 application in question is vulnerable to other security attacks, say XSS. This may allow an attacker to save a file in the victim's local filesystem.
+
+## References
+
+- [CWE-829](https://cwe.mitre.org/data/definitions/829.html): Inclusion of Functionality from Untrusted Control Sphere
+- [CWE-073](https://cwe.mitre.org/data/definitions/73.html): External Control of File Name or Path
+- [API Documentation of `sap.ui.core.util.File`](https://sapui5.hana.ondemand.com/sdk/#/api/sap.ui.core.util.File%23methods/sap.ui.core.util.File.save)
+- [API Documentation of `sap.ui.dom.includeScript`](https://sapui5.hana.ondemand.com/sdk/#/api/module:sap/ui/dom/includeScript) and [`sap.ui.dom.includeStyleSheet`](https://sapui5.hana.ondemand.com/sdk/#/api/module:sap/ui/dom/includeStylesheet)
+- [API Documentation of `jQuery.sap.includeScript`](https://sapui5.hana.ondemand.com/sdk/#/api/module:sap/ui/dom/includeScript) and [`jQuery.sap.includeStyleSheet`](https://sapui5.hana.ondemand.com/sdk/#/api/module:sap/ui/dom/includeScript)

javascript/frameworks/ui5/src/UI5PathInjection/UI5PathInjection.ql

@@ -0,0 +1,37 @@
+/**
+ * @name UI5 Path Injection
+ * @description Constructing path from an uncontrolled remote source to be passed
+ *              to a filesystem API allows for manipulation of the local filesystem.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 7.8
+ * @precision medium
+ * @id js/ui5-path-injection
+ * @tags security
+ *       external/cwe/cwe-022
+ *       external/cwe/cwe-035
+ */
+
+import javascript
+import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow
+import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow::UI5PathGraph
+
+// import semmle.javascript.security.dataflow.TaintedPathQuery as TaintedPathQuery
+class UI5PathInjectionConfiguration extends TaintTracking::Configuration {
+  UI5PathInjectionConfiguration() { this = "UI5 Path Injection" }
+
+  override predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node node) {
+    node = ModelOutput::getASinkNode("ui5-path-injection").asSink()
+  }
+}
+
+from
+  UI5PathInjectionConfiguration config, UI5PathNode source, UI5PathNode sink,
+  UI5PathNode primarySource
+where
+  config.hasFlowPath(source.getPathNode(), sink.getPathNode()) and
+  primarySource = source.getAPrimarySource()
+select sink, primarySource, sink, "The path of a saved file depends on a $@.", primarySource,
+  "user-provided value"

javascript/frameworks/ui5/test/models/sink/formulaSinkTest.expected

@@ -0,0 +1,7 @@
+| sink.js:118:36:118:40 | code1 | code1 |
+| sink.js:119:48:119:52 | code1 | code1 |
+| sink.js:120:47:120:51 | code1 | code1 |
+| sink.js:122:27:122:31 | code0 | code0 |
+| sink.js:123:27:123:31 | code0 | code0 |
+| sink.js:125:44:125:48 | code0 | code0 |
+| sink.js:126:44:126:48 | code0 | code0 |

javascript/frameworks/ui5/test/models/sink/formulaSinkTest.ql

@@ -0,0 +1,14 @@
+/**
+ * @id formula-injection-sinks
+ * @name Formula injection sinks
+ * @kind problem
+ * @problem.severity error
+ */
+
+import javascript
+import advanced_security.javascript.frameworks.ui5.UI5FormulaInjectionQuery
+import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow as UI5DataFlow
+
+from UI5FormulaInjectionConfiguration config, DataFlow::Node sink
+where config.isSink(sink)
+select sink, sink.toString()