Skip to content

Commit b0c1aef

Browse files
knewbury01knewbury01
committed
Add Fragment load model and test cases for basic xss
1 parent 8f80775 commit b0c1aef

16 files changed

Lines changed: 241 additions & 9 deletions

File tree

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/**
2+
* Provides classes for modeling the sap.ui.core.Fragment API.
3+
*/
4+
5+
import javascript
6+
import DataFlow
7+
import advanced_security.javascript.frameworks.ui5.UI5
8+
9+
/**
10+
* Gets a reference to the Fragment module import.
11+
*/
12+
class FragmentLoad extends InvokeNode, MethodCallNode {
13+
FragmentLoad() {
14+
this =
15+
TypeTrackers::hasDependency(["sap/ui/core/Fragment", "sap.ui.core.Fragment"])
16+
.getAMemberCall("load")
17+
or
18+
exists(RequiredObject requiredModule, SapDefineModule sapModule |
19+
this = requiredModule.asSourceNode().getAMemberCall("load") and
20+
//ensure it is an sap module define
21+
requiredModule.getEnclosingFunction() = sapModule.getArgument(1)
22+
)
23+
}
24+
25+
/**
26+
* Gets the configuration object passed to Fragment.load().
27+
*/
28+
DataFlow::Node getConfigObject() { result = this.getArgument(0) }
29+
30+
/**
31+
* Gets the 'name' property value from the config object.
32+
* This specifies the fragment resource to load (e.g., "my.app.fragments.MyFragment").
33+
*/
34+
DataFlow::Node getNameArgument() {
35+
exists(DataFlow::ObjectLiteralNode config |
36+
config = this.getConfigObject().getALocalSource() and
37+
result = config.getAPropertyWrite("name").getRhs()
38+
)
39+
}
40+
41+
/**
42+
* Gets the 'controller' property value from the config object.
43+
* This specifies the fragment's controller, if it has one.
44+
*/
45+
DataFlow::Node getControllerArgument() {
46+
exists(DataFlow::ObjectLiteralNode config |
47+
config = this.getConfigObject().getALocalSource() and
48+
result = config.getAPropertyWrite("controller").getRhs()
49+
)
50+
}
51+
}

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

Lines changed: 59 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import advanced_security.javascript.frameworks.ui5.UI5
44
import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow
55
private import semmle.javascript.frameworks.data.internal.ApiGraphModelsExtensions as ApiGraphModelsExtensions
66
import advanced_security.javascript.frameworks.ui5.Bindings
7+
import advanced_security.javascript.frameworks.ui5.Fragment
78

89
/**
910
* Gets the immediate supertype of a given type from the extensible predicate `typeModel` provided by
@@ -695,10 +696,9 @@ class XmlView extends UI5View instanceof XmlFile {
695696
}
696697

697698
/**
698-
* TODO - consider - if this just copies all predicates - maybe this should be a subtype of XmlView
699-
* and we dont need a separate/parallel type for fragments vs views. this will become clear once
699+
* An xml fragment. It may or may not have controllers associated.
700700
*/
701-
class XmlFragment extends UI5Fragment instanceof XmlFile {
701+
class XmlFragment extends UI5View instanceof XmlFile {
702702
XmlRootElement root;
703703

704704
XmlFragment() {
@@ -711,6 +711,35 @@ class XmlFragment extends UI5Fragment instanceof XmlFile {
711711
root.hasName("FragmentDefinition")
712712
}
713713

714+
override XmlBindingPath getASource() {
715+
exists(XmlElement control, string type, string path, string property |
716+
type = result.getControlTypeName() and
717+
this = control.getFile() and
718+
ApiGraphModelsExtensions::sourceModel(getASuperType(type), path, "remote", _) and
719+
property = path.replaceAll(" ", "").regexpCapture("Member\\[([^\\]]+)\\]", 1) and
720+
result.getBindingTarget() = control.getAttribute(property)
721+
)
722+
}
723+
724+
override XmlBindingPath getAnHtmlISink() {
725+
exists(XmlElement control, string type, string path, string property |
726+
this = control.getFile() and
727+
type = result.getControlTypeName() and
728+
ApiGraphModelsExtensions::sinkModel(getASuperType(type), path, "ui5-html-injection", _) and
729+
property = path.replaceAll(" ", "").regexpCapture("Member\\[([^\\]]+)\\]", 1) and
730+
result.getBindingTarget() = control.getAttribute(property) and
731+
/* If the control is an `sap.ui.core.HTML` then the control should be missing the `sanitizeContent` attribute */
732+
(
733+
getASuperType(type) = "HTMLControl"
734+
implies
735+
(
736+
not exists(control.getAttribute("sanitizeContent")) or
737+
control.getAttribute("sanitizeContent").getValue() = "false"
738+
)
739+
)
740+
)
741+
}
742+
714743
override UI5Control getControl() {
715744
exists(XmlElement element |
716745
result.asXmlControl() = element and
@@ -729,9 +758,33 @@ class XmlFragment extends UI5Fragment instanceof XmlFile {
729758
)
730759
}
731760

732-
override XmlBindingPath getASource() { none() }
733-
734-
override XmlBindingPath getAnHtmlISink() { none() }
761+
/**
762+
* This is either known in the location from which `loadFragment` is called (in a controller's init function)
763+
* OR in the optional controller param of `Fragment.load`.
764+
* This MAY return no value, when the fragment is not associated to any controller.
765+
* When this returns a value it is guaranteed that this xml fragment is instantiated.
766+
*/
767+
override string getControllerName() {
768+
exists(CustomController controller, MethodCallNode loadFragmentCall |
769+
loadFragmentCall.getMethodName() = "loadFragment" and
770+
controller.getAThisNode().flowsTo(loadFragmentCall.getReceiver()) and
771+
controller.getName() = result
772+
)
773+
or
774+
exists(CustomController controller, FragmentLoad fragmentLoad |
775+
controller.getAThisNode().flowsTo(fragmentLoad.getControllerArgument()) and
776+
/*
777+
* extracting just the base name of the fragment (not the fully qualified)
778+
* otherwise difficult to know which part of absolute path is only for the qualified name
779+
*/
780+
781+
fragmentLoad
782+
.getNameArgument()
783+
.getStringValue()
784+
.matches("%" + this.getBaseName().replaceAll(".fragment.xml", "")) and
785+
controller.getName() = result
786+
)
787+
}
735788
}
736789

737790
private newtype TUI5Control =

‎javascript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment-load/UI5Xss.expected‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
nodes
2+
| webapp/controller/app.controller.js:10:17:10:27 | input: null |
3+
| webapp/view/TestFragment.fragment.xml:5:1:7:28 | value={/input} |
4+
| webapp/view/app.view.html:4:11:4:33 | data-content={/input} |
5+
edges
6+
| webapp/controller/app.controller.js:10:17:10:27 | input: null | webapp/view/TestFragment.fragment.xml:5:1:7:28 | value={/input} |
7+
| webapp/controller/app.controller.js:10:17:10:27 | input: null | webapp/view/app.view.html:4:11:4:33 | data-content={/input} |
8+
| webapp/controller/app.controller.js:12:26:12:45 | new JSONModel(oData) | webapp/view/app.view.html:4:11:4:33 | data-content={/input} |
9+
| webapp/view/TestFragment.fragment.xml:5:1:7:28 | value={/input} | webapp/controller/app.controller.js:10:17:10:27 | input: null |
10+
| webapp/view/TestFragment.fragment.xml:5:1:7:28 | value={/input} | webapp/controller/app.controller.js:12:26:12:45 | new JSONModel(oData) |
11+
| webapp/view/app.view.html:4:11:4:33 | data-content={/input} | webapp/controller/app.controller.js:10:17:10:27 | input: null |
12+
#select
13+
| webapp/view/app.view.html:4:11:4:33 | data-content={/input} | webapp/view/TestFragment.fragment.xml:5:1:7:28 | value={/input} | webapp/view/app.view.html:4:11:4:33 | data-content={/input} | XSS vulnerability due to $@. | webapp/view/TestFragment.fragment.xml:5:1:7:28 | value={/input} | user-provided value |

‎javascript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment-load/UI5Xss.qlref‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
UI5Xss/UI5Xss.ql

‎javascript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment-load/package-lock.json‎

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎javascript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment-load/package.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"name": "sap-ui5-xss",
3+
"version": "1.0.0",
4+
"main": "index.js"
5+
}

‎javascript/frameworks/ui5/test/queries/UI5Xss/xss-xml-fragment-load/ui5.yaml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
specVersion: '3.0'
2+
metadata:
3+
name: sap-ui5-xss
4+
type: application
5+
framework:
6+
name: SAPUI5
7+
version: "1.115.0"

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
sap.ui.define([
2+
"sap/ui/core/mvc/Controller",
3+
"sap/ui/model/json/JSONModel",
4+
"sap/ui/core/Fragment"
5+
], function (Controller, JSONModel, Fragment) {
6+
"use strict"
7+
return Controller.extend("codeql-sap-js.controller.app", {
8+
onInit: function () {
9+
var oData = {
10+
input: null
11+
};
12+
var oModel = new JSONModel(oData);
13+
this.getView().setModel(oModel);
14+
15+
Fragment.load({
16+
name: "codeql-sap-js.view.TestFragment",
17+
controller: this,
18+
id: this.getView().getId()
19+
}).then(function (oFragment) {
20+
this.getView().addDependent(oFragment);
21+
oFragment.placeAt("fragmentContainer");
22+
}.bind(this));
23+
}
24+
});
25+
})

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<!DOCTYPE html>
2+
<html>
3+
4+
<head>
5+
6+
<meta charset="utf-8">
7+
<title>SAPUI5 XSS</title>
8+
<script src="https://sdk.openui5.org/resources/sap-ui-core.js"
9+
data-sap-ui-libs="sap.m"
10+
data-sap-ui-onInit="module:codeql-sap-js/index"
11+
data-sap-ui-resourceroots='{
12+
"codeql-sap-js": "./"
13+
}'>
14+
</script>
15+
</head>
16+
17+
<body class="sapUiBody" id="content">
18+
19+
</body>
20+
21+
</html>

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
sap.ui.define([
2+
"sap/ui/core/mvc/HTMLView"
3+
], function (HTMLView) {
4+
"use strict";
5+
HTMLView.create({
6+
viewName: "codeql-sap-js.view.app"
7+
}).then(function (oView) {
8+
oView.placeAt("content");
9+
});
10+
11+
});

0 commit comments

Comments
 (0)