Model sap/ui/core/EventBus

[jeongsoolee09]

What This PR Contributes

Add support for message passing through EventBus, where there are event publishers and ones that subscribe to those events. EventBus is a singleton object that has publish and subscribe methods which are used by the publishers and subscribers, respectively:

• A publisher publishes a message with optional data through a channel: e.g. EventBus.getInstance().publish("someChannel", "someMessageType", data)
• A subscriber receives the message (with optional data) through the channel: e.g. EventBus.getInstance().subscribe("someChannel", "someMessageType", function(channel, event, data) { ... }, this). Here, the callback argument handles the message with parameters bound to the message's channel, event, and data.

Note that for a channel and message type pair, there may be multiple subscribers to a publisher, establishing a broadcasting mechanism.

Three different types of EventBuses supported

1. EventBus.getInstance()
   • This is the global event bus that broadcasts across the entire application.
2. sap.ui.core.getEventBus()
   • This is an old way of the first case.
3. oController.getOwnerComponent().getEventBus()
   • This is an event bus that pertains to a component that is part of a UI5 application.

(Important!) Global tracking of CustomControl / CustomController using MaD + API Graphs

This PR heavily uses MaD + API graphs to track the APIs pertaining to event buses, and moves the QL-driven definition of CustomControl / CustomController to using the combination. The corollary of this is that given this code:

Controller.extend("codeql-sap-js.controller.App3", {
    onInit: function() {
      let oData = {
        input: null,
        output1: null
      };
      let oModel = new JSONModel(oData);
      this.getView().setModel(oModel);
      this.bus = this.getOwnerComponent().getEventBus();
    },

    doSomething1() {
      let oInput = this.getView().byId("input");
      let value = oInput.getValue();
      this.bus.publish("xssChannel", "xss", { message: value });
    }
  })

The existing definition could not infer that this.bus.publish calls this.getOwnerComponent().getEventBus().publish since the code that connects both (this.bus = this.getOwnerComponent().getEventBus()) is in another method. Hacking with isAdditionalFlowStep can go only far, since it deals with access paths (this.bus).

Therefore, instead of dealing with this in QL, we use MaD instead, since the resolution mechanism of MaD is much more powerful yet incredibly succinct compared to describing the same matter in QL.

Rationale of removing ControlTypeInHandlerModel

The ControlTypeInHandlerModel was a subclass of ModelInput::TypeModel:

/**
 * Models controller references in event handlers as types
 */
overlay[local?]
class ControlTypeInHandlerModel extends ModelInput::TypeModel {
  override DataFlow::CallNode getASource(string type) {
    // oEvent.getSource() is of the type of the Control calling the handler
    exists(UI5Handler h |
      type = h.getControl().getImportPath() and
      result.getCalleeName() = "getSource" and
      result.getReceiver().getALocalSource() = h.getParameter(0)
    )
    or
    // this.getView().byId("id") is of the type of the Control with id="id"
    exists(UI5Control c |
      type = c.getImportPath() and
      result = c.getAReference()
    )
  }

  /**
   * Prevents model pruning for `ControlType`types
   */
  bindingset[type]
  override predicate isTypeUsed(string type) { any() }
}

ModelInput::TypeModel is a singleton (Unit) class, so extending this class has the effect of adding rows to the class. Overriding getASource is the extension point to adding data to the class.

In the above code, we were adding two things into the class:

1. Any handler parameter associated with a UI5 control in the JS code
2. Any reference to a UI5 control in the JS code

Then, the handler parameter or the UI5 control in the JS code would have the same type as declared in the typeModel extensible predicate. This enables, for example, a reference to sap.m.Input in the JS code fetched by the byId API this.getView().byId("id-of-Input-control"") to act as its surrogate and this.getView().byId("id-of-Input-control"").getValue() would be recognized as a RemoteFlowSource, and similarly the argument arg as in the reference this.getView().byId("id-of-HTML-control").setContent(arg) to sap.m.HTML to be recognized as a sink.

It's proved itself useful until it's discovered that this has is twofold:

1. The above class overapproximates. For example, if there is a control that isn't capable of being a source for XSS (e.g. sap.m.Button), its reference (e.g. this.getView().byId("id-of-Button-control")) will be recognized as RemoteFlowSource; it's obviously unsound.
2. Most importantly, the above class depends on QL code CustomController, and makes it impossible for CustomController to use API graphs + MaD in any way. This is problematic since delegating API tracking to MaD as much as possible and using QL only sparingly is the direction this repo would want to take (see the this.bus.subscribe example above to see why).

Therefore, we remove the class altogether and define classes that defines remote flow sources / sinks for the above:

1. UserDataFromRemoteControlAPISource for handler parameter + UI5 reference that is associated with an input control capable of being an XSS source, and
2. UI5HTMLControlReferenceContentAPI for HTML control reference whose content property and its setter acts as XSS sinks.

Morale of this: In general, avoid contributing back to MaD using ModelInput::TypeModel and its friends, and only consume from it.

Future Works

(Won't implement) EventBus.unsubscribe

EventBus.unsubscribe allows unsubscribing from a channel for an event (message) type. Exactly when the event bus unsubscribes from a (channel, event type) pair is strictly runtime property and impossible to accurately determine statically. Therefore, we exclude it from our consideration.

[Copilot]

Pull request overview

This PR adds support for modeling message passing through SAP UI5's EventBus API, enabling CodeQL to detect data flow vulnerabilities that traverse the publish-subscribe pattern. The implementation tracks data from EventBus.publish() calls to the corresponding callback parameters in EventBus.subscribe() handlers.

Key Changes

• Added data flow step connecting published event data to subscription handler parameters
• Refactored TypeTrackers module into a separate file for better code organization
• Created comprehensive test case demonstrating XSS vulnerability through EventBus

Reviewed changes

Copilot reviewed 13 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
ui5.model.yml	Adds model definitions for UI5PublishedEventData and UI5EventSubscriptionHandlerDataParameter
FlowSteps.qll	Implements PublishedEventToEventSubscribedEventData flow step
TypeTrackers.qll	New file containing refactored type tracking predicates
UI5.qll	Removes TypeTrackers module and imports it from new location
xss-eventbus-with-data/	Complete test case demonstrating XSS through EventBus publish/subscribe

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mbaluda]

• Can you add a reference to the new test in ui5/test/README.md?
• Can you comment what is new in TypeTrackers.qll?

[mbaluda]

Support the following way of getting an EventBus instance:
this.getOwnerComponent().getEventBus()
sap.ui.getCore().getEventBus()

[knewbury01]

approving with an eye on checks yet to pass, and one small convo to resolve still on PR doc improvement