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 saves a 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) is vulnerable to formula injections, or CSV injections.
+
+## Recommendation
+
+### Escape the leading special characters
+
+CSV cells containing a leading special characters such as equal sign (`=`) may be interpreted as spreadsheet formulas. Therefore, these prefixes should be escaped with surrounding single quotes in order to keep them as literal strings.
+
+### Use a dedicated API function
+
+As in any other injection attacks, it should not be encouraged to assemble the contents manually using string concatenation then pass it onto a generic file-saving function, even if sanitizers are to be used, since it is an error-prone process. 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 to intended to be opened using a spreadsheet program anyways, 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 dynamically configures the path of a file to be saved to or loaded from is vulnerable to injection attacks that allows an attacker to manipulate the location of the file.
+
+## Recommendation
+
+### Make path argument independent of the user input
+
+If possible, do not parameterize the path on a user input. Either make the path string to be hardcoded in the source, or dependent only on data 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 request 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, to increase the degree of control it may be a good idea to check a stable version of the script into the repository to be locally sourced from. If it is not an option, using a trusted package manager such as `npm` is also a good idea.
+
+## 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 a tainted information if the UI5 application in question is already vulnerable to some other vulnerabilities, say XSS. This may lead to the attacker being able to save a file in the victim's local filesystem with the given filename which might be used for further attacks.
+
+## 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()