Skip to content

Commit ff1babc

Browse files
authored
Merge branch 'main' into jeongsoolee09/add-supermodule-more-mad
2 parents a92e242 + 9fac18b commit ff1babc

52 files changed

Lines changed: 725 additions & 8 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/javascript.sarif.expected

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,10 @@ extensions:
7676
- ["UI5ClientStorage", "global", "Member[jQuery].Member[sap].Member[storage]"]
7777
- ["UI5ClientStorage", "sap/ui/core/util/File", ""]
7878
- ["UI5ClientStorage", "global", "Member[sap].Member[ui].Member[core].Member[util].Member[File]"]
79+
# Fragment API - byId returns a Control (models static Fragment.byId() calls)
80+
- ["Fragment", "Fragment", "Instance"]
81+
- ["Fragment", "sap/ui/core/Fragment", ""]
82+
- ["UI5InputControl", "Fragment", "Member[byId].ReturnValue"]
7983
# Publishing and Subscribing to Events
8084
- ["UI5EventBusInstance", "sap/ui/core/EventBus", "Member[getInstance].ReturnValue"]
8185
- ["UI5EventBusPublish", "UI5EventBusInstance", "Member[publish]"]

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -597,6 +597,8 @@ class BindingTarget extends TBindingTarget {
597597
this = TLateJavaScriptBindingTarget(target, _)
598598
or
599599
this = TEarlyJavaScriptPropertyBindingTarget(target, _)
600+
or
601+
this = TLateJavaScriptBindingTarget(target.(BindElementMethodCallNode).getReceiver(), _)
600602
) and
601603
result = target
602604
)
@@ -727,6 +729,22 @@ class Binding extends TBinding {
727729
)
728730
}
729731

732+
DataFlow::Node asDataFlowNode() {
733+
exists(DataFlow::Node target |
734+
(
735+
this = TEarlyJavaScriptPropertyBinding(_, target)
736+
or
737+
this = TLateJavaScriptPropertyBinding(_, target)
738+
or
739+
exists(BindElementMethodCallNode bindElementCall |
740+
target = bindElementCall.getReceiver() and
741+
this = TLateJavaScriptContextBinding(bindElementCall, _)
742+
)
743+
) and
744+
result = target
745+
)
746+
}
747+
730748
BindingPath getBindingPath() { result.getBinding() = this }
731749

732750
BindingTarget getBindingTarget() { result.getBinding() = this }
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/**
2+
* Provides classes for modeling the sap.ui.core.Fragment API.
3+
*/
4+
5+
import javascript
6+
import DataFlow
7+
import advanced_security.javascript.frameworks.ui5.UI5
8+
9+
/**
10+
* Gets a reference to the Fragment module import.
11+
*/
12+
class FragmentLoad extends InvokeNode, MethodCallNode {
13+
FragmentLoad() {
14+
this =
15+
TypeTrackers::hasDependency(["sap/ui/core/Fragment", "sap.ui.core.Fragment"])
16+
.getAMemberCall("load")
17+
or
18+
exists(RequiredObject requiredModule, SapDefineModule sapModule |
19+
this = requiredModule.asSourceNode().getAMemberCall("load") and
20+
/* ensure it is an sap module define */
21+
requiredModule.getEnclosingFunction() = sapModule.getArgument(1)
22+
)
23+
}
24+
25+
/**
26+
* Gets the configuration object passed to Fragment.load().
27+
*/
28+
DataFlow::Node getConfigObject() { result = this.getArgument(0) }
29+
30+
/**
31+
* Gets the 'name' property value from the config object.
32+
* This specifies the fragment resource to load (e.g., "my.app.fragments.MyFragment").
33+
*/
34+
DataFlow::Node getNameArgument() {
35+
exists(DataFlow::ObjectLiteralNode config |
36+
config = this.getConfigObject().getALocalSource() and
37+
result = config.getAPropertyWrite("name").getRhs()
38+
)
39+
}
40+
41+
/**
42+
* Gets the 'controller' property value from the config object.
43+
* This specifies the fragment's controller, if it has one.
44+
*/
45+
DataFlow::Node getControllerArgument() {
46+
exists(DataFlow::ObjectLiteralNode config |
47+
config = this.getConfigObject().getALocalSource() and
48+
result = config.getAPropertyWrite("controller").getRhs()
49+
)
50+
}
51+
52+
DataFlow::ParameterNode getCallbackObjectReference() {
53+
//the load invoke node is actually just a part of the chained load.then.bind
54+
result = this.getAMemberCall(_).getABoundCallbackParameter(_, _)
55+
}
56+
}

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,25 @@ abstract class UI5ExternalModel extends UI5Model, RemoteFlowSource {
110110
abstract string getName();
111111
}
112112

113+
/** Default model which gains content from an SAP OData service (ie no model name is explicitly specified). */
114+
class DefaultODataServiceModel extends UI5ExternalModel {
115+
DefaultODataServiceModel() {
116+
exists(ExternalModelManifest model |
117+
//an OData default model exists
118+
model.getName() = "" and
119+
model.getDataSource() instanceof ODataDataSourceManifest and
120+
//therefore the bindElement calls that exist may be sources and also approximates the model itself
121+
this.getCalleeName() = "bindElement"
122+
)
123+
}
124+
125+
override string getSourceType() { result = "DefaultODataServiceModel" }
126+
127+
override string getName() { result = "" }
128+
129+
Binding asBinding() { result.getBindingTarget().asDataFlowNode() = this }
130+
}
131+
113132
/** Model which gains content from an SAP OData service. */
114133
class ODataServiceModel extends UI5ExternalModel {
115134
string modelName;

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@ private module WebAppResourceRootJsonReader implements JsonParser::MakeJsonReade
1212
class JsonReader extends WebApp {
1313
string getJson() {
1414
// We match on the lowercase to cover all the possible variants of writing the attribute name.
15+
// Support both "data-sap-ui-resourceroots" and "data-sap-ui-resource-roots" (with hyphen)
1516
exists(string resourceRootAttributeName |
16-
resourceRootAttributeName.toLowerCase() = "data-sap-ui-resourceroots"
17+
resourceRootAttributeName.toLowerCase() =
18+
["data-sap-ui-resourceroots", "data-sap-ui-resource-roots"]
1719
|
1820
result = this.getCoreScript().getAttributeByName(resourceRootAttributeName).getValue()
1921
)
@@ -151,6 +153,17 @@ class SapUiCore extends MethodCallNode {
151153
SapUiCore() { this = globalVarRef("sap").getAPropertyRead("ui").getAMethodCall("getCore") }
152154
}
153155

156+
/**
157+
* A reference to the Fragment module (`sap/ui/core/Fragment`).
158+
* Used for static methods like `Fragment.byId(viewId, controlId)`.
159+
*
160+
* Use of `DataFlow::moduleImport` may not cover byId references
161+
* coming from sources with es6 style imports of Fragments.
162+
*/
163+
class FragmentModule extends DataFlow::SourceNode {
164+
FragmentModule() { this = DataFlow::moduleImport("sap/ui/core/Fragment") }
165+
}
166+
154167
/**
155168
* A user-defined module through `sap.ui.define` or `jQuery.sap.declare`.
156169
*/
@@ -382,6 +395,7 @@ class ControlReference extends Reference {
382395
string controlId;
383396

384397
ControlReference() {
398+
// Standard byId patterns: this.byId("id"), this.getView().byId("id"), sap.ui.getCore().byId("id")
385399
this.getArgument(0).getALocalSource().getStringValue() = controlId and
386400
(
387401
exists(CustomController controller |
@@ -391,6 +405,11 @@ class ControlReference extends Reference {
391405
or
392406
exists(SapUiCore sapUiCore | this = sapUiCore.getAMemberCall("byId"))
393407
)
408+
or
409+
// Fragment.byId(viewId, controlId) - static method with 2 arguments
410+
this.getNumArgument() = 2 and
411+
this.getArgument(1).getALocalSource().getStringValue() = controlId and
412+
exists(FragmentModule fragment | this = fragment.getAMemberCall("byId"))
394413
}
395414

396415
CustomControl getDefinition() {

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

Lines changed: 123 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import advanced_security.javascript.frameworks.ui5.UI5
22
import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow
33
private import semmle.javascript.frameworks.data.internal.ApiGraphModelsExtensions as ApiGraphModelsExtensions
44
import advanced_security.javascript.frameworks.ui5.Bindings
5+
import advanced_security.javascript.frameworks.ui5.Fragment
56

67
/**
78
* Gets the immediate supertype of a given type from the extensible predicate `typeModel` provided by
@@ -131,6 +132,24 @@ abstract class UI5BindingPath extends BindingPath {
131132
not exists(controlSetModelCall.getArgument(1)) and
132133
not exists(this.getModelName())
133134
)
135+
or
136+
/* 5. There is no call to `setModel` at all and a default model exists that is related to the binding path this refers to */
137+
exists(DefaultODataServiceModel defaultModel |
138+
result = defaultModel and
139+
not exists(MethodCallNode viewSetModelCall | viewSetModelCall.getMethodName() = "setModel") and
140+
/*
141+
* this binding path can occur in a fragment that is the receiver object for the bindElement model approximation
142+
* i.e. checks that the default model is relevant
143+
*/
144+
145+
exists(FragmentLoad load |
146+
load.getCallbackObjectReference().flowsTo(defaultModel.asBinding().asDataFlowNode()) and
147+
load.getNameArgument()
148+
.getStringValue()
149+
.matches("%" +
150+
this.getLocation().getFile().getBaseName().replaceAll(".fragment.xml", "") + "%")
151+
)
152+
)
134153
)
135154
// and
136155
// /* This binding path and the resulting model should live inside the same webapp */
@@ -679,8 +698,97 @@ class XmlView extends UI5View instanceof XmlFile {
679698
}
680699
}
681700

701+
/**
702+
* An xml fragment. It may or may not have controllers associated.
703+
*/
704+
class XmlFragment extends UI5View instanceof XmlFile {
705+
XmlRootElement root;
706+
707+
XmlFragment() {
708+
root = this.getARootElement() and
709+
(
710+
root.getNamespace().getUri() = "sap.m"
711+
or
712+
root.getNamespace().getUri() = "sap.ui.core"
713+
) and
714+
root.hasName("FragmentDefinition")
715+
}
716+
717+
override XmlBindingPath getASource() {
718+
exists(XmlElement control, string type, string path, string property |
719+
type = result.getControlTypeName() and
720+
this = control.getFile() and
721+
ApiGraphModelsExtensions::sourceModel(getASuperType(type), path, "remote", _) and
722+
property = path.replaceAll(" ", "").regexpCapture("Member\\[([^\\]]+)\\]", 1) and
723+
result.getBindingTarget() = control.getAttribute(property)
724+
)
725+
}
726+
727+
override XmlBindingPath getAnHtmlISink() {
728+
exists(XmlElement control, string type, string path, string property |
729+
this = control.getFile() and
730+
type = result.getControlTypeName() and
731+
ApiGraphModelsExtensions::sinkModel(getASuperType(type), path, "ui5-html-injection", _) and
732+
property = path.replaceAll(" ", "").regexpCapture("Member\\[([^\\]]+)\\]", 1) and
733+
result.getBindingTarget() = control.getAttribute(property)
734+
)
735+
}
736+
737+
override UI5Control getControl() {
738+
exists(XmlElement element |
739+
result.asXmlControl() = element and
740+
/* Use getAChild+ because some controls nest other controls inside them as aggregations */
741+
element = root.getAChild+() and
742+
(
743+
/* 1. A builtin control provided by UI5 */
744+
isBuiltInControl(element.getNamespace().getUri())
745+
or
746+
/* 2. A custom control with implementation code found in the webapp */
747+
exists(CustomControl control |
748+
control.getName() = element.getNamespace().getUri() + "." + element.getName() and
749+
inSameWebApp(control.getFile(), element.getFile())
750+
)
751+
)
752+
)
753+
}
754+
755+
/**
756+
* This is either known in the location from which `loadFragment` is called (in a controller's init function)
757+
* OR in the optional controller param of `Fragment.load`.
758+
* This MAY return no value, when the fragment is not associated to any controller.
759+
* When this returns a value it is guaranteed that this xml fragment is instantiated.
760+
*/
761+
override string getControllerName() {
762+
exists(CustomController controller, MethodCallNode loadFragmentCall |
763+
loadFragmentCall.getMethodName() = "loadFragment" and
764+
controller.getAThisNode().flowsTo(loadFragmentCall.getReceiver()) and
765+
controller.getName() = result
766+
)
767+
or
768+
exists(CustomController controller, FragmentLoad fragmentLoad |
769+
controller.getAThisNode().flowsTo(fragmentLoad.getControllerArgument()) and
770+
/*
771+
* extracting just the base name of the fragment (not the fully qualified)
772+
* otherwise difficult to know which part of absolute path is only for the qualified name
773+
*/
774+
775+
fragmentLoad
776+
.getNameArgument()
777+
.getStringValue()
778+
.matches("%" + this.getBaseName().replaceAll(".fragment.xml", "")) and
779+
controller.getName() = result
780+
)
781+
}
782+
}
783+
682784
private newtype TUI5Control =
683-
TXmlControl(XmlElement control) or
785+
TXmlControl(XmlElement control) {
786+
control
787+
.(Locatable)
788+
.getFile()
789+
.getBaseName()
790+
.matches(["%.view.xml", "%.view.html", "%.fragment.xml"])
791+
} or
684792
TJsonControl(JsonObject control) {
685793
exists(JsonView view | control.getParent() = view.getRoot().getPropValue("content"))
686794
} or
@@ -776,11 +884,23 @@ class UI5Control extends TUI5Control {
776884

777885
/**
778886
* Gets a reference to this control. Currently supports only such references made through `byId`.
887+
* Handles both:
888+
* - `this.byId("controlId")` or `this.getView().byId("controlId")` - ID in argument 0
889+
* - `Fragment.byId(viewId, "controlId")` - ID in argument 1
779890
*/
780891
ControlReference getAReference() {
781892
result.getMethodName() = "byId" and
782-
result.getArgument(0).getALocalSource().asExpr().(StringLiteral).getValue() =
783-
this.getProperty("id").getValue()
893+
(
894+
// Standard byId: ID in first argument
895+
result.getNumArgument() = 1 and
896+
result.getArgument(0).getALocalSource().asExpr().(StringLiteral).getValue() =
897+
this.getProperty("id").getValue()
898+
or
899+
// Fragment.byId: ID in second argument
900+
result.getNumArgument() = 2 and
901+
result.getArgument(1).getALocalSource().asExpr().(StringLiteral).getValue() =
902+
this.getProperty("id").getValue()
903+
)
784904
}
785905

786906
/** Gets a property of this control having the name. */

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,36 @@
11
import javascript
22
import advanced_security.javascript.frameworks.ui5.UI5
3+
import advanced_security.javascript.frameworks.ui5.UI5View
4+
5+
/**
6+
* Step from a value assigned to a JSONModel property to the binding path that reads it.
7+
* This enables tracking data flowing INTO a model constructor argument and OUT through XML bindings.
8+
*
9+
* e.g. Given:
10+
* ```javascript
11+
* var userInput = getUntrustedData();
12+
* var oModel = new JSONModel({ payload: userInput });
13+
* this.getView().setModel(oModel);
14+
* ```
15+
* and
16+
* ```xml
17+
* <core:HTML content="{/payload}" />
18+
* ```
19+
*
20+
* Creates a flow step from `userInput` (the RHS of the property) to the binding path `{/payload}`.
21+
*/
22+
class JsonModelPropertyValueToBindingStep extends DataFlow::SharedFlowStep {
23+
override predicate step(DataFlow::Node start, DataFlow::Node end) {
24+
exists(UI5BindingPath bindingPath, DataFlow::PropWrite propWrite |
25+
// The binding path resolves to a property write in a JSONModel
26+
bindingPath.getNode() = propWrite and
27+
// The start is the RHS value being assigned to that property
28+
start = propWrite.getRhs() and
29+
// The end is the binding path itself (as a node representing where data flows to)
30+
end = propWrite
31+
)
32+
}
33+
}
334

435
/**
536
* Step from a part of internal model to a relevant control property.
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
| sink1.xml:7:5:7:68 | content={path: '/input'} | The binding path `content={path: '/input'}` is an HTML injection sink. |
2-
| sink1.xml:8:5:8:44 | content={path: '/input'} | The binding path `content={path: '/input'}` is an HTML injection sink. |
3-
| sink1.xml:10:5:10:73 | value={path: '/input'} | The binding path `value={path: '/input'}` is an HTML injection sink. |
1+
| sink1.view.xml:7:5:7:68 | content={path: '/input'} | The binding path `content={path: '/input'}` is an HTML injection sink. |
2+
| sink1.view.xml:8:5:8:44 | content={path: '/input'} | The binding path `content={path: '/input'}` is an HTML injection sink. |
3+
| sink1.view.xml:10:5:10:73 | value={path: '/input'} | The binding path `value={path: '/input'}` is an HTML injection sink. |

javascript/frameworks/ui5/test/models/sink/sink1.xml renamed to javascript/frameworks/ui5/test/models/sink/sink1.view.xml

File renamed without changes.

0 commit comments

Comments
 (0)