Skip to content

Commit 65a8a76

Browse files
committed
Additional flow steps do not cross application boundaries
Adds a test comprising of 2 separate UI5 apps
1 parent 549a32e commit 65a8a76

File tree

26 files changed

+188
-181
lines changed

26 files changed

+188
-181
lines changed

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,15 @@ module UI5LogEntryToHttp implements DataFlow::StateConfigSig {
2626
predicate isAdditionalFlowStep(
2727
DataFlow::Node start, FlowState preState, DataFlow::Node end, FlowState postState
2828
) {
29-
UI5LogInjection::isAdditionalFlowStep(start, end) and
30-
preState = postState
31-
or
32-
logArgumentToListener(start, end) and
33-
preState = "not-logged-not-accessed" and
34-
postState = "logged-and-accessed"
29+
inSameWebApp(start.getFile(), end.getFile()) and
30+
(
31+
UI5LogInjection::isAdditionalFlowStep(start, end) and
32+
preState = postState
33+
or
34+
logArgumentToListener(start, end) and
35+
preState = "not-logged-not-accessed" and
36+
postState = "logged-and-accessed"
37+
)
3538
}
3639

3740
predicate isSink(DataFlow::Node node, FlowState state) {

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

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -49,33 +49,36 @@ module UI5Xss implements DataFlow::ConfigSig {
4949
}
5050

5151
predicate isAdditionalFlowStep(DataFlow::Node start, DataFlow::Node end) {
52-
/* Already an additional flow step defined in `DomBasedXssQuery::Configuration` */
53-
DomBasedXss::DomBasedXssConfig::isAdditionalFlowStep(start, _, end, _)
54-
or
55-
/* TODO: Legacy code */
56-
/* Handler argument node to handler parameter */
57-
exists(UI5Handler h |
58-
start = h.getBindingPath().getNode() and
59-
/*
60-
* Ideally we would like to show an intermediate node where
61-
* the handler is bound to a control, but there is no sourceNode there
62-
* `end = h.getBindingPath() or start = h.getBindingPath()`
63-
*/
52+
inSameWebApp(start.getFile(), end.getFile()) and
53+
(
54+
/* Already an additional flow step defined in `DomBasedXssQuery::Configuration` */
55+
DomBasedXss::DomBasedXssConfig::isAdditionalFlowStep(start, _, end, _)
56+
or
57+
/* TODO: Legacy code */
58+
/* Handler argument node to handler parameter */
59+
exists(UI5Handler h |
60+
start = h.getBindingPath().getNode() and
61+
/*
62+
* Ideally we would like to show an intermediate node where
63+
* the handler is bound to a control, but there is no sourceNode there
64+
* `end = h.getBindingPath() or start = h.getBindingPath()`
65+
*/
6466

65-
end = h.getParameter(0)
66-
)
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()
67+
end = h.getParameter(0)
68+
)
69+
or
70+
/* Flow from `setContent` to `getContent` of a control */
71+
exists(
72+
UI5Control control, DataFlow::MethodCallNode getContent, DataFlow::MethodCallNode setContent
73+
|
74+
control.asJsControl() = setContent.getReceiver().getALocalSource() and
75+
control.asJsControl() = getContent.getReceiver().getALocalSource() and
76+
setContent.getMethodName() = "setContent" and
77+
getContent.getMethodName() = "getContent" and
78+
start = setContent.getArgument(0) and
79+
end = getContent and
80+
not control.isHTMLSanitized()
81+
)
7982
)
8083
}
8184
}

javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/FlowSteps.qll

Lines changed: 129 additions & 108 deletions
Large diffs are not rendered by default.

javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/UI5DataFlow.qll

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -263,24 +263,27 @@ module TrackPlaceAtCallConfig implements DataFlow::ConfigSig {
263263
* - [ ] Heuristic by prefix: not an attractive option since heuristics can fail
264264
*/
265265
predicate isAdditionalFlowStep(DataFlow::Node start, DataFlow::Node end) {
266-
exists(DataFlow::PropWrite propWrite |
267-
start = propWrite.getRhs() and
268-
end = propWrite.getBase()
269-
)
270-
or
271-
exists(DataFlow::MethodCallNode maybeAddingChildAPICall |
272-
start = maybeAddingChildAPICall.getAnArgument() and
273-
end = maybeAddingChildAPICall.getReceiver()
274-
)
275-
or
276-
exists(DataFlow::MethodCallNode call |
277-
start = call.getReceiver() and
278-
end = call
279-
)
280-
or
281-
exists(DataFlow::NewNode new |
282-
start = new.getAnArgument() and
283-
end = new
266+
inSameWebApp(start.getFile(), end.getFile()) and
267+
(
268+
exists(DataFlow::PropWrite propWrite |
269+
start = propWrite.getRhs() and
270+
end = propWrite.getBase()
271+
)
272+
or
273+
exists(DataFlow::MethodCallNode maybeAddingChildAPICall |
274+
start = maybeAddingChildAPICall.getAnArgument() and
275+
end = maybeAddingChildAPICall.getReceiver()
276+
)
277+
or
278+
exists(DataFlow::MethodCallNode call |
279+
start = call.getReceiver() and
280+
end = call
281+
)
282+
or
283+
exists(DataFlow::NewNode new |
284+
start = new.getAnArgument() and
285+
end = new
286+
)
284287
)
285288
}
286289
}

javascript/frameworks/ui5/test/queries/UI5Xss/xss-custom-control-api3/UI5Xss.expected

Lines changed: 0 additions & 22 deletions
This file was deleted.

javascript/frameworks/ui5/test/queries/UI5Xss/xss-custom-control-api3/UI5Xss.qlref

Lines changed: 0 additions & 1 deletion
This file was deleted.

javascript/frameworks/ui5/test/queries/UI5Xss/xss-custom-control-api2/UI5Xss.expected renamed to javascript/frameworks/ui5/test/queries/UI5Xss/xss-custom-control-apis/UI5Xss.expected

File renamed without changes.

javascript/frameworks/ui5/test/queries/UI5Xss/xss-custom-control-api2/UI5Xss.qlref renamed to javascript/frameworks/ui5/test/queries/UI5Xss/xss-custom-control-apis/UI5Xss.qlref

File renamed without changes.

javascript/frameworks/ui5/test/queries/UI5Xss/xss-custom-control-api2/package-lock.json renamed to javascript/frameworks/ui5/test/queries/UI5Xss/xss-custom-control-apis/xss-custom-control-api3/package-lock.json

File renamed without changes.

javascript/frameworks/ui5/test/queries/UI5Xss/xss-custom-control-api2/package.json renamed to javascript/frameworks/ui5/test/queries/UI5Xss/xss-custom-control-apis/xss-custom-control-api3/package.json

File renamed without changes.

0 commit comments

Comments
 (0)