From 19e9deb998f4028e9e1237028b009db868398475 Mon Sep 17 00:00:00 2001 From: Jeongsoo Lee Date: Tue, 21 Oct 2025 15:14:41 -0400 Subject: [PATCH 1/7] Draft out `getImportedModule` and `getSubclassedModule` --- .../javascript/frameworks/ui5/UI5.qll | 92 ++++++++++++++++--- .../javascript/frameworks/ui5/UI5View.qll | 7 ++ 2 files changed, 88 insertions(+), 11 deletions(-) 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 be274c2e7..b08d1984d 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 @@ -165,11 +165,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() } @@ -185,20 +191,67 @@ 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()) - ) + // exists(SapExtendCall baseExtendCall, SapExtendCall subclassExtendCall | + // baseExtendCall.getDefine() = this and + // result = subclassExtendCall.getDefine() and + // result + // .getRequiredObject(baseExtendCall.getName().replaceAll(".", "/")) + // .asSourceNode() + // .flowsTo(subclassExtendCall.getReceiver()) + // ) + none() // TODO } } +/** + * Holds if the `importingModule` "imports" the `importedModule` via path `importPath`. + */ +private predicate importerAndImportee( + SapDefineModule importingModule, SapDefineModule importedModule, 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 = importingModule.asModule().getAnImport().getImportedPathExpr().getStringValue() and + importedModuleDefinitionPath = importedModule.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_ = importingModule.asModule().getAnImport() and + import_.resolveImportedPath() = importedModule.getTopLevel() + ) +} + +/** + * Holds if the `importingModule` extends the `importedModule`, imported via path `importPath`. + */ +private predicate importerExtendsImportee( + SapDefineModule importingModule, SapDefineModule importedModule, string importPath +) { + importerAndImportee(importingModule, importedModule, importPath) and + importingModule + .getRequiredObject(importPath) + .asSourceNode() + .flowsTo(importingModule.getExtendCall().getReceiver()) +} + class JQuerySap extends DataFlow::SourceNode { JQuerySap() { exists(DataFlow::GlobalVarRefNode global | @@ -738,6 +791,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 @@ -1118,6 +1176,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. */ @@ -1175,6 +1243,8 @@ class XmlModel extends UI5InternalModel { } override string getPathString() { result = "TODO" } + + override predicate contentIsStaticallyVisible() { exists(this.getPathString()) } } class ResourceModel extends UI5Model, ModelReference { 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 0a309b2cc..4b92412d9 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. */ From 1ca3c1c7b28eae6561053ace13f8dca0f73e434d Mon Sep 17 00:00:00 2001 From: Jeongsoo Lee Date: Tue, 21 Oct 2025 15:24:28 -0400 Subject: [PATCH 2/7] Turn draft predicates into class member predicates --- .../javascript/frameworks/ui5/UI5.qll | 76 +++++++++---------- 1 file changed, 35 insertions(+), 41 deletions(-) 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 b08d1984d..11a0050e6 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 @@ -204,52 +204,46 @@ class SapDefineModule extends AmdModuleDefinition::Range, MethodCallExpr, UserMo // ) none() // TODO } -} -/** - * Holds if the `importingModule` "imports" the `importedModule` via path `importPath`. - */ -private predicate importerAndImportee( - SapDefineModule importingModule, SapDefineModule importedModule, string importPath -) { - /* 1. Absolute import paths: We resolve this ourselves. */ - exists(string importedModuleDefinitionPath, string importedModuleDefinitionPathSlashNormalized | + /** + * 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 /* - * 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`. + * 2. Relative import paths: We delegate the heaving lifting of resolving to + * `Import.resolveImportedPath/0`. */ - importPath = importingModule.asModule().getAnImport().getImportedPathExpr().getStringValue() and - importedModuleDefinitionPath = importedModule.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_ = importingModule.asModule().getAnImport() and - import_.resolveImportedPath() = importedModule.getTopLevel() - ) -} + 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`. - */ -private predicate importerExtendsImportee( - SapDefineModule importingModule, SapDefineModule importedModule, string importPath -) { - importerAndImportee(importingModule, importedModule, importPath) and - importingModule - .getRequiredObject(importPath) - .asSourceNode() - .flowsTo(importingModule.getExtendCall().getReceiver()) + /** + * 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()) + } } class JQuerySap extends DataFlow::SourceNode { From 4931118e55faa5eddbb34f4feb43fc2dd64c0b7c Mon Sep 17 00:00:00 2001 From: Jeongsoo Lee Date: Thu, 23 Oct 2025 13:43:36 -0400 Subject: [PATCH 3/7] Change CustomControl and CustomController to use the new version of `getExtendingModule` --- .../javascript/frameworks/ui5/UI5.qll | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) 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 11a0050e6..5f70d0369 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 @@ -193,17 +193,7 @@ class SapDefineModule extends AmdModuleDefinition::Range, MethodCallExpr, UserMo /** * 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()) - // ) - none() // TODO - } + SapDefineModule getExtendingModule() { result.getSuperModule(_) = this } /** * Gets the module that this module imports via path `importPath`. @@ -291,7 +281,9 @@ class CustomControl extends SapExtendCall { CustomControl() { this.getReceiver().getALocalSource() = TypeTrackers::hasDependency(["sap/ui/core/Control", "sap.ui.core.Control"]) or - exists(SapDefineModule sapModule | this.getDefine() = sapModule.getExtendingModule()) + exists(CustomControl superControl | + superControl.getDefine() = this.getDefine().getSuperModule(_) + ) } CustomController getController() { this = result.getAControlReference().getDefinition() } @@ -464,8 +456,14 @@ class CustomController extends SapExtendCall { string name; CustomController() { - this.getReceiver().getALocalSource() = - TypeTrackers::hasDependency(["sap/ui/core/mvc/Controller", "sap.ui.core.mvc.Controller"]) and + ( + this.getReceiver().getALocalSource() = + TypeTrackers::hasDependency(["sap/ui/core/mvc/Controller", "sap.ui.core.mvc.Controller"]) + or + exists(CustomController superController | + superController.getDefine() = this.getDefine().getSuperModule(_) + ) + ) and name = this.getFile().getBaseName().regexpCapture("([a-zA-Z0-9]+).[cC]ontroller.js", 1) } From e16380102c48225f50854d6930c57d1cc3e29e73 Mon Sep 17 00:00:00 2001 From: Jeongsoo Lee Date: Fri, 31 Oct 2025 15:58:29 -0400 Subject: [PATCH 4/7] Checkpoint --- .../javascript/frameworks/ui5/UI5.qll | 107 +++++++++++++----- 1 file changed, 79 insertions(+), 28 deletions(-) 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 5f70d0369..efa91c6ba 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 @@ -468,23 +469,17 @@ class CustomController extends SapExtendCall { } 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() { this.getAThisNode() = result.getReceiver() and result.getMethodName() = "getOwnerComponent" + or + exists(CustomController baseController | + baseController.getDefine() = this.getDefine().getSuperModule(_) and + result = baseController.getOwnerComponentRef() + ) } /** @@ -811,7 +806,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() } @@ -833,7 +828,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(_) } @@ -862,11 +857,50 @@ 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.getName() = 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" } } @@ -995,7 +1029,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 + } } } @@ -1316,6 +1362,10 @@ class SapExtendCall extends InvokeNode, MethodCallNode { string getName() { result = this.getArgument(0).asExpr().(StringLiteral).getValue() } + string getModuleName() { + result = getSubstringAfterLastOccurrenceOfCharacter(this.getName(), ".") + } + ObjectLiteralNode getContent() { result = this.getArgument(1) } Metadata getMetadata() { @@ -1510,18 +1560,19 @@ 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) +} + +bindingset[input, character] +private string getSubstringAfterLastOccurrenceOfCharacter(string input, string character) { + result = input.splitAt(character, countCharacterInString(input, character)) +} - SourceNode hasDependency(string dependencyPath) { - result = hasDependency(TypeTracker::end(), dependencyPath) +private module Notebook { + MethodCallNode test1(CustomController controller) { + controller.getModuleName() = "EffortDriver" and + result = controller.getOwnerComponentRef() } } From 8e059c414229d6d25b4a8827047011ec68efbdd2 Mon Sep 17 00:00:00 2001 From: Jeongsoo Lee Date: Thu, 20 Nov 2025 15:45:37 -0500 Subject: [PATCH 5/7] Minor code reflow --- .../advanced_security/javascript/frameworks/ui5/UI5.qll | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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 2add5c3df..62a437ebb 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 @@ -286,7 +286,8 @@ class Renderer extends SapExtendCall { class CustomControl extends SapExtendCall { CustomControl() { this.getReceiver().getALocalSource() = - TypeTrackers::hasDependency(["sap/ui/core/Control", "sap.ui.core.Control"]).getAMemberCall("extend") or + TypeTrackers::hasDependency(["sap/ui/core/Control", "sap.ui.core.Control"]) + .getAMemberCall("extend") or exists(CustomControl superControl | superControl.getDefine() = this.getDefine().getSuperModule(_) ) @@ -494,7 +495,7 @@ class CustomController extends SapExtendCall { ( this.getReceiver().getALocalSource() = TypeTrackers::hasDependency(["sap/ui/core/mvc/Controller", "sap.ui.core.mvc.Controller"]) - .getAMemberCall("extend") + .getAMemberCall("extend") or exists(CustomController superController | superController.getDefine() = this.getDefine().getSuperModule(_) @@ -894,7 +895,8 @@ module ManifestJson { * Gets the view this target is associated with. */ UI5View getView() { - result.getName() = getSubstringAfterLastOccurrenceOfCharacter(this.getViewName(), "/") + result.getControllerName() = + getSubstringAfterLastOccurrenceOfCharacter(this.getViewName(), "/") } /** From 445b5732d7fc55f83ae748613ced43662cdaf0fc Mon Sep 17 00:00:00 2001 From: Jeongsoo Lee Date: Thu, 20 Nov 2025 16:21:29 -0500 Subject: [PATCH 6/7] Debug and move type trackers to a new file --- .../javascript/frameworks/ui5/UI5.qll | 13 +--- .../frameworks/ui5/dataflow/TypeTrackers.qll | 64 +++++++++++++++++++ 2 files changed, 67 insertions(+), 10 deletions(-) create mode 100644 javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/TypeTrackers.qll 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 62a437ebb..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 @@ -285,7 +285,7 @@ class Renderer extends SapExtendCall { class CustomControl extends SapExtendCall { CustomControl() { - this.getReceiver().getALocalSource() = + this = TypeTrackers::hasDependency(["sap/ui/core/Control", "sap.ui.core.Control"]) .getAMemberCall("extend") or exists(CustomControl superControl | @@ -493,7 +493,7 @@ class CustomController extends SapExtendCall { CustomController() { ( - this.getReceiver().getALocalSource() = + this = TypeTrackers::hasDependency(["sap/ui/core/mvc/Controller", "sap.ui.core.mvc.Controller"]) .getAMemberCall("extend") or @@ -895,7 +895,7 @@ module ManifestJson { * Gets the view this target is associated with. */ UI5View getView() { - result.getControllerName() = + result.getController().getModuleName() = getSubstringAfterLastOccurrenceOfCharacter(this.getViewName(), "/") } @@ -1550,10 +1550,3 @@ bindingset[input, character] private string getSubstringAfterLastOccurrenceOfCharacter(string input, string character) { result = input.splitAt(character, countCharacterInString(input, character)) } - -private module Notebook { - MethodCallNode test1(CustomController controller) { - controller.getModuleName() = "EffortDriver" and - result = controller.getOwnerComponentRef() - } -} 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) + } +} From 1a52a1e5e384525d1d84bec598cc852fc1b7dce0 Mon Sep 17 00:00:00 2001 From: Jeongsoo Lee Date: Mon, 24 Nov 2025 18:11:08 -0500 Subject: [PATCH 7/7] Relax assumption of ODataServiceModel and add a flow step --- .../frameworks/ui5/RemoteFlowSources.qll | 37 +++++-------------- .../frameworks/ui5/dataflow/FlowSteps.qll | 26 +++++++++++++ 2 files changed, 35 insertions(+), 28 deletions(-) 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/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() + ) + } +}