Skip to content

Commit 5023760

Browse files
committed
- Adds a test case for sanitizers
- Add controls as JsControl
1 parent 2ccc53d commit 5023760

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", ""]
@@ -108,6 +111,7 @@ extensions:
108111
data:
109112
- ["UI5InputControl", "Member[value]", "remote"]
110113
- ["UI5InputControl", "Member[getValue].ReturnValue", "remote"]
114+
- ["UI5HTMLControl", "Member[getContent].ReturnValue", "remote"]
111115
- ["UI5CodeEditor", "Member[value]", "remote"]
112116
- ["UI5CodeEditor", "Member[getCurrentValue].ReturnValue", "remote"]
113117
- ["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
@@ -692,6 +692,8 @@ private newtype TUI5Control =
692692
.(ArrayLiteralNode)
693693
.asExpr()
694694
)
695+
or
696+
control = ModelOutput::getATypeNode("Control").getAnInvocation()
695697
}
696698

697699
class UI5Control extends TUI5Control {
@@ -739,7 +741,9 @@ class UI5Control extends TUI5Control {
739741
)
740742
or
741743
exists(NewNode control | control = this.asJsControl() |
742-
result = this.asJsControl().asExpr().getAChildExpr().(DotExpr).getQualifiedName()
744+
result = control.asExpr().getAChildExpr().(PropAccess).getQualifiedName()
745+
or
746+
control = API::moduleImport(result).getAnInvocation()
743747
)
744748
}
745749

@@ -863,11 +867,12 @@ class UI5Control extends TUI5Control {
863867
)
864868
or
865869
/* 3. `sanitizeContent` attribute is set programmatically using a setter. */
866-
exists(CallNode node |
867-
node =
868-
this.getAReference()
869-
.getAMemberCall("set" + propName.prefix(1).toUpperCase() + propName.suffix(1)) and
870+
exists(CallNode node, string setterName |
871+
setterName = "set" + propName.prefix(1).toUpperCase() + propName.suffix(1) and
870872
not node.getArgument(0).mayHaveBooleanValue(val.booleanNot())
873+
|
874+
node = this.getAReference().getAMemberCall(setterName) or
875+
node = this.asJsControl().getAMemberCall(setterName)
871876
)
872877
}
873878
}

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)