Conversation
javascript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment-load/webapp/view/app.view.html
Dismissed
Show dismissed
Hide dismissed
javascript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment/webapp/view/app.view.html
Dismissed
Show dismissed
Hide dismissed
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds support for modeling XML fragments in SAPUI5 applications to enable XSS vulnerability detection. It introduces the ability to track data flow through programmatically instantiated fragments using both Controller.loadFragment() and Fragment.load() APIs.
Key changes:
- New
Fragment.qllmodule to model Fragment.load() API calls - Extended
UI5View.qllwith XmlFragment class to handle fragment definitions - Added two test cases demonstrating XSS detection through fragments loaded via different methods
Reviewed changes
Copilot reviewed 22 out of 24 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/Fragment.qll | New module defining FragmentLoad class to model sap.ui.core.Fragment.load() API calls and extract configuration parameters |
| javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5View.qll | Extended with XmlFragment class (lines 690-777) to model XML fragments, their controllers, sources, and sinks; updated TUI5Control to include fragment.xml files |
| javascript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment/* | Test case using Controller.loadFragment() to load a fragment with XSS source and sink |
| javascript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment-load/* | Test case using Fragment.load() with explicit controller parameter to load a fragment with XSS source and sink |
Files not reviewed (2)
- javascript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment-load/package-lock.json: Language not supported
- javascript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment-load/webapp/controller/app.controller.js
Outdated
Show resolved
Hide resolved
javascript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment-load/webapp/index.js
Show resolved
Hide resolved
javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/Fragment.qll
Show resolved
Hide resolved
javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5View.qll
Outdated
Show resolved
Hide resolved
...ript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment/webapp/controller/app.controller.js
Outdated
Show resolved
Hide resolved
...ript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment/webapp/controller/app.controller.js
Outdated
Show resolved
Hide resolved
...frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment-load/webapp/controller/app.controller.js
Outdated
Show resolved
Hide resolved
javascript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment/webapp/index.js
Show resolved
Hide resolved
javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/Fragment.qll
Outdated
Show resolved
Hide resolved
…d-security/codeql-sap-js into knewbury01/ui5-fragments
…ments Improve UI5Xss detection for fragments_samples 1
...eworks/ui5/test/queries/UI5Xss/xss-fragment-static-byid/webapp/controller/Main.controller.js
Dismissed
Show dismissed
Hide dismissed
...orks/ui5/test/queries/UI5Xss/xss-urlparams-jsonmodel/webapp/controller/Display.controller.js
Dismissed
Show dismissed
Hide dismissed
...eworks/ui5/test/queries/UI5Xss/xss-urlparams-jsonmodel/webapp/fragments/Content.fragment.xml
Dismissed
Show dismissed
Hide dismissed
...ript/frameworks/ui5/test/queries/UI5Xss/xss-urlparams-jsonmodel/webapp/view/Display.view.xml
Dismissed
Show dismissed
Hide dismissed
…d-security/codeql-sap-js into knewbury01/ui5-fragments
and change htmlsink def to updated one
…d through fragments
data-douser
approved these changes
Jan 22, 2026
Collaborator
There was a problem hiding this comment.
Verified that tests pass locally for all javascript/frameworks/ui5/test/**.
Merging this now, with an eye on re-assessing the changes from #276 for compatibility with changes from this PR.
LGTM! @knewbury01
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What This PR Contributes
QL models and tests for Xml Fragments that are programatically instantiated. Covers both the use case of loading a fragment via the Fragment itself (
Fragment.load) as well as a controller'sloadFragment. Additionally specific modelling to handle use of the default odata model with xml fragments.Future Works
<Fragment>tags and no programatic controller association (declarative views)