Fix cross-app isolation for UI5 default OData model

This commit:

- fixes cross-app isolation for the edge case where multiple
  UI5 apps are under the same "source-root" directory -- and
  are therefore analyzed via the same CodeQL database -- while
  also using the default OData model via bindElement.

- adds expected js/ui5-xss results for the new xss-cross-app-isolation
unit test:
  - webapp_a/fragments/DataDisplay.fragment.xml (line 7)
  - webapp_b/fragments/DataDisplay.fragment.xml (line 7)

These results validate that the improved DefaultODataServiceModel class
correctly isolates XSS detection within webapp boundaries when using
default OData models with bindElement.

Author: data-douser

.github/workflows/javascript.sarif.expected

@@ -113,12 +113,15 @@ abstract class UI5ExternalModel extends UI5Model, RemoteFlowSource {
 /** Default model which gains content from an SAP OData service (ie no model name is explicitly specified). */
 class DefaultODataServiceModel extends UI5ExternalModel {
   DefaultODataServiceModel() {
-    exists(ExternalModelManifest model |
+    exists(ExternalModelManifest model, WebApp webapp |
       //an OData default model exists
       model.getName() = "" and
       model.getDataSource() instanceof ODataDataSourceManifest and
       //therefore the bindElement calls that exist may be sources and also approximates the model itself
-      this.getCalleeName() = "bindElement"
+      this.getCalleeName() = "bindElement" and
+      // The bindElement call must be in the same webapp as the manifest that declares the default model
+      webapp.getAResource() = this.getFile() and
+      webapp.getManifest() = model.getJsonFile()
     )
   }
 

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

@@ -944,16 +944,23 @@ module ManifestJson {
       exists(JsonObject models |
         this = models.getPropValue(modelName) and
         dataSourceName = this.getPropStringValue("dataSource") and
-        /* This data source can be found in the "dataSources" property */
-        exists(DataSourceManifest dataSource | dataSource.getName() = dataSourceName)
+        /* This data source can be found in the "dataSources" property of the same manifest */
+        exists(DataSourceManifest dataSource |
+          dataSource.getName() = dataSourceName and
+          dataSource.getManifestJson() = this.getJsonFile()
+        )
       )
     }
 
     string getName() { result = modelName }
 
     string getDataSourceName() { result = dataSourceName }
 
-    DataSourceManifest getDataSource() { result.getName() = dataSourceName }
+    /** Gets the data source for this external model from the same manifest file. */
+    DataSourceManifest getDataSource() {
+      result.getName() = dataSourceName and
+      result.getManifestJson() = this.getJsonFile()
+    }
   }
 
   class ManifestJson extends File {

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

@@ -150,7 +150,9 @@ abstract class UI5BindingPath extends BindingPath {
           load.getNameArgument()
               .getStringValue()
               .matches("%" +
-                  this.getLocation().getFile().getBaseName().replaceAll(".fragment.xml", "") + "%")
+                  this.getLocation().getFile().getBaseName().replaceAll(".fragment.xml", "") + "%") and
+          // The fragment load call must be in the same webapp as the fragment file
+          inSameWebApp(this.getLocation().getFile(), load.getFile())
         )
       )
     )

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

@@ -0,0 +1,13 @@
+nodes
+| webapp_a/controller/App.controller.js:21:17:21:54 | oFragme ... sA(1)") |
+| webapp_a/fragments/DataDisplay.fragment.xml:7:9:7:43 | content={messageA} |
+| webapp_b/controller/App.controller.js:21:17:21:54 | oFragme ... sB(1)") |
+| webapp_b/fragments/DataDisplay.fragment.xml:7:9:7:43 | content={messageB} |
+edges
+| webapp_a/controller/App.controller.js:21:17:21:54 | oFragme ... sA(1)") | webapp_a/fragments/DataDisplay.fragment.xml:7:9:7:43 | content={messageA} |
+| webapp_a/fragments/DataDisplay.fragment.xml:7:9:7:43 | content={messageA} | webapp_a/controller/App.controller.js:21:17:21:54 | oFragme ... sA(1)") |
+| webapp_b/controller/App.controller.js:21:17:21:54 | oFragme ... sB(1)") | webapp_b/fragments/DataDisplay.fragment.xml:7:9:7:43 | content={messageB} |
+| webapp_b/fragments/DataDisplay.fragment.xml:7:9:7:43 | content={messageB} | webapp_b/controller/App.controller.js:21:17:21:54 | oFragme ... sB(1)") |
+#select
+| webapp_a/fragments/DataDisplay.fragment.xml:7:9:7:43 | content={messageA} | webapp_a/controller/App.controller.js:21:17:21:54 | oFragme ... sA(1)") | webapp_a/fragments/DataDisplay.fragment.xml:7:9:7:43 | content={messageA} | XSS vulnerability due to $@. | webapp_a/controller/App.controller.js:21:17:21:54 | oFragme ... sA(1)") | user-provided value |
+| webapp_b/fragments/DataDisplay.fragment.xml:7:9:7:43 | content={messageB} | webapp_b/controller/App.controller.js:21:17:21:54 | oFragme ... sB(1)") | webapp_b/fragments/DataDisplay.fragment.xml:7:9:7:43 | content={messageB} | XSS vulnerability due to $@. | webapp_b/controller/App.controller.js:21:17:21:54 | oFragme ... sB(1)") | user-provided value |

javascript/frameworks/ui5/test/queries/UI5Xss/xss-cross-app-isolation/UI5Xss.expected

@@ -0,0 +1 @@
+UI5Xss/UI5Xss.ql

javascript/frameworks/ui5/test/queries/UI5Xss/xss-cross-app-isolation/UI5Xss.qlref

@@ -0,0 +1,5 @@
+{
+  "name": "xss-cross-app-isolation",
+  "version": "1.0.0",
+  "description": "Test case for ensuring XSS detection does not cross webapp boundaries with default OData models"
+}

javascript/frameworks/ui5/test/queries/UI5Xss/xss-cross-app-isolation/package.json

@@ -0,0 +1,7 @@
+specVersion: "3.1"
+type: application
+metadata:
+  name: xss-cross-app-isolation
+framework:
+  name: SAPUI5
+  version: "1.120.0"

javascript/frameworks/ui5/test/queries/UI5Xss/xss-cross-app-isolation/ui5.yaml

@@ -0,0 +1,13 @@
+sap.ui.define([
+    "sap/ui/core/UIComponent"
+], function (UIComponent) {
+    "use strict";
+    return UIComponent.extend("app.a.Component", {
+        metadata: {
+            manifest: "json"
+        },
+        init: function () {
+            UIComponent.prototype.init.apply(this, arguments);
+        }
+    });
+});

javascript/frameworks/ui5/test/queries/UI5Xss/xss-cross-app-isolation/webapp_a/Component.js

@@ -0,0 +1,28 @@
+sap.ui.define([
+    "sap/ui/core/mvc/Controller",
+    "sap/ui/core/Fragment"
+], function (Controller, Fragment) {
+    "use strict";
+
+    return Controller.extend("app.a.controller.App", {
+        onInit: function () {
+            // XSS vulnerability pattern in App A:
+            // 1. OData model is configured as default model in manifest.json
+            // 2. Fragment is loaded dynamically
+            // 3. Fragment is bound to OData entity via bindElement
+            // 4. Fragment contains HTML control that renders OData content
+
+            Fragment.load({
+                id: this.getView().getId(),
+                name: "app.a.fragments.DataDisplay",
+                controller: this
+            }).then(function (oFragment) {
+                // Bind fragment to an OData entity - vulnerability is in the backend data
+                oFragment.bindElement("/MessagesA(1)");
+
+                // Add the fragment to the page content
+                this.byId("page").addContent(oFragment);
+            }.bind(this));
+        }
+    });
+});