diff --git a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/RemoteFlowSources.qll b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/RemoteFlowSources.qll index b12dcf430..4a9787935 100644 --- a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/RemoteFlowSources.qll +++ b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/RemoteFlowSources.qll @@ -90,42 +90,23 @@ class ODataServiceModel extends UI5ExternalModel { override string getSourceType() { result = "ODataServiceModel" } ODataServiceModel() { - exists(MethodCallNode setModelCall, CustomController controller | - /* - * 1. This flows from a DF node corresponding to the parent component's model - * to the `this.setModel` call. e.g. - * - * `this.getOwnerComponent().getModel("someModelName")` as in - * `this.getView().setModel(this.getOwnerComponent().getModel("someModelName"))`. - */ - - modelName = this.getArgument(0).getALocalSource().asExpr().(StringLiteral).getValue() and + exists(CustomController controller | this.getCalleeName() = "getModel" and - controller.getOwnerComponentRef().flowsTo(this.(MethodCallNode).getReceiver()) and - this.flowsTo(setModelCall.getArgument(0)) and - setModelCall = controller.getAViewReference().getAMemberCall("setModel") and - /* - * 2. The component's `manifest.json` declares the DataSource as being of OData type. - */ - + modelName = this.getArgument(0).getALocalSource().getStringValue() and controller.getOwnerComponent().getExternalModelDef(modelName).getDataSource() instanceof - ODataDataSourceManifest + ODataDataSourceManifest // A component's `manifest.json` declares the data source as being of OData type. ) or /* - * A constructor call to sap.ui.model.odata.v2.ODataModel or sap.ui.model.odata.v4.ODataModel. + * A constructor call to `sap.ui.model.odata.v2.ODataModel` or `sap.ui.model.odata.v4.ODataModel`. */ this instanceof NewNode and - ( - exists(RequiredObject oDataModel | - oDataModel.asSourceNode().flowsTo(this.getCalleeNode()) and - oDataModel.getDependency() in [ - "sap/ui/model/odata/v2/ODataModel", "sap/ui/model/odata/v4/ODataModel" - ] - ) - or - this.getCalleeName() = "ODataModel" + exists(RequiredObject oDataModel | + oDataModel.asSourceNode().flowsTo(this.getCalleeNode()) and + oDataModel.getDependency() in [ + "sap/ui/model/odata/v2/ODataModel", "sap/ui/model/odata/v4/ODataModel" + ] ) and modelName = "" } diff --git a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5.qll b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5.qll index 9a110016c..2045fa684 100644 --- a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5.qll +++ b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5.qll @@ -1,6 +1,7 @@ import javascript import DataFlow import advanced_security.javascript.frameworks.ui5.JsonParser +import advanced_security.javascript.frameworks.ui5.dataflow.TypeTrackers import semmle.javascript.security.dataflow.DomBasedXssCustomizations import advanced_security.javascript.frameworks.ui5.UI5View import advanced_security.javascript.frameworks.ui5.UI5HTML @@ -167,11 +168,17 @@ class SapDefineModule extends AmdModuleDefinition::Range, MethodCallExpr, UserMo sap.getName() = "sap" and sapUi.getBase() = sap and sapUi.getPropertyName() = "ui" and - this.getReceiver() = sapUiDefine - // and this.getMethodName() = "define" + this.getReceiver() = sapUiDefine and + this.getMethodName() = ["define", "require"] // TODO: Treat sap.ui.declare in its own class ) } + SapExtendCall getExtendCall() { result.getDefine() = this } + + string getName() { result = this.getExtendCall().getName() } + + Module asModule() { result = this.getTopLevel() } + string getDependency(int i) { result = this.(AmdModuleDefinition).getDependencyExpr(i).getStringValue() } @@ -187,17 +194,48 @@ class SapDefineModule extends AmdModuleDefinition::Range, MethodCallExpr, UserMo WebApp getWebApp() { this.getFile() = result.getAResource() } /** - * Gets the module defined with sap.ui.define that imports and extends this module. + * Gets the module defined with sap.ui.define that imports and extends (subclasses) this module. */ - SapDefineModule getExtendingModule() { - exists(SapExtendCall baseExtendCall, SapExtendCall subclassExtendCall | - baseExtendCall.getDefine() = this and - result = subclassExtendCall.getDefine() and - result - .getRequiredObject(baseExtendCall.getName().replaceAll(".", "/")) - .asSourceNode() - .flowsTo(subclassExtendCall.getReceiver()) + SapDefineModule getExtendingModule() { result.getSuperModule(_) = this } + + /** + * Gets the module that this module imports via path `importPath`. + */ + SapDefineModule getImportedModule(string importPath) { + /* 1. Absolute import paths: We resolve this ourselves. */ + exists(string importedModuleDefinitionPath, string importedModuleDefinitionPathSlashNormalized | + /* + * Let `importPath` = "my/app/path1/path2/controller/Some.controller", + * `importedModuleDefinitionPath` = "my.app.path1.path2.controller.Some", + * `importedModuleDefinitionPathSlashNormalized` = "my/app/path1/path2/controller/Some". + * Then, `importedModuleDefinitionPathSlashNormalized` matches `importPath`. + */ + + importPath = this.asModule().getAnImport().getImportedPathExpr().getStringValue() and + importedModuleDefinitionPath = result.getExtendCall().getName() and + importedModuleDefinitionPathSlashNormalized = + importedModuleDefinitionPath.replaceAll(".", "/") and + importPath.matches(importedModuleDefinitionPathSlashNormalized + "%") ) + or + /* + * 2. Relative import paths: We delegate the heaving lifting of resolving to + * `Import.resolveImportedPath/0`. + */ + + exists(Import import_ | + importPath = import_.getImportedPathExpr().getStringValue() and + import_ = this.asModule().getAnImport() and + import_.resolveImportedPath() = result.getTopLevel() + ) + } + + /** + * Holds if the `importingModule` extends the `importedModule`, imported via path `importPath`. + */ + SapDefineModule getSuperModule(string importPath) { + result = this.getImportedModule(importPath) and + this.getRequiredObject(importPath).asSourceNode().flowsTo(this.getExtendCall().getReceiver()) } } @@ -250,7 +288,9 @@ class CustomControl extends SapExtendCall { this = TypeTrackers::hasDependency(["sap/ui/core/Control", "sap.ui.core.Control"]) .getAMemberCall("extend") or - exists(SapDefineModule sapModule | this.getDefine() = sapModule.getExtendingModule()) + exists(CustomControl superControl | + superControl.getDefine() = this.getDefine().getSuperModule(_) + ) } CustomController getController() { this = result.getAControlReference().getDefinition() } @@ -452,29 +492,29 @@ class CustomController extends SapExtendCall { string name; CustomController() { - this = - TypeTrackers::hasDependency(["sap/ui/core/mvc/Controller", "sap.ui.core.mvc.Controller"]) - .getAMemberCall("extend") and + ( + this = + TypeTrackers::hasDependency(["sap/ui/core/mvc/Controller", "sap.ui.core.mvc.Controller"]) + .getAMemberCall("extend") + or + exists(CustomController superController | + superController.getDefine() = this.getDefine().getSuperModule(_) + ) + ) and name = this.getFile().getBaseName().regexpCapture("([a-zA-Z0-9]+).[cC]ontroller.js", 1) } Component getOwnerComponent() { - exists(ManifestJson manifestJson, JsonObject rootObj | manifestJson = result.getManifestJson() | - rootObj - .getPropValue("targets") - .(JsonObject) - // The individual targets - .getPropValue(_) - .(JsonObject) - // The target's "viewName" property - .getPropValue("viewName") - .(JsonString) - .getValue() = name - ) + this = result.getParentManifestJson().getARoutingTarget().getView().getController() } MethodCallNode getOwnerComponentRef() { result = this.getAThisNode().getAMemberCall("getOwnerComponent") + or + exists(CustomController baseController | + baseController.getDefine() = this.getDefine().getSuperModule(_) and + result = baseController.getOwnerComponentRef() + ) } /** @@ -743,6 +783,11 @@ abstract class UI5InternalModel extends UI5Model, NewNode { abstract string getPathString(); abstract string getPathString(Property property); + + /** + * Holds if the content of the model is statically determinable. + */ + abstract predicate contentIsStaticallyVisible(); } import ManifestJson @@ -766,7 +811,7 @@ class Component extends SapExtendCall { string getId() { result = this.getName().regexpCapture("([a-zA-Z0-9.]+).Component", 1) } - ManifestJson getManifestJson() { + ManifestJson getParentManifestJson() { this.getMetadata().getAPropertySource("manifest").asExpr().(StringLiteral).getValue() = "json" and result.getId() = this.getId() } @@ -788,7 +833,7 @@ class Component extends SapExtendCall { } ExternalModelManifest getExternalModelDef(string modelName) { - result.getFile() = this.getManifestJson() and result.getName() = modelName + result.getFile() = this.getParentManifestJson() and result.getName() = modelName } ExternalModelManifest getAnExternalModelDef() { result = this.getExternalModelDef(_) } @@ -815,11 +860,51 @@ module ManifestJson { string getName() { result = dataSourceName } - ManifestJson getManifestJson() { result = manifestJson } + ManifestJson getParentManifestJson() { result = manifestJson } string getType() { result = this.getPropValue("type").(JsonString).getValue() } } + class RoutingTargetManifest extends JsonObject { + /** Note: This is NOT its `viewName` property! */ + string targetName; + ManifestJson manifestJson; + + RoutingTargetManifest() { + exists(JsonObject rootObj | + this.getJsonFile() = manifestJson and + rootObj.getJsonFile() = manifestJson and + this = + rootObj + .getPropValue("sap.ui5") + .(JsonObject) + .getPropValue("routing") + .(JsonObject) + .getPropValue("targets") + .(JsonObject) + .getPropValue(targetName) + ) + } + + /** + * Gets the value of the `viewName` property of this target. + */ + string getViewName() { result = this.getPropStringValue("viewName") } + + /** + * Gets the view this target is associated with. + */ + UI5View getView() { + result.getController().getModuleName() = + getSubstringAfterLastOccurrenceOfCharacter(this.getViewName(), "/") + } + + /** + * Gets the `manifest.json` file that this routing target is a part of. + */ + ManifestJson getParentManifestJson() { result = manifestJson } + } + class ODataDataSourceManifest extends DataSourceManifest { ODataDataSourceManifest() { this.getType() = "OData" } } @@ -948,7 +1033,19 @@ module ManifestJson { this.getBaseName() = "manifest.json" } - DataSourceManifest getDataSource() { this = result.getManifestJson() } + DataSourceManifest getADataSource() { result = this.getDataSource(_) } + + DataSourceManifest getDataSource(string name) { + this = result.getParentManifestJson() and + result.getName() = name + } + + RoutingTargetManifest getARoutingTarget() { result = this.getRoutingTarget(_) } + + RoutingTargetManifest getRoutingTarget(string viewName) { + result.getViewName() = viewName and + result.getParentManifestJson() = this + } } } @@ -1121,6 +1218,16 @@ class JsonModel extends UI5InternalModel { ) } + override predicate contentIsStaticallyVisible() { + /* 1. There is at least one path string that can be constructed out of the path string. */ + exists(this.getPathString()) + or + /* 2. There is a JSON file that can be loaded from. */ + exists(JsonObject jsonObject | + jsonObject = resolveDirectPath(this.getArgument(0).getStringValue()) + ) + } + /** * A model possibly supporting two-way binding explicitly set as a one-way binding model. */ @@ -1178,6 +1285,8 @@ class XmlModel extends UI5InternalModel { } override string getPathString() { result = "TODO" } + + override predicate contentIsStaticallyVisible() { exists(this.getPathString()) } } class ResourceModel extends UI5Model, ModelReference { @@ -1251,6 +1360,10 @@ class SapExtendCall extends InvokeNode, MethodCallNode { string getName() { result = this.getArgument(0).getALocalSource().getStringValue() } + string getModuleName() { + result = getSubstringAfterLastOccurrenceOfCharacter(this.getName(), ".") + } + ObjectLiteralNode getContent() { result = this.getArgument(1) } Metadata getMetadata() { @@ -1428,18 +1541,12 @@ class PropertyMetadata extends ObjectLiteralNode { } } -module TypeTrackers { - private SourceNode hasDependency(TypeTracker t, string dependencyPath) { - t.start() and - exists(UserModule d | - d.getADependency() = dependencyPath and - result = d.getRequiredObject(dependencyPath).asSourceNode() - ) - or - exists(TypeTracker t2 | result = hasDependency(t2, dependencyPath).track(t2, t)) - } +bindingset[input, character] +private int countCharacterInString(string input, string character) { + result = count(int index | character = input.charAt(index) | index) +} - SourceNode hasDependency(string dependencyPath) { - result = hasDependency(TypeTracker::end(), dependencyPath) - } +bindingset[input, character] +private string getSubstringAfterLastOccurrenceOfCharacter(string input, string character) { + result = input.splitAt(character, countCharacterInString(input, character)) } diff --git a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5View.qll b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5View.qll index b7b57c7dc..90e6018c9 100644 --- a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5View.qll +++ b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5View.qll @@ -162,6 +162,13 @@ abstract class UI5BindingPath extends BindingPath { inSameWebApp(this.getLocation().getFile(), result.getFile()) ) or + exists(JsonModel model, CustomController controller | + not model.contentIsStaticallyVisible() and + result = model and + this.getView() = controller.getAViewReference().getDefinition() and + controller.getModel() = result + ) + or /* 2. External (Server-side) model */ result = this.getModel().(UI5ExternalModel) and /* Restrict search to inside the same webapp. */ diff --git a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/FlowSteps.qll b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/FlowSteps.qll index a3463e079..f74a8b8d6 100644 --- a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/FlowSteps.qll +++ b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/FlowSteps.qll @@ -366,3 +366,29 @@ class LogArgumentToListener extends DataFlow::SharedFlowStep { logArgumentToListener(start, end) } } + +/** + * A step within an object wrapped in an `sap.ui.extend` call. Jumps from + * the value written to a property of the object to the read of it. + * + * This step is only established only if the property read and property writes + * are in different methods, so as not to jump to a property read that comes + * before the property write in the same method. + */ +class ThisNodePropertyWriteToThisNodePropertyRead extends DataFlow::SharedFlowStep { + override predicate step(DataFlow::Node start, DataFlow::Node end) { + exists( + SapExtendCall sapExtendCall, ThisNode propReadThisNode, ThisNode propWriteThisNode, + PropRead propRead, PropWrite propWrite, string propName + | + propReadThisNode = sapExtendCall.getAThisNode() and + propWriteThisNode = sapExtendCall.getAThisNode() and + propRead = propReadThisNode.getAPropertyRead(propName) and + propWrite = propWriteThisNode.getAPropertyWrite(propName) and + start = propWrite.getRhs() and + end = propRead and + /* They belong to different methods of the object. */ + propReadThisNode.getBinder() != propWriteThisNode.getBinder() + ) + } +} diff --git a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/TypeTrackers.qll b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/TypeTrackers.qll new file mode 100644 index 000000000..7f47325a8 --- /dev/null +++ b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/TypeTrackers.qll @@ -0,0 +1,64 @@ +import javascript +import DataFlow + +module TypeTrackers { + private SourceNode hasDependency(TypeTracker t, string dependencyPath) { + t.start() and + exists(UserModule d | + d.getADependency() = dependencyPath and + result = d.getRequiredObject(dependencyPath).asSourceNode() + ) + or + exists(TypeTracker t2 | result = hasDependency(t2, dependencyPath).track(t2, t)) + } + + SourceNode hasDependency(string dependencyPath) { + result = hasDependency(TypeTracker::end(), dependencyPath) + } + + private MethodCallNode getOwnerComponentRef(TypeTracker t, CustomController customController) { + customController.getAThisNode() = result.getReceiver() and + result.getMethodName() = "getOwnerComponent" + or + exists(TypeTracker t2 | result = getOwnerComponentRef(t2, customController).track(t2, t)) + } + + /* owner component ref */ + MethodCallNode getOwnerComponentRef(CustomController customController) { + result = getOwnerComponentRef(TypeTracker::end(), customController) + } +} + +module Test { + private import semmle.javascript.dataflow.TypeTracking + + private class ObjFieldStep extends SharedTypeTrackingStep { + override predicate step(DataFlow::Node node1, DataFlow::Node node2) { + exists(DataFlow::SourceNode object, string name | + methodStepPred(object, name, node1) and + methodStepSucc(object, name, node2) + ) + } + } + + private DataFlow::SourceNode objectWithMethods() { + result.flowsTo(any(DataFlow::CallNode call | call.getCalleeName() = "extend").getAnArgument()) + } + + private DataFlow::SourceNode getAnAlias(DataFlow::SourceNode object) { + object = objectWithMethods() and + ( + result = object + or + result = getAnAlias(object).getAPropertySource().(DataFlow::FunctionNode).getReceiver() + ) + } + + private predicate methodStepPred(DataFlow::SourceNode object, string name, DataFlow::Node rhs) { + rhs = getAnAlias(object).getAPropertyWrite(name).getRhs() + } + + private predicate methodStepSucc(DataFlow::SourceNode object, string name, DataFlow::Node read) { + read = getAnAlias(object).getAPropertyRead(name) + } +}