Merge branch 'main' into jeongsoolee09/add-ui5bindingpath-getnode-case

Author: jeongsoolee09

.github/workflows/cds-extractor-dist-bundle.yml

@@ -19,10 +19,10 @@ jobs:
 
     steps:
     - name: Checkout repository
-      uses: actions/checkout@v4
+      uses: actions/checkout@v5
 
     - name: Setup Node.js
-      uses: actions/setup-node@v4
+      uses: actions/setup-node@v6
       with:
         node-version: '20'
         cache: 'npm'

.github/workflows/code_scanning.yml

@@ -24,7 +24,7 @@ jobs:
 
     steps:
     - name: Checkout repository
-      uses: actions/checkout@v4
+      uses: actions/checkout@v5
 
     - name: Prepare local CodeQL model packs
       run: |
@@ -89,7 +89,7 @@ jobs:
 
     - name: Upload sarif change
       if: steps.validate.outcome != 'success'
-      uses: actions/upload-artifact@v4
+      uses: actions/upload-artifact@v5
       with:
         name: sarif
         path: |

.github/workflows/codeql-ql.yml

@@ -0,0 +1,13 @@
+name: "CodeQL - QL"
+
+on:
+  push:
+    branches: [ "main" ]
+  pull_request:
+    branches: [ "main" ]
+  workflow_dispatch:
+
+jobs:
+  codeql:
+    uses: advanced-security/reusable-workflows/.github/workflows/codeql-ql.yml@main
+    secrets: inherit

.github/workflows/run-codeql-unit-tests-javascript.yml

@@ -18,7 +18,7 @@ jobs:
       matrix: ${{ steps.export-unit-test-matrix.outputs.matrix }}
     steps:
       - name: Checkout repository
-        uses: actions/checkout@v4
+        uses: actions/checkout@v5
 
       - name: Install QLT
         id: install-qlt
@@ -43,7 +43,7 @@ jobs:
 
     steps:
       - name: Checkout repository
-        uses: actions/checkout@v4
+        uses: actions/checkout@v5
 
       - name: Install QLT
         id: install-qlt
@@ -78,7 +78,7 @@ jobs:
           qlt query run install-packs
 
       - name: Setup Node.js for CDS compilation
-        uses: actions/setup-node@v4
+        uses: actions/setup-node@v6
         with:
           node-version: '18'
           cache: 'npm'
@@ -121,7 +121,7 @@ jobs:
           --work-dir $RUNNER_TMP
 
       - name: Upload test results
-        uses: actions/upload-artifact@v4
+        uses: actions/upload-artifact@v5
         with:
           name: test-results-${{ runner.os }}-${{ matrix.codeql_cli }}-${{ matrix.codeql_standard_library_ident }}
           path: |
@@ -135,7 +135,7 @@ jobs:
     steps:
 
       - name: Checkout repository
-        uses: actions/checkout@v4
+        uses: actions/checkout@v5
 
       - name: Install QLT
         id: install-qlt
@@ -146,7 +146,7 @@ jobs:
 
 
       - name: Collect test results
-        uses: actions/download-artifact@v4
+        uses: actions/download-artifact@v6
 
       - name: Validate test results
         run: |

.github/workflows/update-codeql.yml

@@ -17,7 +17,7 @@ jobs:
 
         steps:
         - name: Checkout repository
-          uses: actions/checkout@v4
+          uses: actions/checkout@v5
 
         - name: Check latest CodeQL CLI version and update qlt.conf.json
           id: check-version

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,21 +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/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
@@ -69,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"]
@@ -83,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"]
@@ -131,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,31 +92,37 @@ 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
     )
     or
     /*
-     * A constructor call to sap.ui.model.odata.v2.ODataModel.
+     * A constructor call to sap.ui.model.odata.v2.ODataModel or sap.ui.model.odata.v4.ODataModel.
      */
 
     this instanceof NewNode and
     (
       exists(RequiredObject oDataModel |
         oDataModel.asSourceNode().flowsTo(this.getCalleeNode()) and
-        oDataModel.getDependency() = "sap/ui/model/odata/v2/ODataModel"
+        oDataModel.getDependency() in [
+            "sap/ui/model/odata/v2/ODataModel", "sap/ui/model/odata/v4/ODataModel"
+          ]
       )
       or
       this.getCalleeName() = "ODataModel"
@@ -99,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()
       )
     )
   }
@@ -123,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()
     )
   }
 }