Xml Fragments models

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

@@ -76,6 +76,10 @@ extensions:
       - ["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]"]
+      # Fragment API - byId returns a Control (models static Fragment.byId() calls)
+      - ["Fragment", "Fragment", "Instance"]
+      - ["Fragment", "sap/ui/core/Fragment", ""]
+      - ["UI5InputControl", "Fragment", "Member[byId].ReturnValue"]
       # Publishing and Subscribing to Events
       - ["UI5EventBusInstance", "sap/ui/core/EventBus", "Member[getInstance].ReturnValue"]
       - ["UI5EventBusPublish", "UI5EventBusInstance", "Member[publish]"]

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

@@ -0,0 +1,51 @@
+/**
+ * Provides classes for modeling the sap.ui.core.Fragment API.
+ */
+
+import javascript
+import DataFlow
+import advanced_security.javascript.frameworks.ui5.UI5
+
+/**
+ * Gets a reference to the Fragment module import.
+ */
+class FragmentLoad extends InvokeNode, MethodCallNode {
+  FragmentLoad() {
+    this =
+      TypeTrackers::hasDependency(["sap/ui/core/Fragment", "sap.ui.core.Fragment"])
+          .getAMemberCall("load")
+    or
+    exists(RequiredObject requiredModule, SapDefineModule sapModule |
+      this = requiredModule.asSourceNode().getAMemberCall("load") and
+      //ensure it is an sap module define
+      requiredModule.getEnclosingFunction() = sapModule.getArgument(1)
+    )
+  }
+
+  /**
+   * Gets the configuration object passed to Fragment.load().
+   */
+  DataFlow::Node getConfigObject() { result = this.getArgument(0) }
+
+  /**
+   * Gets the 'name' property value from the config object.
+   * This specifies the fragment resource to load (e.g., "my.app.fragments.MyFragment").
+   */
+  DataFlow::Node getNameArgument() {
+    exists(DataFlow::ObjectLiteralNode config |
+      config = this.getConfigObject().getALocalSource() and
+      result = config.getAPropertyWrite("name").getRhs()
+    )
+  }
+
+  /**
+   * Gets the 'controller' property value from the config object.
+   * This specifies the fragment's controller, if it has one.
+   */
+  DataFlow::Node getControllerArgument() {
+    exists(DataFlow::ObjectLiteralNode config |
+      config = this.getConfigObject().getALocalSource() and
+      result = config.getAPropertyWrite("controller").getRhs()
+    )
+  }
+}

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

@@ -12,8 +12,10 @@ private module WebAppResourceRootJsonReader implements JsonParser::MakeJsonReade
   class JsonReader extends WebApp {
     string getJson() {
       // We match on the lowercase to cover all the possible variants of writing the attribute name.
+      // Support both "data-sap-ui-resourceroots" and "data-sap-ui-resource-roots" (with hyphen)
       exists(string resourceRootAttributeName |
-        resourceRootAttributeName.toLowerCase() = "data-sap-ui-resourceroots"
+        resourceRootAttributeName.toLowerCase() =
+          ["data-sap-ui-resourceroots", "data-sap-ui-resource-roots"]
       |
         result = this.getCoreScript().getAttributeByName(resourceRootAttributeName).getValue()
       )
@@ -151,6 +153,17 @@ class SapUiCore extends MethodCallNode {
   SapUiCore() { this = globalVarRef("sap").getAPropertyRead("ui").getAMethodCall("getCore") }
 }
 
+/**
+ * A reference to the Fragment module (`sap/ui/core/Fragment`).
+ * Used for static methods like `Fragment.byId(viewId, controlId)`.
+ *
+ * Use of `DataFlow::moduleImport` may not cover byId references
+ * coming from sources with es6 style imports of Fragments.
+ */
+class FragmentModule extends DataFlow::SourceNode {
+  FragmentModule() { this = DataFlow::moduleImport("sap/ui/core/Fragment") }
+}
+
 /**
  * A user-defined module through `sap.ui.define` or `jQuery.sap.declare`.
  */
@@ -343,6 +356,7 @@ class ControlReference extends Reference {
   string controlId;
 
   ControlReference() {
+    // Standard byId patterns: this.byId("id"), this.getView().byId("id"), sap.ui.getCore().byId("id")
     this.getArgument(0).getALocalSource().getStringValue() = controlId and
     (
       exists(CustomController controller |
@@ -352,6 +366,11 @@ class ControlReference extends Reference {
       or
       exists(SapUiCore sapUiCore | this = sapUiCore.getAMemberCall("byId"))
     )
+    or
+    // Fragment.byId(viewId, controlId) - static method with 2 arguments
+    this.getNumArgument() = 2 and
+    this.getArgument(1).getALocalSource().getStringValue() = controlId and
+    exists(FragmentModule fragment | this = fragment.getAMemberCall("byId"))
   }
 
   CustomControl getDefinition() {

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

@@ -2,6 +2,7 @@ import advanced_security.javascript.frameworks.ui5.UI5
 import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow
 private import semmle.javascript.frameworks.data.internal.ApiGraphModelsExtensions as ApiGraphModelsExtensions
 import advanced_security.javascript.frameworks.ui5.Bindings
+import advanced_security.javascript.frameworks.ui5.Fragment
 
 /**
  * Gets the immediate supertype of a given type from the extensible predicate `typeModel` provided by
@@ -185,6 +186,17 @@ predicate isBuiltInControl(string qualifiedTypeUri) {
   )
 }
 
+/**
+ * A UI5 Fragment that might include XSS sources and sinks in standard controls.
+ */
+abstract class UI5Fragment extends File {
+  abstract UI5Control getControl();
+
+  abstract UI5BindingPath getASource();
+
+  abstract UI5BindingPath getAnHtmlISink();
+}
+
 /**
  * A UI5 View that might include XSS sources and sinks in standard controls.
  */
@@ -672,8 +684,106 @@ class XmlView extends UI5View instanceof XmlFile {
   }
 }
 
+/**
+ * An xml fragment. It may or may not have controllers associated.
+ */
+class XmlFragment extends UI5View instanceof XmlFile {
+  XmlRootElement root;
+
+  XmlFragment() {
+    root = this.getARootElement() and
+    (
+      root.getNamespace().getUri() = "sap.m"
+      or
+      root.getNamespace().getUri() = "sap.ui.core"
+    ) and
+    root.hasName("FragmentDefinition")
+  }
+
+  override XmlBindingPath getASource() {
+    exists(XmlElement control, string type, string path, string property |
+      type = result.getControlTypeName() and
+      this = control.getFile() and
+      ApiGraphModelsExtensions::sourceModel(getASuperType(type), path, "remote", _) and
+      property = path.replaceAll(" ", "").regexpCapture("Member\\[([^\\]]+)\\]", 1) and
+      result.getBindingTarget() = control.getAttribute(property)
+    )
+  }
+
+  override XmlBindingPath getAnHtmlISink() {
+    exists(XmlElement control, string type, string path, string property |
+      this = control.getFile() and
+      type = result.getControlTypeName() and
+      ApiGraphModelsExtensions::sinkModel(getASuperType(type), path, "ui5-html-injection", _) and
+      property = path.replaceAll(" ", "").regexpCapture("Member\\[([^\\]]+)\\]", 1) and
+      result.getBindingTarget() = control.getAttribute(property) and
+      /* If the control is an `sap.ui.core.HTML` then the control should be missing the `sanitizeContent` attribute */
+      (
+        getASuperType(type) = "HTMLControl"
+        implies
+        (
+          not exists(control.getAttribute("sanitizeContent")) or
+          control.getAttribute("sanitizeContent").getValue() = "false"
+        )
+      )
+    )
+  }
+
+  override UI5Control getControl() {
+    exists(XmlElement element |
+      result.asXmlControl() = element and
+      /* Use getAChild+ because some controls nest other controls inside them as aggregations */
+      element = root.getAChild+() and
+      (
+        /* 1. A builtin control provided by UI5 */
+        isBuiltInControl(element.getNamespace().getUri())
+        or
+        /* 2. A custom control with implementation code found in the webapp */
+        exists(CustomControl control |
+          control.getName() = element.getNamespace().getUri() + "." + element.getName() and
+          inSameWebApp(control.getFile(), element.getFile())
+        )
+      )
+    )
+  }
+
+  /**
+   * This is either known in the location from which `loadFragment` is called (in a controller's init function)
+   * OR in the optional controller param of `Fragment.load`.
+   * This MAY return no value, when the fragment is not associated to any controller.
+   * When this returns a value it is guaranteed that this xml fragment is instantiated.
+   */
+  override string getControllerName() {
+    exists(CustomController controller, MethodCallNode loadFragmentCall |
+      loadFragmentCall.getMethodName() = "loadFragment" and
+      controller.getAThisNode().flowsTo(loadFragmentCall.getReceiver()) and
+      controller.getName() = result
+    )
+    or
+    exists(CustomController controller, FragmentLoad fragmentLoad |
+      controller.getAThisNode().flowsTo(fragmentLoad.getControllerArgument()) and
+      /*
+       * extracting just the base name of the fragment (not the fully qualified)
+       * otherwise difficult to know which part of absolute path is only for the qualified name
+       */
+
+      fragmentLoad
+          .getNameArgument()
+          .getStringValue()
+          .matches("%" + this.getBaseName().replaceAll(".fragment.xml", "")) and
+      controller.getName() = result
+    )
+  }
+}
+
 private newtype TUI5Control =
-  TXmlControl(XmlElement control) or
+  TXmlControl(XmlElement control) {
+    control
+        .(Locatable)
+        .getFile()
+        .getBaseName()
+        .matches(["%.view.xml", "%.view.html", "%.fragment.xml"])
+  } or
   TJsonControl(JsonObject control) {
     exists(JsonView view | control.getParent() = view.getRoot().getPropValue("content"))
   } or
@@ -769,11 +879,23 @@ class UI5Control extends TUI5Control {
 
   /**
    * Gets a reference to this control. Currently supports only such references made through `byId`.
+   * Handles both:
+   * - `this.byId("controlId")` or `this.getView().byId("controlId")` - ID in argument 0
+   * - `Fragment.byId(viewId, "controlId")` - ID in argument 1
    */
   ControlReference getAReference() {
     result.getMethodName() = "byId" and
-    result.getArgument(0).getALocalSource().asExpr().(StringLiteral).getValue() =
-      this.getProperty("id").getValue()
+    (
+      // Standard byId: ID in first argument
+      result.getNumArgument() = 1 and
+      result.getArgument(0).getALocalSource().asExpr().(StringLiteral).getValue() =
+        this.getProperty("id").getValue()
+      or
+      // Fragment.byId: ID in second argument
+      result.getNumArgument() = 2 and
+      result.getArgument(1).getALocalSource().asExpr().(StringLiteral).getValue() =
+        this.getProperty("id").getValue()
+    )
   }
 
   /** Gets a property of this control having the name. */

javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/FlowSteps.qll

@@ -1,5 +1,36 @@
 import javascript
 import advanced_security.javascript.frameworks.ui5.UI5
+import advanced_security.javascript.frameworks.ui5.UI5View
+
+/**
+ * Step from a value assigned to a JSONModel property to the binding path that reads it.
+ * This enables tracking data flowing INTO a model constructor argument and OUT through XML bindings.
+ *
+ * e.g. Given:
+ * ```javascript
+ * var userInput = getUntrustedData();
+ * var oModel = new JSONModel({ payload: userInput });
+ * this.getView().setModel(oModel);
+ * ```
+ * and
+ * ```xml
+ * <core:HTML content="{/payload}" />
+ * ```
+ *
+ * Creates a flow step from `userInput` (the RHS of the property) to the binding path `{/payload}`.
+ */
+class JsonModelPropertyValueToBindingStep extends DataFlow::SharedFlowStep {
+  override predicate step(DataFlow::Node start, DataFlow::Node end) {
+    exists(UI5BindingPath bindingPath, DataFlow::PropWrite propWrite |
+      // The binding path resolves to a property write in a JSONModel
+      bindingPath.getNode() = propWrite and
+      // The start is the RHS value being assigned to that property
+      start = propWrite.getRhs() and
+      // The end is the binding path itself (as a node representing where data flows to)
+      end = propWrite
+    )
+  }
+}
 
 /**
  * Step from a part of internal model to a relevant control property.

javascript/frameworks/ui5/test/queries/UI5Xss/xss-fragment-static-byid/UI5Xss.expected

@@ -0,0 +1,12 @@
+nodes
+| webapp/controller/Main.controller.js:20:17:20:29 | sAttackerData |
+| webapp/controller/Main.controller.js:20:33:20:97 | Fragmen ... Value() |
+| webapp/controller/Main.controller.js:24:34:24:69 | "<span> ... /span>" |
+| webapp/controller/Main.controller.js:24:45:24:57 | sAttackerData |
+| webapp/view/Main.view.html:4:10:4:34 | data-content={/payload} |
+edges
+| webapp/controller/Main.controller.js:20:17:20:29 | sAttackerData | webapp/controller/Main.controller.js:24:45:24:57 | sAttackerData |
+| webapp/controller/Main.controller.js:20:33:20:97 | Fragmen ... Value() | webapp/controller/Main.controller.js:20:17:20:29 | sAttackerData |
+| webapp/controller/Main.controller.js:24:45:24:57 | sAttackerData | webapp/controller/Main.controller.js:24:34:24:69 | "<span> ... /span>" |
+#select
+| webapp/controller/Main.controller.js:24:34:24:69 | "<span> ... /span>" | webapp/controller/Main.controller.js:20:33:20:97 | Fragmen ... Value() | webapp/controller/Main.controller.js:24:34:24:69 | "<span> ... /span>" | XSS vulnerability due to $@. | webapp/controller/Main.controller.js:20:33:20:97 | Fragmen ... Value() | user-provided value |

javascript/frameworks/ui5/test/queries/UI5Xss/xss-fragment-static-byid/UI5Xss.qlref

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

javascript/frameworks/ui5/test/queries/UI5Xss/xss-fragment-static-byid/package.json

@@ -0,0 +1,5 @@
+{
+  "name": "ui5-xss-fragment-static-byid",
+  "version": "1.0.0",
+  "main": "index.js"
+}

javascript/frameworks/ui5/test/queries/UI5Xss/xss-fragment-static-byid/ui5.yaml

@@ -0,0 +1,7 @@
+specVersion: '3.0'
+metadata:
+  name: ui5-xss-fragment-static-byid
+type: application
+framework:
+  name: SAPUI5
+  version: "1.115.0"

javascript/frameworks/ui5/test/queries/UI5Xss/xss-fragment-static-byid/webapp/controller/Main.controller.js

@@ -0,0 +1,27 @@
+sap.ui.define([
+    "sap/ui/core/mvc/Controller",
+    "sap/ui/core/Fragment"
+], function (Controller, Fragment) {
+    "use strict";
+    return Controller.extend("ui5-xss-fragment-static-byid.controller.Main", {
+        onInit: function () {
+            // Load fragment with view ID prefix
+            Fragment.load({
+                id: this.getView().getId(),
+                name: "ui5-xss-fragment-static-byid.view.PayloadForm",
+                controller: this
+            }).then(function (oFragment) {
+                this.byId("fragmentArea").addContent(oFragment);
+            }.bind(this));
+        },
+
+        onSubmitPayload: function () {
+            // XSS vulnerability: Fragment.byId() static method to access fragment controls
+            var sAttackerData = Fragment.byId(this.getView().getId(), "attackerInput").getValue();
+            var oHtmlSink = Fragment.byId(this.getView().getId(), "vulnerableOutput");
+
+            // Sink: setContent with unsanitized user input
+            oHtmlSink.setContent("<span>" + sAttackerData + "</span>");
+        }
+    });
+});

javascript/frameworks/ui5/test/queries/UI5Xss/xss-fragment-static-byid/webapp/index.html

@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <meta charset="utf-8">
+    <title>UI5 XSS Fragment Static ById Test</title>
+    <script src="https://sdk.openui5.org/resources/sap-ui-core.js" 
+        data-sap-ui-libs="sap.m"
+        data-sap-ui-onInit="module:ui5-xss-fragment-static-byid/index" 
+        data-sap-ui-resourceroots='{
+            "ui5-xss-fragment-static-byid": "./"
+        }'>
+    </script>
+</head>
+<body class="sapUiBody" id="content">
+</body>
+</html>

javascript/frameworks/ui5/test/queries/UI5Xss/xss-fragment-static-byid/webapp/index.js

@@ -0,0 +1,10 @@
+sap.ui.define([
+    "sap/ui/core/mvc/HTMLView"
+], function (HTMLView) {
+    "use strict";
+    HTMLView.create({
+        viewName: "ui5-xss-fragment-static-byid.view.Main"
+    }).then(function (oView) {
+        oView.placeAt("content");
+    });
+});