Skip to content

Commit b84195a

Browse files
jeongsoolee09jeongsoolee09
committed
Support instantiated controls via NewNode
A TODO is to model `placeAt` and the DOM hierarchy between instantiated controls.
1 parent dcfc88b commit b84195a

File tree

6 files changed

+218
-93
lines changed

6 files changed

+218
-93
lines changed

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

Lines changed: 25 additions & 23 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,21 +41,21 @@ 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/m/ComboBoxTextField", ""]
50-
- ["UI5Control", "sap/m/MaskEnabler", ""]
51-
- ["UI5Control", "sap/m/MaskInput", ""]
52-
- ["UI5Control", "sap/m/TextArea", ""]
53-
- ["UI5Control", "sap/m/ComboBoxBase", ""]
54-
- ["UI5Control", "sap/m/MultiInput", ""]
55-
- ["UI5Control", "sap/ui/webc/main/MultiInput", ""]
56-
- ["UI5Control", "sap/m/SearchField", ""]
57-
- ["UI5Control", "sap/m/FeedInput", ""]
58-
- ["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/m/ComboBoxTextField", ""]
50+
- ["UI5InputControl", "sap/m/MaskEnabler", ""]
51+
- ["UI5InputControl", "sap/m/MaskInput", ""]
52+
- ["UI5InputControl", "sap/m/TextArea", ""]
53+
- ["UI5InputControl", "sap/m/ComboBoxBase", ""]
54+
- ["UI5InputControl", "sap/m/MultiInput", ""]
55+
- ["UI5InputControl", "sap/ui/webc/main/MultiInput", ""]
56+
- ["UI5InputControl", "sap/m/SearchField", ""]
57+
- ["UI5InputControl", "sap/m/FeedInput", ""]
58+
- ["UI5InputControl", "sap/ui/richtexteditor/RichTextEditor", ""]
5959
- ["UI5CodeEditor", "UI5CodeEditor", "Instance"]
6060
- ["UI5CodeEditor", "sap/ui/codeeditor/CodeEditor", ""]
6161
# Classes that provide Path injection APIs
@@ -69,8 +69,8 @@ extensions:
6969
pack: codeql/javascript-all
7070
extensible: "sourceModel"
7171
data:
72-
- ["UI5Control", "Member[value]", "remote"]
73-
- ["UI5Control", "Member[getValue].ReturnValue", "remote"]
72+
- ["UI5InputControl", "Member[value]", "remote"]
73+
- ["UI5InputControl", "Member[getValue].ReturnValue", "remote"]
7474
- ["UI5CodeEditor", "Member[value]", "remote"]
7575
- ["UI5CodeEditor", "Member[getCurrentValue].ReturnValue", "remote"]
7676
- ["global", "Member[jQuery].Member[sap].Member[syncHead,syncGet,syncGetText,syncPost,syncPostText].ReturnValue", "remote"]
@@ -83,9 +83,9 @@ extensions:
8383
data:
8484
# html-injection
8585
- ["global", "Member[jQuery].Member[sap].Member[globalEval].Argument[0]", "ui5-html-injection"]
86-
- ["HTMLControl", "Argument[0].Member[content]", "ui5-html-injection"]
87-
- ["HTMLControl", "Member[content]", "ui5-html-injection"]
88-
- ["HTMLControl", "Member[setContent].Argument[0]", "ui5-html-injection"]
86+
- ["UI5HTMLControl", "Argument[0].Member[content]", "ui5-html-injection"]
87+
- ["UI5HTMLControl", "Member[content]", "ui5-html-injection"]
88+
- ["UI5HTMLControl", "Member[setContent].Argument[0]", "ui5-html-injection"]
8989
- ["Patcher", "Member[unsafeHtml].Argument[0..]", "ui5-html-injection"]
9090
- ["RenderManager", "Member[write,unsafeHtml].Argument[0..]", "ui5-html-injection"]
9191
- ["RenderManager", "Member[writeAttribute,addStyle].Argument[0..1]", "ui5-html-injection"]
@@ -131,3 +131,5 @@ extensions:
131131
- ["sap/base/security/encodeURL", "", "Argument[0]", "ReturnValue", "taint"]
132132
- ["sap/base/security/encodeURLParameters", "", "Argument[0]", "ReturnValue", "taint"]
133133
- ["sap/base/security/encodeXML", "", "Argument[0]", "ReturnValue", "taint"]
134+
- ["UI5HTMLControl", "", "Argument[0].Member[content]", "ReturnValue.Member[content]", "taint"]
135+
- ["UI5HTMLControl", "Instance.Member[getContent]", "Argument[this].Member[content]", "ReturnValue", "taint"]

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

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,52 @@
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 class DataFromInstantiatedAndPlacedAtControl extends RemoteFlowSource, XssThroughDom::Source
28+
{
29+
DataFromInstantiatedAndPlacedAtControl() {
30+
exists(
31+
InputControlInstantiation controlInstantiation, string typeAlias,
32+
ControlReference controlReference
33+
|
34+
/* Double check that the type derives a remote flow source. */
35+
typeModel(typeAlias, controlInstantiation.getImportPath(), _) and
36+
sourceModel(typeAlias, _, "remote", _) and
37+
controlInstantiation.getId() = controlReference.getId() and
38+
(
39+
this = controlReference.getAMemberCall("getValue") or
40+
this = controlReference.getAPropertyRead("value")
41+
)
42+
)
43+
}
44+
45+
override string getSourceType() {
46+
result = "Data from an instantiated control placed in a DOM tree"
47+
}
48+
}
49+
2050
class LocalModelContentBoundBidirectionallyToSourceControl extends RemoteFlowSource {
2151
UI5BindingPath bindingPath;
2252
UI5Control controlDeclaration;
@@ -60,24 +90,28 @@ class ODataServiceModel extends UI5ExternalModel {
6090
ODataServiceModel() {
6191
exists(MethodCallNode setModelCall, CustomController controller |
6292
/*
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"))`
93+
* 1. This flows from a DF node corresponding to the parent component's model
94+
* to the `this.setModel` call. e.g.
95+
*
96+
* `this.getOwnerComponent().getModel("someModelName")` as in
97+
* `this.getView().setModel(this.getOwnerComponent().getModel("someModelName"))`.
6698
*/
6799

68100
modelName = this.getArgument(0).getALocalSource().asExpr().(StringLiteral).getValue() and
69101
this.getCalleeName() = "getModel" and
70102
controller.getOwnerComponentRef().flowsTo(this.(MethodCallNode).getReceiver()) and
71103
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 */
104+
setModelCall = controller.getAViewReference().getAMemberCall("setModel") and
105+
/*
106+
* 2. The component's `manifest.json` declares the DataSource as being of OData type.
107+
*/
108+
75109
controller.getOwnerComponent().getExternalModelDef(modelName).getDataSource() instanceof
76110
ODataDataSourceManifest
77111
)
78112
or
79113
/*
80-
* A constructor call to sap.ui.model.odata.v2.ODataModel.
114+
* A constructor call to `sap.ui.model.odata.v2.ODataModel`.
81115
*/
82116

83117
this instanceof NewNode and

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

Lines changed: 79 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import semmle.javascript.security.dataflow.DomBasedXssCustomizations
55
import advanced_security.javascript.frameworks.ui5.UI5View
66
import advanced_security.javascript.frameworks.ui5.UI5HTML
77
import codeql.util.FileSystem
8+
private import semmle.javascript.frameworks.data.internal.ApiGraphModelsExtensions as ApiGraphModelsExtensions
89

910
private module WebAppResourceRootJsonReader implements JsonParser::MakeJsonReaderSig<WebApp> {
1011
class JsonReader extends WebApp {
@@ -133,10 +134,11 @@ class WebApp extends HTML::HtmlFile {
133134
}
134135

135136
/**
136-
* https://sapui5.hana.ondemand.com/sdk/#/api/sap.ui.loader%23methods/sap.ui.loader.config
137+
* The global instance of `sap.ui.Core` that represents this UI5 application, as retrieved
138+
* by a call to `sap.ui.getCore()`.
137139
*/
138-
class Loader extends CallNode {
139-
Loader() { this = globalVarRef("sap").getAPropertyRead("ui").getAMethodCall("loader") }
140+
class SapUiCore extends MethodCallNode {
141+
SapUiCore() { this = globalVarRef("sap").getAPropertyRead("ui").getAMethodCall("getCore") }
140142
}
141143

142144
/**
@@ -291,24 +293,31 @@ class CustomControl extends SapExtendCall {
291293
result = renderer.getRenderer()
292294
)
293295
}
296+
297+
ValueNode getAThisNode() {
298+
exists(ThisNode thisNode | thisNode.getBinder() = this.getAMethod() |
299+
result.getALocalSource() = thisNode
300+
)
301+
}
294302
}
295303

296304
abstract class Reference extends MethodCallNode { }
297305

298306
/**
299-
* A JS reference to a `UI5Control`, commonly obtained via `View.byId(controlId)`.
307+
* A JS reference to a `UI5Control`, commonly obtained via its ID.
300308
*/
301309
class ControlReference extends Reference {
302310
string controlId;
303311

304312
ControlReference() {
305-
exists(CustomController controller |
306-
(
307-
controller.getAViewReference().flowsTo(this.getReceiver()) or
308-
controller.getAThisNode() = this.getReceiver()
309-
) and
310-
this.getMethodName() = "byId" and
311-
this.getArgument(0).getALocalSource().asExpr().(StringLiteral).getValue() = controlId
313+
this.getArgument(0).getStringValue() = controlId and
314+
(
315+
exists(CustomController controller |
316+
this = controller.getAViewReference().getAMemberCall("byId") or
317+
this.getReceiver() = controller.getAThisNode()
318+
)
319+
or
320+
exists(SapUiCore sapUiCore | this = sapUiCore.getAMemberCall("byId"))
312321
)
313322
}
314323

@@ -453,13 +462,7 @@ class CustomController extends SapExtendCall {
453462
UI5View getView() { this = result.getController() }
454463

455464
ControlReference getAControlReference() {
456-
exists(MethodCallNode viewRef |
457-
viewRef = this.getAViewReference() and
458-
/* There is a view */
459-
viewRef.flowsTo(result.(MethodCallNode).getReceiver()) and
460-
/* The result is a member of this view */
461-
result.(MethodCallNode).getMethodName() = "byId"
462-
)
465+
result = this.getAViewReference().getAMemberCall("byId")
463466
}
464467

465468
ValueNode getAThisNode() {
@@ -481,19 +484,16 @@ class CustomController extends SapExtendCall {
481484
}
482485

483486
UI5Model getModel() {
484-
exists(MethodCallNode setModelCall |
485-
this.getAViewReference().flowsTo(setModelCall.getReceiver()) and
486-
setModelCall.getMethodName() = "setModel" and
487-
result.flowsTo(setModelCall.getAnArgument())
488-
)
487+
result = this.getAViewReference().getAMemberCall("setModel").getAnArgument().getALocalSource()
489488
}
490489

491490
ModelReference getModelReference(string modelName) {
492-
this.getAViewReference().flowsTo(result.getReceiver()) and
493-
result.getModelName() = modelName
491+
result = this.getAViewReference().getAMemberCall(modelName)
494492
}
495493

496-
ModelReference getAModelReference() { this.getAViewReference().flowsTo(result.getReceiver()) }
494+
ModelReference getAModelReference() {
495+
result = this.getAViewReference().getAMemberCall("getModel")
496+
}
497497

498498
RouterReference getARouterReference() {
499499
result.getMethodName() = "getRouter" and
@@ -514,9 +514,10 @@ class RouteReference extends MethodCallNode {
514514
string name;
515515

516516
RouteReference() {
517-
this.getMethodName() = "getRoute" and
518-
this.getArgument(0).getALocalSource().asExpr().(StringLiteral).getValue() = name and
519-
exists(RouterReference routerReference | routerReference.flowsTo(this.getReceiver()))
517+
exists(RouterReference routerReference |
518+
this = routerReference.getAMemberCall("getRoute") and
519+
this.getArgument(0).getStringValue() = name
520+
)
520521
}
521522

522523
string getName() { result = name }
@@ -1238,21 +1239,18 @@ class RequiredObject extends Expr {
12381239
*/
12391240
class SapExtendCall extends InvokeNode, MethodCallNode {
12401241
SapExtendCall() {
1241-
/* 1. The receiver object is an imported one */
12421242
exists(RequiredObject requiredModule |
1243-
requiredModule.asSourceNode().flowsTo(this.getReceiver())
1244-
) and
1245-
/* 2. The method name is `extend` */
1246-
this.(MethodCallNode).getMethodName() = "extend"
1243+
this = requiredModule.asSourceNode().getAMemberCall("extend")
1244+
)
12471245
}
12481246

12491247
FunctionNode getMethod(string methodName) {
1250-
result = this.getContent().(ObjectLiteralNode).getAPropertySource(methodName).(FunctionNode)
1248+
result = this.getContent().(ObjectLiteralNode).getAPropertySource(methodName)
12511249
}
12521250

12531251
FunctionNode getAMethod() { result = this.getMethod(_) }
12541252

1255-
string getName() { result = this.getArgument(0).asExpr().(StringLiteral).getValue() }
1253+
string getName() { result = this.getArgument(0).getStringValue() }
12561254

12571255
ObjectLiteralNode getContent() { result = this.getArgument(1) }
12581256

@@ -1269,14 +1267,34 @@ class SapExtendCall extends InvokeNode, MethodCallNode {
12691267
SapDefineModule getDefine() { this.getEnclosingFunction() = result.getArgument(1) }
12701268
}
12711269

1270+
class ElementInstantiation extends NewNode {
1271+
string importPath;
1272+
1273+
ElementInstantiation() {
1274+
exists(RequiredObject requiredObject |
1275+
this = requiredObject.asSourceNode().getAnInstantiation() and
1276+
importPath = requiredObject.getDependency()
1277+
)
1278+
}
1279+
1280+
string getId() {
1281+
result = this.getArgument(0).(SourceNode).getAPropertyWrite("id").getRhs().getStringValue()
1282+
}
1283+
1284+
string getImportPath() { result = importPath }
1285+
}
1286+
12721287
private newtype TSapElement =
1273-
DefinitionOfElement(SapExtendCall extension) or
1274-
ReferenceOfElement(Reference reference)
1288+
TDefinitionOfElement(SapExtendCall extension) or
1289+
TReferenceOfElement(Reference reference) or
1290+
TInstantiationOfElement(ElementInstantiation newNode)
12751291

12761292
class SapElement extends TSapElement {
1277-
SapExtendCall asDefinition() { this = DefinitionOfElement(result) }
1293+
SapExtendCall asDefinition() { this = TDefinitionOfElement(result) }
12781294

1279-
Reference asReference() { this = ReferenceOfElement(result) }
1295+
Reference asReference() { this = TReferenceOfElement(result) }
1296+
1297+
ElementInstantiation asInstantiation() { this = TInstantiationOfElement(result) }
12801298

12811299
SapElement getParentElement() {
12821300
result.asReference() = this.asDefinition().(CustomControl).getController().getAViewReference() or
@@ -1286,8 +1304,27 @@ class SapElement extends TSapElement {
12861304
result.asDefinition() = this.asDefinition().(CustomController).getOwnerComponent() or
12871305
result.asDefinition() =
12881306
this.asReference().(ControllerReference).getDefinition().getOwnerComponent()
1307+
/* TODO: Implement branch for `asInstantiation` */
12891308
}
12901309

1310+
string getId() {
1311+
result = this.asInstantiation().getId()
1312+
or
1313+
/* TODO: Needs testing */
1314+
result =
1315+
this.asDefinition()
1316+
.(CustomControl)
1317+
.getMetadata()
1318+
.getProperty("id")
1319+
.getAPropertySource()
1320+
.getStringValue()
1321+
/*
1322+
* Note that because we cannot statically determine the ID of an element from the references alone,
1323+
* we do not implement the branch of `TReferenceOfElement`.
1324+
*/
1325+
1326+
}
1327+
12911328
string toString() {
12921329
result = this.asDefinition().toString() or
12931330
result = this.asReference().toString()
@@ -1312,11 +1349,8 @@ class Metadata extends ObjectLiteralNode {
13121349

13131350
Metadata() { this = extension.getContent().getAPropertySource("metadata") }
13141351

1315-
SourceNode getProperty(string name) {
1316-
result =
1317-
any(PropertyMetadata property |
1318-
property.getParentMetadata() = this and property.getName() = name
1319-
)
1352+
PropertyMetadata getProperty(string name) {
1353+
result.getParentMetadata() = this and result.getName() = name
13201354
}
13211355
}
13221356

0 commit comments

Comments
 (0)