Add small investigative modelling for fragments - WIP #270

knewbury01 · 2025-12-10T21:22:20Z

What This PR Contributes

QL models and tests for Xml Fragments that are programatically instantiated

Future Works

js fragments entirely
fragments statically associated to views (declared via <Fragment> tags and no programatic controller association (declarative views)
consideration for the way to express whether a fragment is instantiated or not (currently built into getControllerName in XmlFragment, but this could be replaced with a similar mechanism to the placeAt dynamic model in remote flow sources, or possibly inherently in the detection of the fragment itself (ie to say a fragment is only a fragment once it is instantiated - though may be complex when there is no controller)?

javascript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment-load/webapp/view/app.view.html

javascript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment/webapp/view/app.view.html

Copilot

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.qll module to model Fragment.load() API calls
Extended UI5View.qll with 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.

Copilot · 2026-01-02T13:46:38Z

...frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment-load/webapp/controller/app.controller.js

+    "sap/ui/model/json/JSONModel",
+    "sap/ui/core/Fragment"
+], function (Controller, JSONModel, Fragment) {
+    "use strict"


Missing semicolon after the "use strict" directive. This should be "use strict"; for consistency and to avoid potential issues with ASI (Automatic Semicolon Insertion).

Suggested change

"use strict"

"use strict";

Copilot · 2026-01-02T13:46:38Z

javascript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment-load/webapp/index.js

+        oView.placeAt("content");
+    });
+
+}); 


Missing semicolon after the closing parenthesis of the function call. The statement should end with a semicolon for consistency with JavaScript best practices.

Suggested change

});

});

Copilot · 2026-01-02T13:46:38Z

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

+import advanced_security.javascript.frameworks.ui5.UI5
+
+/**
+ * Gets a reference to the Fragment module import.


The documentation comment says "Gets a reference to the Fragment module import" but the class name is FragmentLoad, which represents a Fragment.load() call rather than just a module import. The comment should be updated to clarify that this class models calls to Fragment.load() or Controller.loadFragment().

Suggested change

* Gets a reference to the Fragment module import.

* Models calls to sap.ui.core.Fragment.load() or Controller.loadFragment().

Copilot · 2026-01-02T13:46:39Z

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

+/**
+ * A UI5 Fragment that might include XSS sources and sinks in standard controls.
+ */
+abstract class UI5Fragment extends File {
+  abstract UI5Control getControl();
+
+  abstract UI5BindingPath getASource();
+
+  abstract UI5BindingPath getAnHtmlISink();
+}


The abstract class UI5Fragment is defined but never used. The XmlFragment class extends UI5View instead of UI5Fragment. Either remove this unused abstract class or update XmlFragment to extend UI5Fragment if that was the original intent. If UI5Fragment is intended for future use (as mentioned in the PR description for JS fragments), consider adding a comment to clarify this.

Copilot · 2026-01-02T13:46:39Z

...ript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment/webapp/controller/app.controller.js

+    "sap/ui/core/mvc/Controller",
+    "sap/ui/model/json/JSONModel"
+], function (Controller, JSONModel) {
+    "use strict"


Missing semicolon after the "use strict" directive. This should be "use strict"; for consistency and to avoid potential issues with ASI (Automatic Semicolon Insertion).

Suggested change

"use strict"

"use strict";

Copilot · 2026-01-02T13:46:39Z

...ript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment/webapp/controller/app.controller.js

+            }.bind(this));
+        }
+    });
+})


Missing semicolon after the closing parenthesis of the function call. The statement should end with a semicolon for consistency with JavaScript best practices.

Suggested change

})

});

Copilot · 2026-01-02T13:46:39Z

...frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment-load/webapp/controller/app.controller.js

+            }.bind(this));
+        }
+    });
+})


Missing semicolon after the closing parenthesis of the function call. The statement should end with a semicolon for consistency with JavaScript best practices.

Suggested change

})

});

Copilot · 2026-01-02T13:46:40Z

javascript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment/webapp/index.js

+        oView.placeAt("content");
+    });
+
+}); 


Missing semicolon after the closing parenthesis of the function call. The statement should end with a semicolon for consistency with JavaScript best practices.

Suggested change

});

});

;

Copilot · 2026-01-02T13:46:40Z

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

+    or
+    exists(RequiredObject requiredModule, SapDefineModule sapModule |
+      this = requiredModule.asSourceNode().getAMemberCall("load") and
+      //ensure it is an sap module define


Inconsistent comment style. This line uses a C++-style comment (//) while the rest of the file uses QLDoc comments (/** ... */). Consider using a QLDoc comment or block comment for consistency.

Suggested change

//ensure it is an sap module define

/* ensure it is an sap module define */

Add small investigative modelling for fragments - WIP

02c8eda

knewbury01 mentioned this pull request Dec 11, 2025

UI5 xml control improvement #265

Closed

knewbury01 added 3 commits December 16, 2025 10:53

Fix controls in xml detection

ad42e91

Add simple testcase xss fragment in html view

8f80775

Add Fragment load model and test cases for basic xss

b0c1aef

knewbury01 requested a review from mbaluda December 30, 2025 17:42

github-advanced-security bot found potential problems Dec 30, 2025

View reviewed changes

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

knewbury01 requested a review from data-douser December 30, 2025 17:43

data-douser added enhancement New feature or request javascript Pull requests that update javascript code labels Dec 30, 2025

Merge branch 'main' into knewbury01/ui5-fragments

97af1ac

mbaluda requested a review from Copilot January 2, 2026 13:42

Copilot started reviewing on behalf of mbaluda January 2, 2026 13:43 View session

Copilot AI reviewed Jan 2, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add small investigative modelling for fragments - WIP #270

Add small investigative modelling for fragments - WIP #270

Uh oh!

knewbury01 commented Dec 10, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 2, 2026

Uh oh!

Copilot AI Jan 2, 2026

Uh oh!

Copilot AI Jan 2, 2026

Uh oh!

Copilot AI Jan 2, 2026

Uh oh!

Copilot AI Jan 2, 2026

Uh oh!

Copilot AI Jan 2, 2026

Uh oh!

Copilot AI Jan 2, 2026

Uh oh!

Copilot AI Jan 2, 2026

Uh oh!

Copilot AI Jan 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	* Gets a reference to the Fragment module import.
	* Models calls to sap.ui.core.Fragment.load() or Controller.loadFragment().

	//ensure it is an sap module define
	/* ensure it is an sap module define */

Add small investigative modelling for fragments - WIP #270

Are you sure you want to change the base?

Add small investigative modelling for fragments - WIP #270

Uh oh!

Conversation

knewbury01 commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What This PR Contributes

Future Works

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

knewbury01 commented Dec 10, 2025 •

edited

Loading