Merge pull request #270 from advanced-security/knewbury01/ui5-fragments

Xml Fragments models

Author: data-douser

.github/workflows/javascript.sarif.expected

@@ -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/ext/ui5.model.yml

@@ -597,6 +597,8 @@ class BindingTarget extends TBindingTarget {
         this = TLateJavaScriptBindingTarget(target, _)
         or
         this = TEarlyJavaScriptPropertyBindingTarget(target, _)
+        or
+        this = TLateJavaScriptBindingTarget(target.(BindElementMethodCallNode).getReceiver(), _)
       ) and
       result = target
     )
@@ -727,6 +729,22 @@ class Binding extends TBinding {
     )
   }
 
+  DataFlow::Node asDataFlowNode() {
+    exists(DataFlow::Node target |
+      (
+        this = TEarlyJavaScriptPropertyBinding(_, target)
+        or
+        this = TLateJavaScriptPropertyBinding(_, target)
+        or
+        exists(BindElementMethodCallNode bindElementCall |
+          target = bindElementCall.getReceiver() and
+          this = TLateJavaScriptContextBinding(bindElementCall, _)
+        )
+      ) and
+      result = target
+    )
+  }
+
   BindingPath getBindingPath() { result.getBinding() = this }
 
   BindingTarget getBindingTarget() { result.getBinding() = this }

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

@@ -0,0 +1,56 @@
+/**
+ * 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()
+    )
+  }
+
+  DataFlow::ParameterNode getCallbackObjectReference() {
+    //the load invoke node is actually just a part of the chained load.then.bind
+    result = this.getAMemberCall(_).getABoundCallbackParameter(_, _)
+  }
+}

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

@@ -110,6 +110,25 @@ abstract class UI5ExternalModel extends UI5Model, RemoteFlowSource {
   abstract string getName();
 }
 
+/** 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 |
+      //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"
+    )
+  }
+
+  override string getSourceType() { result = "DefaultODataServiceModel" }
+
+  override string getName() { result = "" }
+
+  Binding asBinding() { result.getBindingTarget().asDataFlowNode() = this }
+}
+
 /** Model which gains content from an SAP OData service. */
 class ODataServiceModel extends UI5ExternalModel {
   string modelName;

javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/RemoteFlowSources.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/UI5.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
@@ -131,6 +132,24 @@ abstract class UI5BindingPath extends BindingPath {
         not exists(controlSetModelCall.getArgument(1)) and
         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 */
+      exists(DefaultODataServiceModel defaultModel |
+        result = defaultModel and
+        not exists(MethodCallNode viewSetModelCall | viewSetModelCall.getMethodName() = "setModel") 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
+         */
+
+        exists(FragmentLoad load |
+          load.getCallbackObjectReference().flowsTo(defaultModel.asBinding().asDataFlowNode()) and
+          load.getNameArgument()
+              .getStringValue()
+              .matches("%" +
+                  this.getLocation().getFile().getBaseName().replaceAll(".fragment.xml", "") + "%")
+        )
+      )
     )
     // and
     // /* This binding path and the resulting model should live inside the same webapp */
@@ -672,8 +691,97 @@ 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)
+    )
+  }
+
+  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 +877,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/UI5View.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/lib/advanced_security/javascript/frameworks/ui5/dataflow/FlowSteps.qll

@@ -1,3 +1,3 @@
-| sink1.xml:7:5:7:68 | content={path: '/input'} | The binding path `content={path: '/input'}` is an HTML injection sink. |
-| sink1.xml:8:5:8:44 | content={path: '/input'} | The binding path `content={path: '/input'}` is an HTML injection sink. |
-| sink1.xml:10:5:10:73 | value={path: '/input'} | The binding path `value={path: '/input'}` is an HTML injection sink. |
+| sink1.view.xml:7:5:7:68 | content={path: '/input'} | The binding path `content={path: '/input'}` is an HTML injection sink. |
+| sink1.view.xml:8:5:8:44 | content={path: '/input'} | The binding path `content={path: '/input'}` is an HTML injection sink. |
+| sink1.view.xml:10:5:10:73 | value={path: '/input'} | The binding path `value={path: '/input'}` is an HTML injection sink. |