From 19e9deb998f4028e9e1237028b009db868398475 Mon Sep 17 00:00:00 2001 From: Jeongsoo Lee Date: Tue, 21 Oct 2025 15:14:41 -0400 Subject: [PATCH 1/3] 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/3] 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/3] 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) }