Model sap/ui/core/EventBus
#258
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -0,0 +1 @@ | |||
| UI5Xss/UI5Xss.ql No newline at end of file | |||
Check warning
Code scanning / CodeQL-Community
Query test without inline test expectations
| customController.getAThisNode() = result.getReceiver() and | ||
| result.getMethodName() = "getOwnerComponent" | ||
| or | ||
| exists(TypeTracker t2 | result = getOwnerComponentRef(t2, customController).track(t2, t)) |
Check warning
Code scanning / CodeQL-Community
Var only used in one side of disjunct.
| exists(TypeTracker t2 | result = getOwnerComponentRef(t2, customController).track(t2, t)) | ||
| } | ||
|
|
||
| /* owner component ref */ |
Check warning
Code scanning / CodeQL-Community
Block comment that is not QLDoc
| result = getOwnerComponentRef(TypeTracker::end(), customController) | ||
| } | ||
|
|
||
| private class ObjFieldStep extends SharedTypeTrackingStep { |
Check warning
Code scanning / CodeQL-Community
Dead code
| } | ||
| } | ||
|
|
||
| private DataFlow::SourceNode getAnAlias(DataFlow::SourceNode object) { |
Check warning
Code scanning / CodeQL-Community
Dead code
...rameworks/ui5/test/queries/UI5Xss/xss-eventbus-with-data/webapp/controller/app.controller.js
Dismissed
Show dismissed
Hide dismissed
jeongsoolee09
changed the title
sap/ui/core/EventBussap/ui/core/EventBus
The previously uploaded javascript.sarif.expected file had its encoding broken and chocked the job. We make it turn back to the previous version to unblock it.
There was a problem hiding this comment.
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.
...ipt/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/TypeTrackers.qll
Show resolved
Hide resolved
mbaluda
left a comment
mbaluda
left a comment
There was a problem hiding this comment.
- Can you add a reference to the new test in
ui5/test/README.md? - Can you comment what is new in
TypeTrackers.qll?
mbaluda
left a comment
mbaluda
left a comment
There was a problem hiding this comment.
Support the following way of getting an EventBus instance:
this.getOwnerComponent().getEventBus()
sap.ui.getCore().getEventBus()
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
publishandsubscribemethods which are used by the publishers and subscribers, respectively:EventBus.getInstance().publish("someChannel", "someMessageType", data)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.
Future Works
UI5PathNode.getAPrimarySource/0only matches on data binding paths and does not on the optionalidproperty. Doing so enable the same level of rich alert information by incorporating the XML view in the path information, but it requires large-scale rewrite of the library. We leave this as a future work for now.