Improve UI5Xss.ql query to detect default OData model with bindElement

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

@@ -748,4 +748,9 @@ class Binding extends TBinding {
   BindingPath getBindingPath() { result.getBinding() = this }
 
   BindingTarget getBindingTarget() { result.getBinding() = this }
+
+  /**
+   * Gets the `BindElementMethodCallNode` for this binding, if it is a context binding via `bindElement`.
+   */
+  BindElementMethodCallNode getBindElementCall() { this = TLateJavaScriptContextBinding(result, _) }
 }

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

@@ -113,20 +113,28 @@ 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()
     )
   }
 
   override string getSourceType() { result = "DefaultODataServiceModel" }
 
   override string getName() { result = "" }
 
-  Binding asBinding() { result.getBindingTarget().asDataFlowNode() = this }
+  /**
+   * Gets bindings associated with this default OData model source.
+   * Since `DefaultODataServiceModel` represents a `bindElement` call,
+   * we match context bindings whose `bindElement` call is this node.
+   */
+  Binding asBinding() { result.getBindElementCall() = this }
 }
 
 /** Model which gains content from an SAP OData service. */

javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5.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/UI5View.qll

@@ -133,10 +133,13 @@ abstract class UI5BindingPath extends BindingPath {
         not exists(this.getModelName())
       )
       or
-      /* 5.  There is no call to `setModel` at all and a default model exists that is related to the binding path this refers to */
+      /* 5.  There is no call to `setModel` in the same webapp and a default model exists that is related to the binding path this refers to */
       exists(DefaultODataServiceModel defaultModel |
         result = defaultModel and
-        not exists(MethodCallNode viewSetModelCall | viewSetModelCall.getMethodName() = "setModel") and
+        not exists(MethodCallNode viewSetModelCall |
+          viewSetModelCall.getMethodName() = "setModel" and
+          inSameWebApp(this.getLocation().getFile(), viewSetModelCall.getFile())
+        ) and
         /*
          * this binding path can occur in a fragment that is the receiver object for the bindElement model approximation
          * i.e. checks that the default model is relevant
@@ -147,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/test/queries/UI5Xss/xss-cross-app-isolation/UI5Xss.expected

@@ -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.qlref

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

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

@@ -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/ui5.yaml

@@ -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/webapp_a/Component.js

@@ -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/controller/App.controller.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));
+        }
+    });
+});

javascript/frameworks/ui5/test/queries/UI5Xss/xss-cross-app-isolation/webapp_a/fragments/DataDisplay.fragment.xml

@@ -0,0 +1,9 @@
+<core:FragmentDefinition
+    xmlns="sap.m"
+    xmlns:core="sap.ui.core">
+    <VBox class="sapUiSmallMargin">
+        <Label text="Message from OData in App A:" />
+        <!-- XSS vulnerability: HTML content bound to OData property containing unsanitized data -->
+        <core:HTML content="{messageA}" />
+    </VBox>
+</core:FragmentDefinition>

javascript/frameworks/ui5/test/queries/UI5Xss/xss-cross-app-isolation/webapp_a/index.html

@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <meta charset="utf-8">
+    <title>App A - XSS Cross-App Isolation Test</title>
+    <script
+        id="sap-ui-bootstrap"
+        src="https://openui5.hana.ondemand.com/resources/sap-ui-core.js"
+        data-sap-ui-theme="sap_fiori_3"
+        data-sap-ui-resourceroots='{
+            "app.a": "./"
+        }'
+        data-sap-ui-oninit="module:app/a/index"
+        data-sap-ui-async="true">
+    </script>
+</head>
+<body class="sapUiBody" id="content">
+</body>
+</html>

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

@@ -0,0 +1,12 @@
+sap.ui.define([
+    "sap/ui/core/ComponentContainer"
+], function (ComponentContainer) {
+    "use strict";
+    new ComponentContainer({
+        name: "app.a",
+        settings: {
+            id: "app.a"
+        },
+        async: true
+    }).placeAt("content");
+});

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

@@ -0,0 +1,48 @@
+{
+  "_version": "1.58.0",
+  "sap.app": {
+    "id": "app.a",
+    "type": "application",
+    "title": "App A - XSS Cross-App Isolation Test",
+    "applicationVersion": {
+      "version": "1.0.0"
+    },
+    "dataSources": {
+      "mainService": {
+        "uri": "/odata/v4/catalog_a/",
+        "type": "OData",
+        "settings": {
+          "odataVersion": "4.0"
+        }
+      }
+    }
+  },
+  "sap.ui": {
+    "technology": "UI5",
+    "deviceTypes": {
+      "desktop": true,
+      "tablet": true,
+      "phone": true
+    }
+  },
+  "sap.ui5": {
+    "rootView": {
+      "viewName": "app.a.view.App",
+      "type": "XML",
+      "id": "app"
+    },
+    "dependencies": {
+      "minUI5Version": "1.60",
+      "libs": {
+        "sap.m": {}
+      }
+    },
+    "models": {
+      "": {
+        "dataSource": "mainService",
+        "preload": true,
+        "settings": {}
+      }
+    }
+  }
+}

javascript/frameworks/ui5/test/queries/UI5Xss/xss-cross-app-isolation/webapp_a/view/App.view.xml

@@ -0,0 +1,14 @@
+<mvc:View
+    controllerName="app.a.controller.App"
+    xmlns:mvc="sap.ui.core.mvc"
+    xmlns="sap.m"
+    xmlns:core="sap.ui.core"
+    displayBlock="true">
+    <App id="app">
+        <Page id="page" title="App A - XSS Cross-App Isolation Test">
+            <content>
+                <!-- Fragment will be loaded here dynamically -->
+            </content>
+        </Page>
+    </App>
+</mvc:View>

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

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

javascript/frameworks/ui5/test/queries/UI5Xss/xss-cross-app-isolation/webapp_b/controller/App.controller.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.b.controller.App", {
+        onInit: function () {
+            // XSS vulnerability pattern in App B:
+            // 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.b.fragments.DataDisplay",
+                controller: this
+            }).then(function (oFragment) {
+                // Bind fragment to an OData entity - vulnerability is in the backend data
+                oFragment.bindElement("/MessagesB(1)");
+
+                // Add the fragment to the page content
+                this.byId("page").addContent(oFragment);
+            }.bind(this));
+        }
+    });
+});

javascript/frameworks/ui5/test/queries/UI5Xss/xss-cross-app-isolation/webapp_b/fragments/DataDisplay.fragment.xml

@@ -0,0 +1,9 @@
+<core:FragmentDefinition
+    xmlns="sap.m"
+    xmlns:core="sap.ui.core">
+    <VBox class="sapUiSmallMargin">
+        <Label text="Message from OData in App B:" />
+        <!-- XSS vulnerability: HTML content bound to OData property containing unsanitized data -->
+        <core:HTML content="{messageB}" />
+    </VBox>
+</core:FragmentDefinition>

javascript/frameworks/ui5/test/queries/UI5Xss/xss-cross-app-isolation/webapp_b/index.html

@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <meta charset="utf-8">
+    <title>App B - XSS Cross-App Isolation Test</title>
+    <script
+        id="sap-ui-bootstrap"
+        src="https://openui5.hana.ondemand.com/resources/sap-ui-core.js"
+        data-sap-ui-theme="sap_fiori_3"
+        data-sap-ui-resourceroots='{
+            "app.b": "./"
+        }'
+        data-sap-ui-oninit="module:app/b/index"
+        data-sap-ui-async="true">
+    </script>
+</head>
+<body class="sapUiBody" id="content">
+</body>
+</html>

javascript/frameworks/ui5/test/queries/UI5Xss/xss-cross-app-isolation/webapp_b/index.js

@@ -0,0 +1,12 @@
+sap.ui.define([
+    "sap/ui/core/ComponentContainer"
+], function (ComponentContainer) {
+    "use strict";
+    new ComponentContainer({
+        name: "app.b",
+        settings: {
+            id: "app.b"
+        },
+        async: true
+    }).placeAt("content");
+});