Skip to content

Commit 1a52a1e

Browse files
committed
Relax assumption of ODataServiceModel and add a flow step
1 parent 445b573 commit 1a52a1e

File tree

2 files changed

+35
-28
lines changed

2 files changed

+35
-28
lines changed

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

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -90,42 +90,23 @@ class ODataServiceModel extends UI5ExternalModel {
9090
override string getSourceType() { result = "ODataServiceModel" }
9191

9292
ODataServiceModel() {
93-
exists(MethodCallNode setModelCall, CustomController controller |
94-
/*
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"))`.
100-
*/
101-
102-
modelName = this.getArgument(0).getALocalSource().asExpr().(StringLiteral).getValue() and
93+
exists(CustomController controller |
10394
this.getCalleeName() = "getModel" and
104-
controller.getOwnerComponentRef().flowsTo(this.(MethodCallNode).getReceiver()) and
105-
this.flowsTo(setModelCall.getArgument(0)) and
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-
95+
modelName = this.getArgument(0).getALocalSource().getStringValue() and
11196
controller.getOwnerComponent().getExternalModelDef(modelName).getDataSource() instanceof
112-
ODataDataSourceManifest
97+
ODataDataSourceManifest // A component's `manifest.json` declares the data source as being of OData type.
11398
)
11499
or
115100
/*
116-
* A constructor call to sap.ui.model.odata.v2.ODataModel or sap.ui.model.odata.v4.ODataModel.
101+
* A constructor call to `sap.ui.model.odata.v2.ODataModel` or `sap.ui.model.odata.v4.ODataModel`.
117102
*/
118103

119104
this instanceof NewNode and
120-
(
121-
exists(RequiredObject oDataModel |
122-
oDataModel.asSourceNode().flowsTo(this.getCalleeNode()) and
123-
oDataModel.getDependency() in [
124-
"sap/ui/model/odata/v2/ODataModel", "sap/ui/model/odata/v4/ODataModel"
125-
]
126-
)
127-
or
128-
this.getCalleeName() = "ODataModel"
105+
exists(RequiredObject oDataModel |
106+
oDataModel.asSourceNode().flowsTo(this.getCalleeNode()) and
107+
oDataModel.getDependency() in [
108+
"sap/ui/model/odata/v2/ODataModel", "sap/ui/model/odata/v4/ODataModel"
109+
]
129110
) and
130111
modelName = "<no name>"
131112
}

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,3 +366,29 @@ class LogArgumentToListener extends DataFlow::SharedFlowStep {
366366
logArgumentToListener(start, end)
367367
}
368368
}
369+
370+
/**
371+
* A step within an object wrapped in an `sap.ui.extend` call. Jumps from
372+
* the value written to a property of the object to the read of it.
373+
*
374+
* This step is only established only if the property read and property writes
375+
* are in different methods, so as not to jump to a property read that comes
376+
* before the property write in the same method.
377+
*/
378+
class ThisNodePropertyWriteToThisNodePropertyRead extends DataFlow::SharedFlowStep {
379+
override predicate step(DataFlow::Node start, DataFlow::Node end) {
380+
exists(
381+
SapExtendCall sapExtendCall, ThisNode propReadThisNode, ThisNode propWriteThisNode,
382+
PropRead propRead, PropWrite propWrite, string propName
383+
|
384+
propReadThisNode = sapExtendCall.getAThisNode() and
385+
propWriteThisNode = sapExtendCall.getAThisNode() and
386+
propRead = propReadThisNode.getAPropertyRead(propName) and
387+
propWrite = propWriteThisNode.getAPropertyWrite(propName) and
388+
start = propWrite.getRhs() and
389+
end = propRead and
390+
/* They belong to different methods of the object. */
391+
propReadThisNode.getBinder() != propWriteThisNode.getBinder()
392+
)
393+
}
394+
}

0 commit comments

Comments
 (0)