Skip to content

Commit 863ea7b

Browse files
mbaludambaluda
committed
Address review 2
1 parent e2bf1b3 commit 863ea7b

File tree

4 files changed

+71
-34
lines changed

4 files changed

+71
-34
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -861,6 +861,12 @@ class UI5Control extends TUI5Control {
861861
node.getArgument(0).getStringValue() = propName and
862862
not node.getArgument(1).mayHaveBooleanValue(val.booleanNot())
863863
)
864+
or
865+
/* 3. `sanitizeContent` attribute is set programmatically using a setter. */
866+
exists(CallNode node |
867+
node = this.getAReference().getAMemberCall("setS" + propName.suffix(1)) and
868+
not node.getArgument(0).mayHaveBooleanValue(val.booleanNot())
869+
)
864870
}
865871
}
866872

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

Lines changed: 43 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ nodes
44
| webapp/controller/app.controller.js:11:17:11:28 | input2: null |
55
| webapp/controller/app.controller.js:12:17:12:28 | input3: null |
66
| webapp/controller/app.controller.js:13:17:13:28 | input4: null |
7+
| webapp/controller/app.controller.js:14:17:14:28 | input5: null |
8+
| webapp/controller/app.controller.js:15:17:15:28 | input6: null |
79
| webapp/view/app.view.xml:6:5:8:28 | value={/input} |
810
| webapp/view/app.view.xml:9:5:10:28 | value={/input} |
911
| webapp/view/app.view.xml:11:5:12:28 | value={/input} |
@@ -23,7 +25,11 @@ nodes
2325
| webapp/view/app.view.xml:42:5:44:29 | value={/input3} |
2426
| webapp/view/app.view.xml:45:5:45:59 | content={/input3} |
2527
| webapp/view/app.view.xml:47:5:49:29 | value={/input4} |
26-
| webapp/view/app.view.xml:50:5:50:80 | content={/input4} |
28+
| webapp/view/app.view.xml:50:5:50:81 | content={/input4} |
29+
| webapp/view/app.view.xml:52:5:54:29 | value={/input5} |
30+
| webapp/view/app.view.xml:55:5:55:59 | content={/input5} |
31+
| webapp/view/app.view.xml:57:5:57:43 | value={/input} |
32+
| webapp/view/app.view.xml:58:5:58:65 | value={/input6} |
2733
edges
2834
| webapp/controller/app.controller.js:9:17:9:28 | input0: null | webapp/view/app.view.xml:27:5:29:29 | value={/input0} |
2935
| webapp/controller/app.controller.js:9:17:9:28 | input0: null | webapp/view/app.view.xml:30:5:30:37 | content={/input0} |
@@ -34,38 +40,51 @@ edges
3440
| webapp/controller/app.controller.js:12:17:12:28 | input3: null | webapp/view/app.view.xml:42:5:44:29 | value={/input3} |
3541
| webapp/controller/app.controller.js:12:17:12:28 | input3: null | webapp/view/app.view.xml:45:5:45:59 | content={/input3} |
3642
| webapp/controller/app.controller.js:13:17:13:28 | input4: null | webapp/view/app.view.xml:47:5:49:29 | value={/input4} |
37-
| webapp/controller/app.controller.js:13:17:13:28 | input4: null | webapp/view/app.view.xml:50:5:50:80 | content={/input4} |
38-
| webapp/controller/app.controller.js:15:26:15:45 | new JSONModel(oData) | webapp/view/app.view.xml:25:5:25:36 | content={/input} |
39-
| webapp/controller/app.controller.js:15:26:15:45 | new JSONModel(oData) | webapp/view/app.view.xml:30:5:30:37 | content={/input0} |
40-
| webapp/controller/app.controller.js:15:26:15:45 | new JSONModel(oData) | webapp/view/app.view.xml:35:5:35:79 | content={/input1} |
41-
| webapp/controller/app.controller.js:15:26:15:45 | new JSONModel(oData) | webapp/view/app.view.xml:40:5:40:59 | content={/input2} |
42-
| webapp/controller/app.controller.js:15:26:15:45 | new JSONModel(oData) | webapp/view/app.view.xml:45:5:45:59 | content={/input3} |
43-
| webapp/controller/app.controller.js:15:26:15:45 | new JSONModel(oData) | webapp/view/app.view.xml:50:5:50:80 | content={/input4} |
44-
| webapp/view/app.view.xml:6:5:8:28 | value={/input} | webapp/controller/app.controller.js:15:26:15:45 | new JSONModel(oData) |
45-
| webapp/view/app.view.xml:9:5:10:28 | value={/input} | webapp/controller/app.controller.js:15:26:15:45 | new JSONModel(oData) |
46-
| webapp/view/app.view.xml:11:5:12:28 | value={/input} | webapp/controller/app.controller.js:15:26:15:45 | new JSONModel(oData) |
47-
| webapp/view/app.view.xml:13:5:14:28 | value={/input} | webapp/controller/app.controller.js:15:26:15:45 | new JSONModel(oData) |
48-
| webapp/view/app.view.xml:15:5:16:28 | value={/input} | webapp/controller/app.controller.js:15:26:15:45 | new JSONModel(oData) |
49-
| webapp/view/app.view.xml:17:5:18:28 | value={/input} | webapp/controller/app.controller.js:15:26:15:45 | new JSONModel(oData) |
50-
| webapp/view/app.view.xml:19:5:20:28 | value={/input} | webapp/controller/app.controller.js:15:26:15:45 | new JSONModel(oData) |
51-
| webapp/view/app.view.xml:21:5:22:28 | value={/input} | webapp/controller/app.controller.js:15:26:15:45 | new JSONModel(oData) |
52-
| webapp/view/app.view.xml:23:5:24:28 | value={/input} | webapp/controller/app.controller.js:15:26:15:45 | new JSONModel(oData) |
43+
| webapp/controller/app.controller.js:13:17:13:28 | input4: null | webapp/view/app.view.xml:50:5:50:81 | content={/input4} |
44+
| webapp/controller/app.controller.js:14:17:14:28 | input5: null | webapp/view/app.view.xml:52:5:54:29 | value={/input5} |
45+
| webapp/controller/app.controller.js:14:17:14:28 | input5: null | webapp/view/app.view.xml:55:5:55:59 | content={/input5} |
46+
| webapp/controller/app.controller.js:15:17:15:28 | input6: null | webapp/view/app.view.xml:58:5:58:65 | value={/input6} |
47+
| webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) | webapp/view/app.view.xml:25:5:25:36 | content={/input} |
48+
| webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) | webapp/view/app.view.xml:30:5:30:37 | content={/input0} |
49+
| webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) | webapp/view/app.view.xml:35:5:35:79 | content={/input1} |
50+
| webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) | webapp/view/app.view.xml:40:5:40:59 | content={/input2} |
51+
| webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) | webapp/view/app.view.xml:45:5:45:59 | content={/input3} |
52+
| webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) | webapp/view/app.view.xml:50:5:50:81 | content={/input4} |
53+
| webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) | webapp/view/app.view.xml:55:5:55:59 | content={/input5} |
54+
| webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) | webapp/view/app.view.xml:57:5:57:43 | value={/input} |
55+
| webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) | webapp/view/app.view.xml:58:5:58:65 | value={/input6} |
56+
| webapp/view/app.view.xml:6:5:8:28 | value={/input} | webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) |
57+
| webapp/view/app.view.xml:9:5:10:28 | value={/input} | webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) |
58+
| webapp/view/app.view.xml:11:5:12:28 | value={/input} | webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) |
59+
| webapp/view/app.view.xml:13:5:14:28 | value={/input} | webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) |
60+
| webapp/view/app.view.xml:15:5:16:28 | value={/input} | webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) |
61+
| webapp/view/app.view.xml:17:5:18:28 | value={/input} | webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) |
62+
| webapp/view/app.view.xml:19:5:20:28 | value={/input} | webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) |
63+
| webapp/view/app.view.xml:21:5:22:28 | value={/input} | webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) |
64+
| webapp/view/app.view.xml:23:5:24:28 | value={/input} | webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) |
5365
| webapp/view/app.view.xml:27:5:29:29 | value={/input0} | webapp/controller/app.controller.js:9:17:9:28 | input0: null |
54-
| webapp/view/app.view.xml:27:5:29:29 | value={/input0} | webapp/controller/app.controller.js:15:26:15:45 | new JSONModel(oData) |
66+
| webapp/view/app.view.xml:27:5:29:29 | value={/input0} | webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) |
5567
| webapp/view/app.view.xml:30:5:30:37 | content={/input0} | webapp/controller/app.controller.js:9:17:9:28 | input0: null |
5668
| webapp/view/app.view.xml:32:5:34:29 | value={/input1} | webapp/controller/app.controller.js:10:17:10:28 | input1: null |
57-
| webapp/view/app.view.xml:32:5:34:29 | value={/input1} | webapp/controller/app.controller.js:15:26:15:45 | new JSONModel(oData) |
69+
| webapp/view/app.view.xml:32:5:34:29 | value={/input1} | webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) |
5870
| webapp/view/app.view.xml:35:5:35:79 | content={/input1} | webapp/controller/app.controller.js:10:17:10:28 | input1: null |
5971
| webapp/view/app.view.xml:37:5:39:29 | value={/input2} | webapp/controller/app.controller.js:11:17:11:28 | input2: null |
60-
| webapp/view/app.view.xml:37:5:39:29 | value={/input2} | webapp/controller/app.controller.js:15:26:15:45 | new JSONModel(oData) |
72+
| webapp/view/app.view.xml:37:5:39:29 | value={/input2} | webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) |
6173
| webapp/view/app.view.xml:40:5:40:59 | content={/input2} | webapp/controller/app.controller.js:11:17:11:28 | input2: null |
6274
| webapp/view/app.view.xml:42:5:44:29 | value={/input3} | webapp/controller/app.controller.js:12:17:12:28 | input3: null |
63-
| webapp/view/app.view.xml:42:5:44:29 | value={/input3} | webapp/controller/app.controller.js:15:26:15:45 | new JSONModel(oData) |
75+
| webapp/view/app.view.xml:42:5:44:29 | value={/input3} | webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) |
6476
| webapp/view/app.view.xml:45:5:45:59 | content={/input3} | webapp/controller/app.controller.js:12:17:12:28 | input3: null |
6577
| webapp/view/app.view.xml:47:5:49:29 | value={/input4} | webapp/controller/app.controller.js:13:17:13:28 | input4: null |
66-
| webapp/view/app.view.xml:47:5:49:29 | value={/input4} | webapp/controller/app.controller.js:15:26:15:45 | new JSONModel(oData) |
67-
| webapp/view/app.view.xml:50:5:50:80 | content={/input4} | webapp/controller/app.controller.js:13:17:13:28 | input4: null |
78+
| webapp/view/app.view.xml:47:5:49:29 | value={/input4} | webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) |
79+
| webapp/view/app.view.xml:50:5:50:81 | content={/input4} | webapp/controller/app.controller.js:13:17:13:28 | input4: null |
80+
| webapp/view/app.view.xml:52:5:54:29 | value={/input5} | webapp/controller/app.controller.js:14:17:14:28 | input5: null |
81+
| webapp/view/app.view.xml:52:5:54:29 | value={/input5} | webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) |
82+
| webapp/view/app.view.xml:55:5:55:59 | content={/input5} | webapp/controller/app.controller.js:14:17:14:28 | input5: null |
83+
| webapp/view/app.view.xml:57:5:57:43 | value={/input} | webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) |
84+
| webapp/view/app.view.xml:58:5:58:65 | value={/input6} | webapp/controller/app.controller.js:15:17:15:28 | input6: null |
85+
| webapp/view/app.view.xml:58:5:58:65 | value={/input6} | webapp/controller/app.controller.js:17:26:17:45 | new JSONModel(oData) |
6886
#select
6987
| webapp/view/app.view.xml:30:5:30:37 | content={/input0} | webapp/view/app.view.xml:27:5:29:29 | value={/input0} | webapp/view/app.view.xml:30:5:30:37 | content={/input0} | XSS vulnerability due to $@. | webapp/view/app.view.xml:27:5:29:29 | value={/input0} | user-provided value |
7088
| webapp/view/app.view.xml:45:5:45:59 | content={/input3} | webapp/view/app.view.xml:42:5:44:29 | value={/input3} | webapp/view/app.view.xml:45:5:45:59 | content={/input3} | XSS vulnerability due to $@. | webapp/view/app.view.xml:42:5:44:29 | value={/input3} | user-provided value |
71-
| webapp/view/app.view.xml:50:5:50:80 | content={/input4} | webapp/view/app.view.xml:47:5:49:29 | value={/input4} | webapp/view/app.view.xml:50:5:50:80 | content={/input4} | XSS vulnerability due to $@. | webapp/view/app.view.xml:47:5:49:29 | value={/input4} | user-provided value |
89+
| webapp/view/app.view.xml:50:5:50:81 | content={/input4} | webapp/view/app.view.xml:47:5:49:29 | value={/input4} | webapp/view/app.view.xml:50:5:50:81 | content={/input4} | XSS vulnerability due to $@. | webapp/view/app.view.xml:47:5:49:29 | value={/input4} | user-provided value |
90+
| webapp/view/app.view.xml:58:5:58:65 | value={/input6} | webapp/view/app.view.xml:58:5:58:65 | value={/input6} | webapp/view/app.view.xml:58:5:58:65 | value={/input6} | XSS vulnerability due to $@. | webapp/view/app.view.xml:58:5:58:65 | value={/input6} | user-provided value |

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

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,23 @@ sap.ui.define([
1010
input1: null,
1111
input2: null,
1212
input3: null,
13-
input4: null
13+
input4: null,
14+
input5: null,
15+
input6: null
1416
};
1517
var oModel = new JSONModel(oData);
1618
this.getView().setModel(oModel);
17-
19+
1820
// enable sanitization programmatically
19-
this.getView().byId("htmlJsSanitized2").setProperty("sanitizeContent", true);
21+
this.getView().byId("htmlJsSanitized2").setProperty("sanitizeContent", true); /* SAFE */
22+
23+
this.getView().byId("htmlJsSanitized3").sanitizeContent = true; /* UNSAFE: setting the property directly is not sufficient */
24+
25+
this.getView().byId("htmlUnsanitized").setProperty("sanitizeContent", false); /* UNSAFE: setProperty(..., false) */
2026

21-
// setting the property directly is not sufficient
22-
this.getView().byId("htmlJsSanitized3").sanitizeContent = true;
27+
this.getView().byId("htmlJsSanitized5").setSanitizeContent(true); /* SAFE: setSanitizeContent(true) */
2328

24-
// disable sanitization programmatically
25-
this.getView().byId("htmUnsanitized").setProperty("sanitizeContent", false);
29+
this.getView().byId("rteUnsanitized").setSanitizeValue(false); /* UNSAFE: RTE setSanitizeContent(false) */
2630
}
2731
});
2832
})

‎javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control/webapp/view/app.view.xml‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
<mvc:View controllerName="codeql-sap-js.controller.app"
22
xmlns="sap.m"
33
xmlns:core="sap.ui.core"
4-
xmlns:mvc="sap.ui.core.mvc">
5-
4+
xmlns:mvc="sap.ui.core.mvc"
5+
xmlns:rte="sap.ui.richtexteditor">
66
<Input placeholder="Enter Payload"
77
description="Try: &lt;img src=x onerror=alert(&quot;XSS&quot;)&gt;"
88
value="{/input}" /> <!--User input source sap.m.Input.value -->
@@ -47,5 +47,13 @@
4747
<Input placeholder="Enter Payload"
4848
description="Try: &lt;img src=x onerror=alert(&quot;XSS&quot;)&gt;"
4949
value="{/input4}" /> <!--User input source sap.m.Input.value -->
50-
<core:HTML id="htmUnsanitized" content="{/input4}" sanitizeContent="true"/> <!--XSS sink sap.ui.core.HTML.content -->
50+
<core:HTML id="htmlUnsanitized" content="{/input4}" sanitizeContent="true"/> <!--XSS sink sap.ui.core.HTML.content -->
51+
52+
<Input placeholder="Enter Payload (will be sanitized with setSanitizeContent)"
53+
description="Try: &lt;img src=x onerror=alert(&quot;XSS&quot;)&gt;"
54+
value="{/input5}" /> <!--User input source sap.m.Input.value -->
55+
<core:HTML id="htmlJsSanitized5" content="{/input5}"/> <!--sanitized XSS sink sap.ui.core.HTML.content -->
56+
57+
<rte:RichTextEditor value="{/input}"/> <!-- sanitized by default sink sap.ui.richtexteditor.RichTextEditor.value -->
58+
<rte:RichTextEditor id="rteUnsanitized" value="{/input6}"/> <!-- XSS unsinitized programmatically sap.ui.richtexteditor.RichTextEditor.value -->
5159
</mvc:View>

0 commit comments

Comments
 (0)