Skip to content

Commit 2e644cf

Browse files
jeongsoolee09jeongsoolee09
authored
Merge pull request #240 from advanced-security/jeongsoolee09/fix-ui5-fn
Support dynamically instantiated UI5 controls placed at a DOM tree
2 parents 524a912 + e868b5d commit 2e644cf

File tree

18 files changed

+494
-228
lines changed

18 files changed

+494
-228
lines changed

‎javascript/frameworks/cap/lib/advanced_security/javascript_sap_cap_all/Customizations.qll‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1-
// This file is included for use in custom CodeQL bundles (https://github.com/advanced-security/codeql-bundle).
2-
// The contents of this file will be included in the standard library `Customizations.qll`, and will therefore
3-
// be included in the out-of-the-box security queries.
4-
//
5-
// We import under alias to avoid any potential naming conflicts
1+
/**
2+
* This file is included for use in custom CodeQL bundles (https://github.com/advanced-security/codeql-bundle).
3+
* The contents of this file will be included in the standard library `Customizations.qll`, and will therefore
4+
* be included in the out-of-the-box security queries.
5+
*/
6+
7+
/* We import under alias to avoid any potential naming conflicts */
68
import advanced_security.javascript.frameworks.cap.RemoteFlowSources as CAPRemoteFlowSources

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

Lines changed: 31 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@ extensions:
1515
- ["RenderManager", "Renderer", "Member[render].Parameter[0]"]
1616
- ["Patcher", "Patcher", "Instance"]
1717
- ["Patcher", "sap/ui/core/Patcher", ""]
18-
- ["HTMLControl", "HTMLControl", "Instance"]
19-
- ["HTMLControl", "sap/ui/core/HTML", ""]
20-
- ["HTMLControl", "global", "Member[sap].Member[ui].Member[core].Member[HTML]"]
18+
- ["UI5HTMLControl", "UI5HTMLControl", "Instance"]
19+
- ["UI5HTMLControl", "sap/ui/core/HTML", ""]
20+
- ["UI5HTMLControl", "global", "Member[sap].Member[ui].Member[core].Member[HTML]"]
2121
- ["Assert", "global", "Member[sap].Member[base].Member[assert]"]
2222
- ["Assert", "global", "Member[jQuery].Member[sap].Member[assert]"]
2323
- ["SapLogger", "sap/base/Log", ""]
@@ -41,27 +41,27 @@ extensions:
4141
- ["SapUIDom", "sap/ui/dom", ""]
4242
- ["SapUIDom", "global", "Member[sap].Member[ui].Member[dom]"]
4343
# model Controls inheritance
44-
- ["UI5Control", "UI5Control", "Instance"]
45-
- ["UI5Control", "sap/m/InputBase", ""]
46-
- ["UI5Control", "sap/m/Input", ""]
47-
- ["UI5Control", "sap/ui/webc/main/Input", ""]
48-
- ["UI5Control", "sap/ui/commons/TextField", ""]
49-
- ["UI5Control", "sap/ui/commons/PasswordField", ""]
50-
- ["UI5Control", "sap/ui/commons/ValueHelpField", ""]
51-
- ["UI5Control", "sap/ui/commons/SearchField", ""]
52-
- ["UI5Control", "sap/ui/commons/ComboBox", ""]
53-
- ["UI5Control", "sap/ui/commons/TextArea", ""]
54-
- ["UI5Control", "sap/ui/webc/main/TextArea", ""]
55-
- ["UI5Control", "sap/m/ComboBoxTextField", ""]
56-
- ["UI5Control", "sap/m/MaskEnabler", ""]
57-
- ["UI5Control", "sap/m/MaskInput", ""]
58-
- ["UI5Control", "sap/m/TextArea", ""]
59-
- ["UI5Control", "sap/m/ComboBoxBase", ""]
60-
- ["UI5Control", "sap/m/MultiInput", ""]
61-
- ["UI5Control", "sap/ui/webc/main/MultiInput", ""]
62-
- ["UI5Control", "sap/m/SearchField", ""]
63-
- ["UI5Control", "sap/m/FeedInput", ""]
64-
- ["UI5Control", "sap/ui/richtexteditor/RichTextEditor", ""]
44+
- ["UI5InputControl", "UI5InputControl", "Instance"]
45+
- ["UI5InputControl", "sap/m/InputBase", ""]
46+
- ["UI5InputControl", "sap/m/Input", ""]
47+
- ["UI5InputControl", "sap/ui/webc/main/Input", ""]
48+
- ["UI5InputControl", "sap/ui/commons/TextField", ""]
49+
- ["UI5InputControl", "sap/ui/commons/PasswordField", ""]
50+
- ["UI5InputControl", "sap/ui/commons/ValueHelpField", ""]
51+
- ["UI5InputControl", "sap/ui/commons/SearchField", ""]
52+
- ["UI5InputControl", "sap/ui/commons/ComboBox", ""]
53+
- ["UI5InputControl", "sap/ui/commons/TextArea", ""]
54+
- ["UI5InputControl", "sap/ui/webc/main/TextArea", ""]
55+
- ["UI5InputControl", "sap/m/ComboBoxTextField", ""]
56+
- ["UI5InputControl", "sap/m/MaskEnabler", ""]
57+
- ["UI5InputControl", "sap/m/MaskInput", ""]
58+
- ["UI5InputControl", "sap/m/TextArea", ""]
59+
- ["UI5InputControl", "sap/m/ComboBoxBase", ""]
60+
- ["UI5InputControl", "sap/m/MultiInput", ""]
61+
- ["UI5InputControl", "sap/ui/webc/main/MultiInput", ""]
62+
- ["UI5InputControl", "sap/m/SearchField", ""]
63+
- ["UI5InputControl", "sap/m/FeedInput", ""]
64+
- ["UI5InputControl", "sap/ui/richtexteditor/RichTextEditor", ""]
6565
- ["UI5CodeEditor", "UI5CodeEditor", "Instance"]
6666
- ["UI5CodeEditor", "sap/ui/codeeditor/CodeEditor", ""]
6767
# Classes that provide Path injection APIs
@@ -75,8 +75,8 @@ extensions:
7575
pack: codeql/javascript-all
7676
extensible: "sourceModel"
7777
data:
78-
- ["UI5Control", "Member[value]", "remote"]
79-
- ["UI5Control", "Member[getValue].ReturnValue", "remote"]
78+
- ["UI5InputControl", "Member[value]", "remote"]
79+
- ["UI5InputControl", "Member[getValue].ReturnValue", "remote"]
8080
- ["UI5CodeEditor", "Member[value]", "remote"]
8181
- ["UI5CodeEditor", "Member[getCurrentValue].ReturnValue", "remote"]
8282
- ["global", "Member[jQuery].Member[sap].Member[syncHead,syncGet,syncGetText,syncPost,syncPostText].ReturnValue", "remote"]
@@ -89,9 +89,9 @@ extensions:
8989
data:
9090
# html-injection
9191
- ["global", "Member[jQuery].Member[sap].Member[globalEval].Argument[0]", "ui5-html-injection"]
92-
- ["HTMLControl", "Argument[0].Member[content]", "ui5-html-injection"]
93-
- ["HTMLControl", "Member[content]", "ui5-html-injection"]
94-
- ["HTMLControl", "Member[setContent].Argument[0]", "ui5-html-injection"]
92+
- ["UI5HTMLControl", "Argument[0].Member[content]", "ui5-html-injection"]
93+
- ["UI5HTMLControl", "Member[content]", "ui5-html-injection"]
94+
- ["UI5HTMLControl", "Member[setContent].Argument[0]", "ui5-html-injection"]
9595
- ["Patcher", "Member[unsafeHtml].Argument[0..]", "ui5-html-injection"]
9696
- ["RenderManager", "Member[write,unsafeHtml].Argument[0..]", "ui5-html-injection"]
9797
- ["RenderManager", "Member[writeAttribute,addStyle].Argument[0..1]", "ui5-html-injection"]
@@ -137,3 +137,5 @@ extensions:
137137
- ["sap/base/security/encodeURL", "", "Argument[0]", "ReturnValue", "taint"]
138138
- ["sap/base/security/encodeURLParameters", "", "Argument[0]", "ReturnValue", "taint"]
139139
- ["sap/base/security/encodeXML", "", "Argument[0]", "ReturnValue", "taint"]
140+
- ["UI5HTMLControl", "", "Argument[0].Member[content]", "ReturnValue.Member[content]", "taint"]
141+
- ["UI5HTMLControl", "Instance.Member[getContent]", "Argument[this].Member[content]", "ReturnValue", "taint"]

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

Lines changed: 57 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,54 @@
11
import javascript
22
import advanced_security.javascript.frameworks.ui5.UI5
33
import advanced_security.javascript.frameworks.ui5.UI5View
4-
private import semmle.javascript.frameworks.data.internal.ApiGraphModelsExtensions as ApiGraphModelsExtensions
4+
import semmle.javascript.security.dataflow.XssThroughDomCustomizations
5+
private import semmle.javascript.frameworks.data.internal.ApiGraphModelsExtensions
56

6-
private class DataFromRemoteControlReference extends RemoteFlowSource, MethodCallNode {
7+
private class DataFromRemoteControlReference extends RemoteFlowSource {
78
DataFromRemoteControlReference() {
89
exists(UI5Control sourceControl, string typeAlias, ControlReference controlReference |
9-
ApiGraphModelsExtensions::typeModel(typeAlias, sourceControl.getImportPath(), _) and
10-
ApiGraphModelsExtensions::sourceModel(typeAlias, _, "remote", _) and
10+
typeModel(typeAlias, sourceControl.getImportPath(), _) and
11+
sourceModel(typeAlias, _, "remote", _) and
1112
sourceControl.getAReference() = controlReference and
12-
controlReference.flowsTo(this.getReceiver()) and
13-
this.getMethodName() = "getValue"
13+
(
14+
this = controlReference.getAMemberCall("getValue") or
15+
this = controlReference.getAPropertyRead("value")
16+
)
1417
)
1518
}
1619

1720
override string getSourceType() { result = "Data from a remote control" }
1821
}
1922

23+
private class InputControlInstantiation extends ElementInstantiation {
24+
InputControlInstantiation() { typeModel("UI5InputControl", this.getImportPath(), _) }
25+
}
26+
27+
private module TrackPlaceAtCallConfigFlow = TaintTracking::Global<TrackPlaceAtCallConfig>;
28+
29+
class DataFromInstantiatedAndPlacedAtControl extends RemoteFlowSource, XssThroughDom::Source {
30+
InputControlInstantiation controlInstantiation;
31+
ControlPlaceAtCall placeAtCall;
32+
33+
DataFromInstantiatedAndPlacedAtControl() {
34+
exists(string typeAlias, ControlReference controlReference |
35+
/* Double check that the type derives a remote flow source. */
36+
typeModel(typeAlias, controlInstantiation.getImportPath(), _) and
37+
sourceModel(typeAlias, _, "remote", _) and
38+
controlInstantiation.getId() = controlReference.getId() and
39+
(
40+
this = controlReference.getAMemberCall("getValue") or
41+
this = controlReference.getAPropertyRead("value")
42+
)
43+
) and
44+
TrackPlaceAtCallConfigFlow::flow(controlInstantiation, placeAtCall)
45+
}
46+
47+
override string getSourceType() {
48+
result = "Data from an instantiated control placed in a DOM tree"
49+
}
50+
}
51+
2052
class LocalModelContentBoundBidirectionallyToSourceControl extends RemoteFlowSource {
2153
UI5BindingPath bindingPath;
2254
UI5Control controlDeclaration;
@@ -60,18 +92,22 @@ class ODataServiceModel extends UI5ExternalModel {
6092
ODataServiceModel() {
6193
exists(MethodCallNode setModelCall, CustomController controller |
6294
/*
63-
* 1. This flows from a DF node corresponding to the parent component's model to the `this.setModel` call
64-
* i.e. Aims to capture something like `this.getOwnerComponent().getModel("someModelName")` as in
65-
* `this.getView().setModel(this.getOwnerComponent().getModel("someModelName"))`
95+
* 1. This flows from a DF node corresponding to the parent component's model
96+
* to the `this.setModel` call. e.g.
97+
*
98+
* `this.getOwnerComponent().getModel("someModelName")` as in
99+
* `this.getView().setModel(this.getOwnerComponent().getModel("someModelName"))`.
66100
*/
67101

68102
modelName = this.getArgument(0).getALocalSource().asExpr().(StringLiteral).getValue() and
69103
this.getCalleeName() = "getModel" and
70104
controller.getOwnerComponentRef().flowsTo(this.(MethodCallNode).getReceiver()) and
71105
this.flowsTo(setModelCall.getArgument(0)) and
72-
setModelCall.getMethodName() = "setModel" and
73-
setModelCall.getReceiver() = controller.getAViewReference() and
74-
/* 2. The component's manifest.json declares the DataSource as being of OData type */
106+
setModelCall = controller.getAViewReference().getAMemberCall("setModel") and
107+
/*
108+
* 2. The component's `manifest.json` declares the DataSource as being of OData type.
109+
*/
110+
75111
controller.getOwnerComponent().getExternalModelDef(modelName).getDataSource() instanceof
76112
ODataDataSourceManifest
77113
)
@@ -101,21 +137,17 @@ private class RouteParameterAccess extends RemoteFlowSource instanceof PropRead
101137
override string getSourceType() { result = "RouteParameterAccess" }
102138

103139
RouteParameterAccess() {
104-
exists(
105-
ControllerHandler handler, RouteManifest routeManifest, ParameterNode handlerParameter,
106-
MethodCallNode getParameterCall
107-
|
140+
exists(ControllerHandler handler, RouteManifest routeManifest, MethodCallNode getParameterCall |
108141
handler.isAttachedToRoute(routeManifest.getName()) and
109142
this.asExpr().getEnclosingFunction() = handler.getFunction() and
110-
handlerParameter = handler.getParameter(0) and
111-
getParameterCall.getMethodName() = "getParameter" and
112-
getParameterCall.getReceiver().getALocalSource() = handlerParameter and
143+
getParameterCall = handler.getParameter(0).getAMemberCall("getParameter") and
113144
(
114-
routeManifest.matchesPathString(this.getPropertyName()) and
115-
this.getBase().getALocalSource() = getParameterCall
145+
exists(string path |
146+
this = getParameterCall.getAPropertyRead(path) and
147+
routeManifest.matchesPathString(path)
148+
)
116149
or
117-
/* TODO: Why does `routeManifest.matchesPathString` not work for propertyName?? */
118-
this.getBase().(PropRead).getBase().getALocalSource() = getParameterCall
150+
this = getParameterCall.getAPropertyRead().getAPropertyRead()
119151
)
120152
)
121153
}
@@ -125,10 +157,8 @@ private class DisplayEventHandlerParameterAccess extends RemoteFlowSource instan
125157
override string getSourceType() { result = "DisplayEventHandlerParameterAccess" }
126158

127159
DisplayEventHandlerParameterAccess() {
128-
exists(DisplayEventHandler handler, MethodCallNode getParameterCall |
129-
getParameterCall.getMethodName() = "getParameter" and
130-
this.getBase().getALocalSource() = getParameterCall and
131-
handler.getParameter(0) = getParameterCall.getReceiver().getALocalSource()
160+
exists(DisplayEventHandler handler |
161+
this = handler.getParameter(0).getAMemberCall("getParameter").getAPropertyRead()
132162
)
133163
}
134164
}

0 commit comments

Comments
 (0)