Skip to content

Commit a71ec82

Browse files
mbaludambaluda
committed
Models the flow between the getContent and setContent methods in an HTML control, taking sanitization into account
1 parent 0176dd4 commit a71ec82

3 files changed

Lines changed: 51 additions & 8 deletions

File tree

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

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,14 @@ module UI5Xss implements DataFlow::ConfigSig {
3131
node.(DataFlow::CallNode).getReceiver().asExpr().(PropAccess).getQualifiedName() = "jQuery.sap" and
3232
node.(DataFlow::CallNode).getCalleeName() =
3333
["encodeCSS", "encodeJS", "encodeURL", "encodeURLParameters", "encodeXML", "encodeHTML"]
34+
or
35+
/* Flow through `setContent/getContent` of a sanitized UI5Control */
36+
exists(UI5Control control, DataFlow::MethodCallNode content |
37+
control.asJsControl() = content.getReceiver().getALocalSource() and
38+
control.isHTMLSanitized() and
39+
content.getMethodName() = ["setContent", "getContent"] and
40+
node = [content, content.getArgument(0)]
41+
)
3442
}
3543

3644
predicate isSink(DataFlow::Node node) {
@@ -56,6 +64,19 @@ module UI5Xss implements DataFlow::ConfigSig {
5664

5765
end = h.getParameter(0)
5866
)
67+
or
68+
/* Flow from `setContent` to `getContent` of a control */
69+
exists(
70+
UI5Control control, DataFlow::MethodCallNode getContent, DataFlow::MethodCallNode setContent
71+
|
72+
control.asJsControl() = setContent.getReceiver().getALocalSource() and
73+
control.asJsControl() = getContent.getReceiver().getALocalSource() and
74+
setContent.getMethodName() = "setContent" and
75+
getContent.getMethodName() = "getContent" and
76+
start = setContent.getArgument(0) and
77+
end = getContent and
78+
not control.isHTMLSanitized()
79+
)
5980
}
6081
}
6182

@@ -93,10 +114,6 @@ private class UI5ExtHtmlISink extends DataFlow::Node {
93114
}
94115
}
95116

96-
private class HTMLControlInstantiation extends ElementInstantiation {
97-
HTMLControlInstantiation() { typeModel("UI5HTMLControl", this.getImportPath(), _) }
98-
}
99-
100117
private module TrackPlaceAtCallConfigFlow = TaintTracking::Global<TrackPlaceAtCallConfig>;
101118

102119
abstract class DynamicallySetElementValueOfHTML extends DataFlow::Node { }

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
nodes
2+
| webapp/controller/app.controller.js:10:17:10:27 | input: null |
3+
| webapp/controller/app.controller.js:16:35:16:62 | oModel. ... input') |
4+
| webapp/controller/app.controller.js:19:36:19:63 | oModel. ... input') |
5+
| webapp/controller/app.controller.js:20:35:20:58 | unsanit ... ntent() |
6+
| webapp/view/app.view.xml:5:5:7:28 | value={/input} |
7+
| webapp/view/app.view.xml:8:5:8:79 | content={/output} |
8+
edges
9+
| webapp/controller/app.controller.js:10:17:10:27 | input: null | webapp/controller/app.controller.js:16:35:16:62 | oModel. ... input') |
10+
| webapp/controller/app.controller.js:10:17:10:27 | input: null | webapp/controller/app.controller.js:19:36:19:63 | oModel. ... input') |
11+
| webapp/controller/app.controller.js:10:17:10:27 | input: null | webapp/view/app.view.xml:5:5:7:28 | value={/input} |
12+
| webapp/controller/app.controller.js:11:17:11:28 | output: null | webapp/view/app.view.xml:8:5:8:79 | content={/output} |
13+
| webapp/controller/app.controller.js:13:26:13:45 | new JSONModel(oData) | webapp/view/app.view.xml:8:5:8:79 | content={/output} |
14+
| webapp/controller/app.controller.js:19:36:19:63 | oModel. ... input') | webapp/controller/app.controller.js:20:35:20:58 | unsanit ... ntent() |
15+
| webapp/view/app.view.xml:5:5:7:28 | value={/input} | webapp/controller/app.controller.js:10:17:10:27 | input: null |
16+
| webapp/view/app.view.xml:5:5:7:28 | value={/input} | webapp/controller/app.controller.js:13:26:13:45 | new JSONModel(oData) |
17+
| webapp/view/app.view.xml:8:5:8:79 | content={/output} | webapp/controller/app.controller.js:11:17:11:28 | output: null |
18+
#select
19+
| webapp/controller/app.controller.js:16:35:16:62 | oModel. ... input') | webapp/view/app.view.xml:5:5:7:28 | value={/input} | webapp/controller/app.controller.js:16:35:16:62 | oModel. ... input') | XSS vulnerability due to $@. | webapp/view/app.view.xml:5:5:7:28 | value={/input} | user-provided value |
20+
| webapp/controller/app.controller.js:19:36:19:63 | oModel. ... input') | webapp/view/app.view.xml:5:5:7:28 | value={/input} | webapp/controller/app.controller.js:19:36:19:63 | oModel. ... input') | XSS vulnerability due to $@. | webapp/view/app.view.xml:5:5:7:28 | value={/input} | user-provided value |
21+
| webapp/controller/app.controller.js:20:35:20:58 | unsanit ... ntent() | webapp/controller/app.controller.js:20:35:20:58 | unsanit ... ntent() | webapp/controller/app.controller.js:20:35:20:58 | unsanit ... ntent() | XSS vulnerability due to $@. | webapp/controller/app.controller.js:20:35:20:58 | unsanit ... ntent() | user-provided value |
22+
| webapp/controller/app.controller.js:20:35:20:58 | unsanit ... ntent() | webapp/view/app.view.xml:5:5:7:28 | value={/input} | webapp/controller/app.controller.js:20:35:20:58 | unsanit ... ntent() | XSS vulnerability due to $@. | webapp/view/app.view.xml:5:5:7:28 | value={/input} | user-provided value |

‎javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/webapp/controller/app.controller.js‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,14 @@ sap.ui.define([
1515

1616
jQuery.sap.globalEval(oModel.getProperty('/input')); // UNSAFE: evaluating user input
1717

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
18+
var unsanitized = new HTML();
19+
unsanitized.setContent(oModel.getProperty('/input')); // UNSAFE: evaluating user input
20+
jQuery.sap.globalEval(unsanitized.getContent()); // UNSAFE: setContent->getContent flow step
21+
22+
var sanitized = new HTML();
23+
sanitized.setSanitizeContent(true);
24+
sanitized.setContent(oModel.getProperty('/input')); // SAFE: content is sanitized before eval
25+
jQuery.sap.globalEval(sanitized.getContent()); // SAFE: content is sanitized before eval
2226

2327
let htmlSanitized = this.getView().byId("htmlSanitized");
2428
jQuery.sap.globalEval(htmlSanitized.getContent()); // SAFE: content is sanitized declaratively in the view

0 commit comments

Comments
 (0)