Support dynamically instantiated UI5 controls placed at a DOM tree

javascript/frameworks/cap/lib/advanced_security/javascript_sap_cap_all/Customizations.qll

@@ -1,6 +1,8 @@
-// This file is included for use in custom CodeQL bundles (https://github.com/advanced-security/codeql-bundle).
-// The contents of this file will be included in the standard library `Customizations.qll`, and will therefore
-// be included in the out-of-the-box security queries.
-//
-// We import under alias to avoid any potential naming conflicts
+/**
+ * This file is included for use in custom CodeQL bundles (https://github.com/advanced-security/codeql-bundle).
+ * The contents of this file will be included in the standard library `Customizations.qll`, and will therefore
+ * be included in the out-of-the-box security queries.
+ */
+
+/* We import under alias to avoid any potential naming conflicts */
 import advanced_security.javascript.frameworks.cap.RemoteFlowSources as CAPRemoteFlowSources

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

@@ -15,9 +15,9 @@ extensions:
       - ["RenderManager", "Renderer", "Member[render].Parameter[0]"]
       - ["Patcher", "Patcher", "Instance"]
       - ["Patcher", "sap/ui/core/Patcher", ""]
-      - ["HTMLControl", "HTMLControl", "Instance"]
-      - ["HTMLControl", "sap/ui/core/HTML", ""]
-      - ["HTMLControl", "global", "Member[sap].Member[ui].Member[core].Member[HTML]"]
+      - ["UI5HTMLControl", "UI5HTMLControl", "Instance"]
+      - ["UI5HTMLControl", "sap/ui/core/HTML", ""]
+      - ["UI5HTMLControl", "global", "Member[sap].Member[ui].Member[core].Member[HTML]"]
       - ["Assert", "global", "Member[sap].Member[base].Member[assert]"]
       - ["Assert", "global", "Member[jQuery].Member[sap].Member[assert]"]
       - ["SapLogger", "sap/base/Log", ""]
@@ -41,27 +41,27 @@ extensions:
       - ["SapUIDom", "sap/ui/dom", ""]
       - ["SapUIDom", "global", "Member[sap].Member[ui].Member[dom]"]
       # model Controls inheritance
-      - ["UI5Control", "UI5Control", "Instance"]
-      - ["UI5Control", "sap/m/InputBase", ""]
-      - ["UI5Control", "sap/m/Input", ""]
-      - ["UI5Control", "sap/ui/webc/main/Input", ""]
-      - ["UI5Control", "sap/ui/commons/TextField", ""]
-      - ["UI5Control", "sap/ui/commons/PasswordField", ""]
-      - ["UI5Control", "sap/ui/commons/ValueHelpField", ""]
-      - ["UI5Control", "sap/ui/commons/SearchField", ""]
-      - ["UI5Control", "sap/ui/commons/ComboBox", ""]
-      - ["UI5Control", "sap/ui/commons/TextArea", ""]
-      - ["UI5Control", "sap/ui/webc/main/TextArea", ""]
-      - ["UI5Control", "sap/m/ComboBoxTextField", ""]
-      - ["UI5Control", "sap/m/MaskEnabler", ""]
-      - ["UI5Control", "sap/m/MaskInput", ""]
-      - ["UI5Control", "sap/m/TextArea", ""]
-      - ["UI5Control", "sap/m/ComboBoxBase", ""]
-      - ["UI5Control", "sap/m/MultiInput", ""]
-      - ["UI5Control", "sap/ui/webc/main/MultiInput", ""]
-      - ["UI5Control", "sap/m/SearchField", ""]
-      - ["UI5Control", "sap/m/FeedInput", ""]
-      - ["UI5Control", "sap/ui/richtexteditor/RichTextEditor", ""]
+      - ["UI5InputControl", "UI5InputControl", "Instance"]
+      - ["UI5InputControl", "sap/m/InputBase", ""]
+      - ["UI5InputControl", "sap/m/Input", ""]
+      - ["UI5InputControl", "sap/ui/webc/main/Input", ""]
+      - ["UI5InputControl", "sap/ui/commons/TextField", ""]
+      - ["UI5InputControl", "sap/ui/commons/PasswordField", ""]
+      - ["UI5InputControl", "sap/ui/commons/ValueHelpField", ""]
+      - ["UI5InputControl", "sap/ui/commons/SearchField", ""]
+      - ["UI5InputControl", "sap/ui/commons/ComboBox", ""]
+      - ["UI5InputControl", "sap/ui/commons/TextArea", ""]
+      - ["UI5InputControl", "sap/ui/webc/main/TextArea", ""]
+      - ["UI5InputControl", "sap/m/ComboBoxTextField", ""]
+      - ["UI5InputControl", "sap/m/MaskEnabler", ""]
+      - ["UI5InputControl", "sap/m/MaskInput", ""]
+      - ["UI5InputControl", "sap/m/TextArea", ""]
+      - ["UI5InputControl", "sap/m/ComboBoxBase", ""]
+      - ["UI5InputControl", "sap/m/MultiInput", ""]
+      - ["UI5InputControl", "sap/ui/webc/main/MultiInput", ""]
+      - ["UI5InputControl", "sap/m/SearchField", ""]
+      - ["UI5InputControl", "sap/m/FeedInput", ""]
+      - ["UI5InputControl", "sap/ui/richtexteditor/RichTextEditor", ""]
       - ["UI5CodeEditor", "UI5CodeEditor", "Instance"]
       - ["UI5CodeEditor", "sap/ui/codeeditor/CodeEditor", ""]
       # Classes that provide Path injection APIs
@@ -75,8 +75,8 @@ extensions:
       pack: codeql/javascript-all
       extensible: "sourceModel"
     data:
-      - ["UI5Control", "Member[value]", "remote"]
-      - ["UI5Control", "Member[getValue].ReturnValue", "remote"]
+      - ["UI5InputControl", "Member[value]", "remote"]
+      - ["UI5InputControl", "Member[getValue].ReturnValue", "remote"]
       - ["UI5CodeEditor", "Member[value]", "remote"]
       - ["UI5CodeEditor", "Member[getCurrentValue].ReturnValue", "remote"]
       - ["global", "Member[jQuery].Member[sap].Member[syncHead,syncGet,syncGetText,syncPost,syncPostText].ReturnValue", "remote"]
@@ -89,9 +89,9 @@ extensions:
     data:
       # html-injection
       - ["global", "Member[jQuery].Member[sap].Member[globalEval].Argument[0]", "ui5-html-injection"]
-      - ["HTMLControl", "Argument[0].Member[content]", "ui5-html-injection"]
-      - ["HTMLControl", "Member[content]", "ui5-html-injection"]
-      - ["HTMLControl", "Member[setContent].Argument[0]", "ui5-html-injection"]
+      - ["UI5HTMLControl", "Argument[0].Member[content]", "ui5-html-injection"]
+      - ["UI5HTMLControl", "Member[content]", "ui5-html-injection"]
+      - ["UI5HTMLControl", "Member[setContent].Argument[0]", "ui5-html-injection"]
       - ["Patcher", "Member[unsafeHtml].Argument[0..]", "ui5-html-injection"]
       - ["RenderManager", "Member[write,unsafeHtml].Argument[0..]", "ui5-html-injection"]
       - ["RenderManager", "Member[writeAttribute,addStyle].Argument[0..1]", "ui5-html-injection"]
@@ -137,3 +137,5 @@ extensions:
       - ["sap/base/security/encodeURL", "", "Argument[0]", "ReturnValue", "taint"]
       - ["sap/base/security/encodeURLParameters", "", "Argument[0]", "ReturnValue", "taint"]
       - ["sap/base/security/encodeXML", "", "Argument[0]", "ReturnValue", "taint"]
+      - ["UI5HTMLControl", "", "Argument[0].Member[content]", "ReturnValue.Member[content]", "taint"]
+      - ["UI5HTMLControl", "Instance.Member[getContent]", "Argument[this].Member[content]", "ReturnValue", "taint"]

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

@@ -1,22 +1,54 @@
 import javascript
 import advanced_security.javascript.frameworks.ui5.UI5
 import advanced_security.javascript.frameworks.ui5.UI5View
-private import semmle.javascript.frameworks.data.internal.ApiGraphModelsExtensions as ApiGraphModelsExtensions
+import semmle.javascript.security.dataflow.XssThroughDomCustomizations
+private import semmle.javascript.frameworks.data.internal.ApiGraphModelsExtensions
 
-private class DataFromRemoteControlReference extends RemoteFlowSource, MethodCallNode {
+private class DataFromRemoteControlReference extends RemoteFlowSource {
   DataFromRemoteControlReference() {
     exists(UI5Control sourceControl, string typeAlias, ControlReference controlReference |
-      ApiGraphModelsExtensions::typeModel(typeAlias, sourceControl.getImportPath(), _) and
-      ApiGraphModelsExtensions::sourceModel(typeAlias, _, "remote", _) and
+      typeModel(typeAlias, sourceControl.getImportPath(), _) and
+      sourceModel(typeAlias, _, "remote", _) and
       sourceControl.getAReference() = controlReference and
-      controlReference.flowsTo(this.getReceiver()) and
-      this.getMethodName() = "getValue"
+      (
+        this = controlReference.getAMemberCall("getValue") or
+        this = controlReference.getAPropertyRead("value")
+      )
     )
   }
 
   override string getSourceType() { result = "Data from a remote control" }
 }
 
+private class InputControlInstantiation extends ElementInstantiation {
+  InputControlInstantiation() { typeModel("UI5InputControl", this.getImportPath(), _) }
+}
+
+private module TrackPlaceAtCallConfigFlow = TaintTracking::Global<TrackPlaceAtCallConfig>;
+
+class DataFromInstantiatedAndPlacedAtControl extends RemoteFlowSource, XssThroughDom::Source {
+  InputControlInstantiation controlInstantiation;
+  ControlPlaceAtCall placeAtCall;
+
+  DataFromInstantiatedAndPlacedAtControl() {
+    exists(string typeAlias, ControlReference controlReference |
+      /* Double check that the type derives a remote flow source. */
+      typeModel(typeAlias, controlInstantiation.getImportPath(), _) and
+      sourceModel(typeAlias, _, "remote", _) and
+      controlInstantiation.getId() = controlReference.getId() and
+      (
+        this = controlReference.getAMemberCall("getValue") or
+        this = controlReference.getAPropertyRead("value")
+      )
+    ) and
+    TrackPlaceAtCallConfigFlow::flow(controlInstantiation, placeAtCall)
+  }
+
+  override string getSourceType() {
+    result = "Data from an instantiated control placed in a DOM tree"
+  }
+}
+
 class LocalModelContentBoundBidirectionallyToSourceControl extends RemoteFlowSource {
   UI5BindingPath bindingPath;
   UI5Control controlDeclaration;
@@ -60,18 +92,22 @@ class ODataServiceModel extends UI5ExternalModel {
   ODataServiceModel() {
     exists(MethodCallNode setModelCall, CustomController controller |
       /*
-       * 1. This flows from a DF node corresponding to the parent component's model to the `this.setModel` call
-       * i.e. Aims to capture something like `this.getOwnerComponent().getModel("someModelName")` as in
-       * `this.getView().setModel(this.getOwnerComponent().getModel("someModelName"))`
+       * 1. This flows from a DF node corresponding to the parent component's model
+       * to the `this.setModel` call. e.g.
+       *
+       * `this.getOwnerComponent().getModel("someModelName")` as in
+       * `this.getView().setModel(this.getOwnerComponent().getModel("someModelName"))`.
        */
 
       modelName = this.getArgument(0).getALocalSource().asExpr().(StringLiteral).getValue() and
       this.getCalleeName() = "getModel" and
       controller.getOwnerComponentRef().flowsTo(this.(MethodCallNode).getReceiver()) and
       this.flowsTo(setModelCall.getArgument(0)) and
-      setModelCall.getMethodName() = "setModel" and
-      setModelCall.getReceiver() = controller.getAViewReference() and
-      /* 2. The component's manifest.json declares the DataSource as being of OData type */
+      setModelCall = controller.getAViewReference().getAMemberCall("setModel") and
+      /*
+       * 2. The component's `manifest.json` declares the DataSource as being of OData type.
+       */
+
       controller.getOwnerComponent().getExternalModelDef(modelName).getDataSource() instanceof
         ODataDataSourceManifest
     )
@@ -101,21 +137,17 @@ private class RouteParameterAccess extends RemoteFlowSource instanceof PropRead
   override string getSourceType() { result = "RouteParameterAccess" }
 
   RouteParameterAccess() {
-    exists(
-      ControllerHandler handler, RouteManifest routeManifest, ParameterNode handlerParameter,
-      MethodCallNode getParameterCall
-    |
+    exists(ControllerHandler handler, RouteManifest routeManifest, MethodCallNode getParameterCall |
       handler.isAttachedToRoute(routeManifest.getName()) and
       this.asExpr().getEnclosingFunction() = handler.getFunction() and
-      handlerParameter = handler.getParameter(0) and
-      getParameterCall.getMethodName() = "getParameter" and
-      getParameterCall.getReceiver().getALocalSource() = handlerParameter and
+      getParameterCall = handler.getParameter(0).getAMemberCall("getParameter") and
       (
-        routeManifest.matchesPathString(this.getPropertyName()) and
-        this.getBase().getALocalSource() = getParameterCall
+        exists(string path |
+          this = getParameterCall.getAPropertyRead(path) and
+          routeManifest.matchesPathString(path)
+        )
         or
-        /* TODO: Why does `routeManifest.matchesPathString` not work for propertyName?? */
-        this.getBase().(PropRead).getBase().getALocalSource() = getParameterCall
+        this = getParameterCall.getAPropertyRead().getAPropertyRead()
       )
     )
   }
@@ -125,10 +157,8 @@ private class DisplayEventHandlerParameterAccess extends RemoteFlowSource instan
   override string getSourceType() { result = "DisplayEventHandlerParameterAccess" }
 
   DisplayEventHandlerParameterAccess() {
-    exists(DisplayEventHandler handler, MethodCallNode getParameterCall |
-      getParameterCall.getMethodName() = "getParameter" and
-      this.getBase().getALocalSource() = getParameterCall and
-      handler.getParameter(0) = getParameterCall.getReceiver().getALocalSource()
+    exists(DisplayEventHandler handler |
+      this = handler.getParameter(0).getAMemberCall("getParameter").getAPropertyRead()
     )
   }
 }