Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
02c8eda
Add small investigative modelling for fragments - WIP
knewbury01 Dec 10, 2025
ad42e91
Fix controls in xml detection
knewbury01 Dec 16, 2025
8f80775
Add simple testcase xss fragment in html view
knewbury01 Dec 17, 2025
b0c1aef
Add Fragment load model and test cases for basic xss
knewbury01 Dec 30, 2025
530df03
Improve UI5Xss detection for fragments_samples 1
data-douser Dec 31, 2025
bb9452e
More potential detection improvements for UI5 XSS
data-douser Dec 31, 2025
97af1ac
Merge branch 'main' into knewbury01/ui5-fragments
mbaluda Jan 2, 2026
4be732a
Fix syntax error in testcases for fragments
knewbury01 Jan 5, 2026
52cab4a
Merge branch 'knewbury01/ui5-fragments' of https://github.com/advance…
knewbury01 Jan 5, 2026
8b2be19
Fix sytnax fragment testcases
knewbury01 Jan 5, 2026
e75912c
Cleanup UI5 & RemoteFlowSources qll
data-douser Jan 6, 2026
56ab432
Cleanup xss-urlparams-jsonmodel query test
data-douser Jan 6, 2026
255b3ca
Add comment to FragmentModule class def
data-douser Jan 8, 2026
caf2596
Merge branch 'knewbury01/ui5-fragments' into knewbury01/dd/ui5-fragments
data-douser Jan 8, 2026
e762b43
Merge pull request #281 from advanced-security/knewbury01/dd/ui5-frag…
knewbury01 Jan 8, 2026
3b1847d
Merge branch 'knewbury01/ui5-fragments' of https://github.com/advance…
knewbury01 Jan 8, 2026
e66d3d8
Update expected js sarif for scan
knewbury01 Jan 8, 2026
af9af3d
Fix testcase naming for views in view files
knewbury01 Jan 8, 2026
e251725
Update expected file for xss-urlparams-jsonmodel after known model up…
knewbury01 Jan 8, 2026
86fd7d5
Add modelling for binding paths and default models when those are use…
knewbury01 Jan 21, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/javascript.sarif.expected

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions javascript/frameworks/ui5/ext/ui5.model.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ extensions:
- ["UI5ClientStorage", "global", "Member[jQuery].Member[sap].Member[storage]"]
- ["UI5ClientStorage", "sap/ui/core/util/File", ""]
- ["UI5ClientStorage", "global", "Member[sap].Member[ui].Member[core].Member[util].Member[File]"]
# Fragment API - byId returns a Control (models static Fragment.byId() calls)
- ["Fragment", "Fragment", "Instance"]
- ["Fragment", "sap/ui/core/Fragment", ""]
- ["UI5InputControl", "Fragment", "Member[byId].ReturnValue"]
# Publishing and Subscribing to Events
- ["UI5EventBusInstance", "sap/ui/core/EventBus", "Member[getInstance].ReturnValue"]
- ["UI5EventBusPublish", "UI5EventBusInstance", "Member[publish]"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,8 @@ class BindingTarget extends TBindingTarget {
this = TLateJavaScriptBindingTarget(target, _)
or
this = TEarlyJavaScriptPropertyBindingTarget(target, _)
or
this = TLateJavaScriptBindingTarget(target.(BindElementMethodCallNode).getReceiver(), _)
) and
result = target
)
Expand Down Expand Up @@ -727,6 +729,22 @@ class Binding extends TBinding {
)
}

DataFlow::Node asDataFlowNode() {
exists(DataFlow::Node target |
(
this = TEarlyJavaScriptPropertyBinding(_, target)
or
this = TLateJavaScriptPropertyBinding(_, target)
or
exists(BindElementMethodCallNode bindElementCall |
target = bindElementCall.getReceiver() and
this = TLateJavaScriptContextBinding(bindElementCall, _)
)
) and
result = target
)
}

BindingPath getBindingPath() { result.getBinding() = this }

BindingTarget getBindingTarget() { result.getBinding() = this }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/**
* Provides classes for modeling the sap.ui.core.Fragment API.
*/

import javascript
import DataFlow
import advanced_security.javascript.frameworks.ui5.UI5

/**
* Gets a reference to the Fragment module import.
Comment thread
knewbury01 marked this conversation as resolved.
*/
class FragmentLoad extends InvokeNode, MethodCallNode {
FragmentLoad() {
this =
TypeTrackers::hasDependency(["sap/ui/core/Fragment", "sap.ui.core.Fragment"])
.getAMemberCall("load")
or
exists(RequiredObject requiredModule, SapDefineModule sapModule |
this = requiredModule.asSourceNode().getAMemberCall("load") and
/* ensure it is an sap module define */
requiredModule.getEnclosingFunction() = sapModule.getArgument(1)
)
}

/**
* Gets the configuration object passed to Fragment.load().
*/
DataFlow::Node getConfigObject() { result = this.getArgument(0) }

/**
* Gets the 'name' property value from the config object.
* This specifies the fragment resource to load (e.g., "my.app.fragments.MyFragment").
*/
DataFlow::Node getNameArgument() {
exists(DataFlow::ObjectLiteralNode config |
config = this.getConfigObject().getALocalSource() and
result = config.getAPropertyWrite("name").getRhs()
)
}

/**
* Gets the 'controller' property value from the config object.
* This specifies the fragment's controller, if it has one.
*/
DataFlow::Node getControllerArgument() {
exists(DataFlow::ObjectLiteralNode config |
config = this.getConfigObject().getALocalSource() and
result = config.getAPropertyWrite("controller").getRhs()
)
}

DataFlow::ParameterNode getCallbackObjectReference() {
//the load invoke node is actually just a part of the chained load.then.bind
result = this.getAMemberCall(_).getABoundCallbackParameter(_, _)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,25 @@ abstract class UI5ExternalModel extends UI5Model, RemoteFlowSource {
abstract string getName();
}

/** Default model which gains content from an SAP OData service (ie no model name is explicitly specified). */
class DefaultODataServiceModel extends UI5ExternalModel {
DefaultODataServiceModel() {
exists(ExternalModelManifest model |
//an OData default model exists
model.getName() = "" and
model.getDataSource() instanceof ODataDataSourceManifest and
//therefore the bindElement calls that exist may be sources and also approximates the model itself
this.getCalleeName() = "bindElement"
)
}

override string getSourceType() { result = "DefaultODataServiceModel" }

override string getName() { result = "" }

Binding asBinding() { result.getBindingTarget().asDataFlowNode() = this }
}

/** Model which gains content from an SAP OData service. */
class ODataServiceModel extends UI5ExternalModel {
string modelName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ private module WebAppResourceRootJsonReader implements JsonParser::MakeJsonReade
class JsonReader extends WebApp {
string getJson() {
// We match on the lowercase to cover all the possible variants of writing the attribute name.
// Support both "data-sap-ui-resourceroots" and "data-sap-ui-resource-roots" (with hyphen)
exists(string resourceRootAttributeName |
resourceRootAttributeName.toLowerCase() = "data-sap-ui-resourceroots"
resourceRootAttributeName.toLowerCase() =
["data-sap-ui-resourceroots", "data-sap-ui-resource-roots"]
|
result = this.getCoreScript().getAttributeByName(resourceRootAttributeName).getValue()
)
Expand Down Expand Up @@ -151,6 +153,17 @@ class SapUiCore extends MethodCallNode {
SapUiCore() { this = globalVarRef("sap").getAPropertyRead("ui").getAMethodCall("getCore") }
}

/**
* A reference to the Fragment module (`sap/ui/core/Fragment`).
* Used for static methods like `Fragment.byId(viewId, controlId)`.
*
* Use of `DataFlow::moduleImport` may not cover byId references
* coming from sources with es6 style imports of Fragments.
*/
class FragmentModule extends DataFlow::SourceNode {
FragmentModule() { this = DataFlow::moduleImport("sap/ui/core/Fragment") }
}

/**
* A user-defined module through `sap.ui.define` or `jQuery.sap.declare`.
*/
Expand Down Expand Up @@ -343,6 +356,7 @@ class ControlReference extends Reference {
string controlId;

ControlReference() {
// Standard byId patterns: this.byId("id"), this.getView().byId("id"), sap.ui.getCore().byId("id")
this.getArgument(0).getALocalSource().getStringValue() = controlId and
(
exists(CustomController controller |
Expand All @@ -352,6 +366,11 @@ class ControlReference extends Reference {
or
exists(SapUiCore sapUiCore | this = sapUiCore.getAMemberCall("byId"))
)
or
// Fragment.byId(viewId, controlId) - static method with 2 arguments
this.getNumArgument() = 2 and
this.getArgument(1).getALocalSource().getStringValue() = controlId and
exists(FragmentModule fragment | this = fragment.getAMemberCall("byId"))
}

CustomControl getDefinition() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import advanced_security.javascript.frameworks.ui5.UI5
import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow
private import semmle.javascript.frameworks.data.internal.ApiGraphModelsExtensions as ApiGraphModelsExtensions
import advanced_security.javascript.frameworks.ui5.Bindings
import advanced_security.javascript.frameworks.ui5.Fragment

/**
* Gets the immediate supertype of a given type from the extensible predicate `typeModel` provided by
Expand Down Expand Up @@ -131,6 +132,24 @@ abstract class UI5BindingPath extends BindingPath {
not exists(controlSetModelCall.getArgument(1)) and
not exists(this.getModelName())
)
or
/* 5. There is no call to `setModel` at all and a default model exists that is related to the binding path this refers to */
exists(DefaultODataServiceModel defaultModel |
result = defaultModel and
not exists(MethodCallNode viewSetModelCall | viewSetModelCall.getMethodName() = "setModel") and
/*
* this binding path can occur in a fragment that is the receiver object for the bindElement model approximation
* i.e. checks that the default model is relevant
*/

exists(FragmentLoad load |
load.getCallbackObjectReference().flowsTo(defaultModel.asBinding().asDataFlowNode()) and
load.getNameArgument()
.getStringValue()
.matches("%" +
this.getLocation().getFile().getBaseName().replaceAll(".fragment.xml", "") + "%")
)
)
)
// and
// /* This binding path and the resulting model should live inside the same webapp */
Expand Down Expand Up @@ -672,8 +691,97 @@ class XmlView extends UI5View instanceof XmlFile {
}
}

/**
* An xml fragment. It may or may not have controllers associated.
*/
class XmlFragment extends UI5View instanceof XmlFile {
XmlRootElement root;

XmlFragment() {
root = this.getARootElement() and
(
root.getNamespace().getUri() = "sap.m"
or
root.getNamespace().getUri() = "sap.ui.core"
) and
root.hasName("FragmentDefinition")
}

override XmlBindingPath getASource() {
exists(XmlElement control, string type, string path, string property |
type = result.getControlTypeName() and
this = control.getFile() and
ApiGraphModelsExtensions::sourceModel(getASuperType(type), path, "remote", _) and
property = path.replaceAll(" ", "").regexpCapture("Member\\[([^\\]]+)\\]", 1) and
result.getBindingTarget() = control.getAttribute(property)
)
}

override XmlBindingPath getAnHtmlISink() {
exists(XmlElement control, string type, string path, string property |
this = control.getFile() and
type = result.getControlTypeName() and
ApiGraphModelsExtensions::sinkModel(getASuperType(type), path, "ui5-html-injection", _) and
property = path.replaceAll(" ", "").regexpCapture("Member\\[([^\\]]+)\\]", 1) and
result.getBindingTarget() = control.getAttribute(property)
)
}

override UI5Control getControl() {
exists(XmlElement element |
result.asXmlControl() = element and
/* Use getAChild+ because some controls nest other controls inside them as aggregations */
element = root.getAChild+() and
(
/* 1. A builtin control provided by UI5 */
isBuiltInControl(element.getNamespace().getUri())
or
/* 2. A custom control with implementation code found in the webapp */
exists(CustomControl control |
control.getName() = element.getNamespace().getUri() + "." + element.getName() and
inSameWebApp(control.getFile(), element.getFile())
)
)
)
}

/**
* This is either known in the location from which `loadFragment` is called (in a controller's init function)
* OR in the optional controller param of `Fragment.load`.
* This MAY return no value, when the fragment is not associated to any controller.
* When this returns a value it is guaranteed that this xml fragment is instantiated.
*/
override string getControllerName() {
exists(CustomController controller, MethodCallNode loadFragmentCall |
loadFragmentCall.getMethodName() = "loadFragment" and
controller.getAThisNode().flowsTo(loadFragmentCall.getReceiver()) and
controller.getName() = result
)
or
exists(CustomController controller, FragmentLoad fragmentLoad |
controller.getAThisNode().flowsTo(fragmentLoad.getControllerArgument()) and
/*
* extracting just the base name of the fragment (not the fully qualified)
* otherwise difficult to know which part of absolute path is only for the qualified name
*/

fragmentLoad
.getNameArgument()
.getStringValue()
.matches("%" + this.getBaseName().replaceAll(".fragment.xml", "")) and
controller.getName() = result
)
}
}

private newtype TUI5Control =
TXmlControl(XmlElement control) or
TXmlControl(XmlElement control) {
control
.(Locatable)
.getFile()
.getBaseName()
.matches(["%.view.xml", "%.view.html", "%.fragment.xml"])
} or
TJsonControl(JsonObject control) {
exists(JsonView view | control.getParent() = view.getRoot().getPropValue("content"))
} or
Expand Down Expand Up @@ -769,11 +877,23 @@ class UI5Control extends TUI5Control {

/**
* Gets a reference to this control. Currently supports only such references made through `byId`.
* Handles both:
* - `this.byId("controlId")` or `this.getView().byId("controlId")` - ID in argument 0
* - `Fragment.byId(viewId, "controlId")` - ID in argument 1
*/
ControlReference getAReference() {
result.getMethodName() = "byId" and
result.getArgument(0).getALocalSource().asExpr().(StringLiteral).getValue() =
this.getProperty("id").getValue()
(
// Standard byId: ID in first argument
result.getNumArgument() = 1 and
result.getArgument(0).getALocalSource().asExpr().(StringLiteral).getValue() =
this.getProperty("id").getValue()
or
// Fragment.byId: ID in second argument
result.getNumArgument() = 2 and
result.getArgument(1).getALocalSource().asExpr().(StringLiteral).getValue() =
this.getProperty("id").getValue()
)
}

/** Gets a property of this control having the name. */
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,36 @@
import javascript
import advanced_security.javascript.frameworks.ui5.UI5
import advanced_security.javascript.frameworks.ui5.UI5View

/**
* Step from a value assigned to a JSONModel property to the binding path that reads it.
* This enables tracking data flowing INTO a model constructor argument and OUT through XML bindings.
*
* e.g. Given:
* ```javascript
* var userInput = getUntrustedData();
* var oModel = new JSONModel({ payload: userInput });
* this.getView().setModel(oModel);
* ```
* and
* ```xml
* <core:HTML content="{/payload}" />
* ```
*
* Creates a flow step from `userInput` (the RHS of the property) to the binding path `{/payload}`.
*/
class JsonModelPropertyValueToBindingStep extends DataFlow::SharedFlowStep {
override predicate step(DataFlow::Node start, DataFlow::Node end) {
exists(UI5BindingPath bindingPath, DataFlow::PropWrite propWrite |
// The binding path resolves to a property write in a JSONModel
bindingPath.getNode() = propWrite and
// The start is the RHS value being assigned to that property
start = propWrite.getRhs() and
// The end is the binding path itself (as a node representing where data flows to)
end = propWrite
)
}
}

/**
* Step from a part of internal model to a relevant control property.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
| sink1.xml:7:5:7:68 | content={path: '/input'} | The binding path `content={path: '/input'}` is an HTML injection sink. |
| sink1.xml:8:5:8:44 | content={path: '/input'} | The binding path `content={path: '/input'}` is an HTML injection sink. |
| sink1.xml:10:5:10:73 | value={path: '/input'} | The binding path `value={path: '/input'}` is an HTML injection sink. |
| sink1.view.xml:7:5:7:68 | content={path: '/input'} | The binding path `content={path: '/input'}` is an HTML injection sink. |
| sink1.view.xml:8:5:8:44 | content={path: '/input'} | The binding path `content={path: '/input'}` is an HTML injection sink. |
| sink1.view.xml:10:5:10:73 | value={path: '/input'} | The binding path `value={path: '/input'}` is an HTML injection sink. |
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
nodes
| webapp/controller/Main.controller.js:20:17:20:29 | sAttackerData |
| webapp/controller/Main.controller.js:20:33:20:97 | Fragmen ... Value() |
| webapp/controller/Main.controller.js:24:34:24:69 | "<span> ... /span>" |
| webapp/controller/Main.controller.js:24:45:24:57 | sAttackerData |
| webapp/view/Main.view.html:4:10:4:34 | data-content={/payload} |
edges
| webapp/controller/Main.controller.js:20:17:20:29 | sAttackerData | webapp/controller/Main.controller.js:24:45:24:57 | sAttackerData |
| webapp/controller/Main.controller.js:20:33:20:97 | Fragmen ... Value() | webapp/controller/Main.controller.js:20:17:20:29 | sAttackerData |
| webapp/controller/Main.controller.js:24:45:24:57 | sAttackerData | webapp/controller/Main.controller.js:24:34:24:69 | "<span> ... /span>" |
#select
| webapp/controller/Main.controller.js:24:34:24:69 | "<span> ... /span>" | webapp/controller/Main.controller.js:20:33:20:97 | Fragmen ... Value() | webapp/controller/Main.controller.js:24:34:24:69 | "<span> ... /span>" | XSS vulnerability due to $@. | webapp/controller/Main.controller.js:20:33:20:97 | Fragmen ... Value() | user-provided value |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
UI5Xss/UI5Xss.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "ui5-xss-fragment-static-byid",
"version": "1.0.0",
"main": "index.js"
}
Loading