Skip to content

Commit 98a9458

Browse files
committed
- Adds a test case for sanitizers
- Add controls as JsControl
1 parent 75557a1 commit 98a9458

12 files changed

Lines changed: 113 additions & 5 deletions

File tree

javascript/frameworks/ui5/ext/ui5.model.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ extensions:
66
- ["SapUICoreInstance", "global", "Member[sap].Member[ui].Member[getCore].ReturnValue"]
77
- ["Control", "Control", "Instance"]
88
- ["Control", "sap/ui/core/Control", ""]
9+
- ["Control", "UI5HTMLControl", ""]
10+
- ["Control", "UI5InputControl", ""]
11+
- ["Control", "CustomControl", ""]
912
- ["Control", "global", "Member[sap].Member[ui].Member[core].Member[Control]"]
1013
- ["Controller", "Controller", "Instance"]
1114
- ["Controller", "sap/ui/core/mvc/Controller", ""]
@@ -112,6 +115,7 @@ extensions:
112115
data:
113116
- ["UI5InputControl", "Member[value]", "remote"]
114117
- ["UI5InputControl", "Member[getValue].ReturnValue", "remote"]
118+
- ["UI5HTMLControl", "Member[getContent].ReturnValue", "remote"]
115119
- ["UI5CodeEditor", "Member[value]", "remote"]
116120
- ["UI5CodeEditor", "Member[getCurrentValue].ReturnValue", "remote"]
117121
- ["global", "Member[jQuery].Member[sap].Member[syncHead,syncGet,syncGetText,syncPost,syncPostText].ReturnValue", "remote"]

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -800,6 +800,8 @@ private newtype TUI5Control =
800800
.(ArrayLiteralNode)
801801
.asExpr()
802802
)
803+
or
804+
control = ModelOutput::getATypeNode("Control").getAnInvocation()
803805
}
804806

805807
class UI5Control extends TUI5Control {
@@ -847,7 +849,9 @@ class UI5Control extends TUI5Control {
847849
)
848850
or
849851
exists(NewNode control | control = this.asJsControl() |
850-
result = this.asJsControl().asExpr().getAChildExpr().(DotExpr).getQualifiedName()
852+
result = control.asExpr().getAChildExpr().(PropAccess).getQualifiedName()
853+
or
854+
control = API::moduleImport(result).getAnInvocation()
851855
)
852856
}
853857

@@ -983,11 +987,12 @@ class UI5Control extends TUI5Control {
983987
)
984988
or
985989
/* 3. `sanitizeContent` attribute is set programmatically using a setter. */
986-
exists(CallNode node |
987-
node =
988-
this.getAReference()
989-
.getAMemberCall("set" + propName.prefix(1).toUpperCase() + propName.suffix(1)) and
990+
exists(CallNode node, string setterName |
991+
setterName = "set" + propName.prefix(1).toUpperCase() + propName.suffix(1) and
990992
not node.getArgument(0).mayHaveBooleanValue(val.booleanNot())
993+
|
994+
node = this.getAReference().getAMemberCall(setterName) or
995+
node = this.asJsControl().getAMemberCall(setterName)
991996
)
992997
}
993998
}

javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/UI5Xss.expected

Whitespace-only changes.
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-html-control-sanitized/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.
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+
}
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"
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
sap.ui.define([
2+
"sap/ui/core/mvc/Controller",
3+
"sap/ui/model/json/JSONModel",
4+
"sap/ui/core/HTML"
5+
], function (Controller, JSONModel, HTML) {
6+
"use strict";
7+
return Controller.extend("codeql-sap-js.controller.app", {
8+
onInit: function () {
9+
var oData = {
10+
input: null,
11+
output: null,
12+
};
13+
var oModel = new JSONModel(oData);
14+
this.getView().setModel(oModel);
15+
16+
jQuery.sap.globalEval(oModel.getProperty('/input')); // UNSAFE: evaluating user input
17+
18+
var sanitizer = new HTML();
19+
sanitizer.setSanitizeContent(true);
20+
sanitizer.setContent(oModel.getProperty('/input')); // SAFE: content is sanitized before eval
21+
jQuery.sap.globalEval(sanitizer.getContent()); // SAFE: content is sanitized before eval
22+
23+
let htmlSanitized = this.getView().byId("htmlSanitized");
24+
jQuery.sap.globalEval(htmlSanitized.getContent()); // SAFE: content is sanitized declaratively in the view
25+
}
26+
});
27+
}
28+
);
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>
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/XMLView"
3+
], function (XMLView) {
4+
"use strict";
5+
XMLView.create({
6+
viewName: "codeql-sap-js.view.app"
7+
}).then(function (oView) {
8+
oView.placeAt("content");
9+
});
10+
11+
});

0 commit comments

Comments
 (0)