Skip to content

Commit 2dbaf58

Browse files
rvermeulenrvermeulen
committed
Replace Project with WebApp
Project resolution relied on a property that doesn't hold for all SAP UI5 projects. The WebApp approach follows the bootstrapping process to determine the definition of a webapp and uses the resource roots to resolve resources beloning to a webapp.
1 parent a9a885e commit 2dbaf58

File tree

5 files changed

+64
-53
lines changed

5 files changed

+64
-53
lines changed

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

Lines changed: 44 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,6 @@ private import semmle.javascript.security.dataflow.DomBasedXssCustomizations
55
private import advanced_security.javascript.frameworks.ui5.UI5View
66

77
module UI5 {
8-
/**
9-
* Helper predicate checking if two elements are in the same Project
10-
*/
11-
predicate inSameUI5Project(File f1, File f2) {
12-
exists(Project p | p.isInThisProject(f1) and p.isInThisProject(f2))
13-
}
148

159
bindingset[this]
1610
private class JsonStringReader extends string {
@@ -91,30 +85,49 @@ module UI5 {
9185
}
9286
}
9387

94-
class Project extends Folder {
95-
/**
96-
* An UI5 project root folder.
97-
*/
98-
Project() { exists(File yamlFile | yamlFile = this.getFile("ui5.yaml")) }
88+
/** A UI5 web application manifest associated with a bootstrapped UI5 web application. */
89+
class WebAppManifest extends File {
90+
WebApp webapp;
91+
WebAppManifest() {
92+
this.getBaseName() = "manifest.json" and
93+
this.getParentContainer() = webapp.getWebAppFolder()
94+
}
9995

100-
/**
101-
* The `ui5.yaml` file that declares a UI5 application.
102-
*/
103-
File getProjectYaml() { result = this.getFile("ui5.yaml") }
96+
WebApp getWebapp() { result = webapp }
97+
}
10498

105-
predicate isInThisProject(File file) { this = file.getParentContainer*() }
99+
/** A UI5 bootstrapped web application. */
100+
class WebApp extends HTML::HtmlFile {
101+
SapUiCoreScript coreScript;
106102

107-
private HTML::HtmlFile getSapUICoreScript() {
108-
exists(HTML::ScriptElement script |
109-
result = script.getFile() and
110-
this.isInThisProject(result) and
111-
script.getSourcePath().matches("%/sap-ui-core.js")
112-
)
103+
WebApp() {
104+
coreScript.getFile() = this
113105
}
114106

115-
HTML::HtmlFile getMainHTML() { result = this.getSapUICoreScript() }
116-
}
107+
File getAResource() {
108+
coreScript.getAResolvedResourceRoot().contains(result)
109+
}
110+
111+
File getResource(string path) {
112+
getWebAppFolder().getAbsolutePath() + "/" + path = result.getAbsolutePath()
113+
}
117114

115+
Folder getWebAppFolder() {
116+
result = this.getParentContainer()
117+
}
118+
119+
WebAppManifest getManifest() {
120+
result.getWebapp() = this
121+
}
122+
123+
File getInitialModule() {
124+
exists(string initialModuleResourcePath, string resolvedModulePath, ResolvedResourceRoot resourceRoot |
125+
initialModuleResourcePath = coreScript.getAttributeByName("data-sap-ui-onInit").getValue() and coreScript.getAResolvedResourceRoot() = resourceRoot and
126+
resolvedModulePath = initialModuleResourcePath.regexpReplaceAll("^module\\s*:\\s*", "").replaceAll(resourceRoot.getName(), resourceRoot.getRoot().getAbsolutePath()) and
127+
result.getAbsolutePath() = resolvedModulePath + ".js"
128+
)
129+
}
130+
}
118131
/**
119132
* https://sapui5.hana.ondemand.com/sdk/#/api/sap.ui.loader%23methods/sap.ui.loader.config
120133
*/
@@ -156,7 +169,9 @@ module UI5 {
156169
)
157170
}
158171

159-
Project getProject() { result = this.getFile().getParentContainer*() }
172+
WebApp getWebApp() {
173+
this.getFile() = result.getAResource()
174+
}
160175

161176
SapDefineModule getExtendingDefine() {
162177
exists(Extension baseExtension, Extension subclassExtension, SapDefineModule subclassDefine |
@@ -371,12 +386,8 @@ module UI5 {
371386
*/
372387
bindingset[path]
373388
JsonObject resolveDirectPath(string path) {
374-
exists(Project project, File jsonFile |
375-
// project contains this file
376-
project.isInThisProject(jsonFile) and
377-
jsonFile.getExtension() = "json" and
378-
jsonFile.getAbsolutePath() = project.getASubFolder().getAbsolutePath() + "/" + path and
379-
result.getJsonFile() = jsonFile
389+
exists(WebApp webApp|
390+
result.getJsonFile() = webApp.getResource(path)
380391
)
381392
}
382393

@@ -582,14 +593,14 @@ module UI5 {
582593
result.getMethodName() = "setProperty" and
583594
result.getArgument(0).asExpr().(StringLiteral).getValue() = propName and
584595
// TODO: in same controller
585-
inSameUI5Project(this.getFile(), result.getFile())
596+
exists(WebApp webApp | webApp.getAResource() = this.getFile() and webApp.getAResource() = result.getFile())
586597
}
587598

588599
bindingset[propName]
589600
MethodCallNode getARead(string propName) {
590601
result.getMethodName() = "get" + capitalize(propName) and
591602
// TODO: in same controller
592-
inSameUI5Project(this.getFile(), result.getFile())
603+
exists(WebApp webApp | webApp.getAResource() = this.getFile() and webApp.getAResource() = result.getFile())
593604
}
594605
}
595606
}

‎javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5DataFlow.qll‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ module UI5DataFlow {
1212
private predicate bidiModelControl(DataFlow::Node start, DataFlow::Node end) {
1313
exists(DataFlow::SourceNode property, Metadata metadata, UI5BoundNode node |
1414
// same project
15-
inSameUI5Project(metadata.getFile(), node.getFile()) and
15+
exists(WebApp webApp | webApp.getAResource() = metadata.getFile() and webApp.getAResource() = node.getFile()) and
1616
(
1717
// same control
1818
metadata.getControl().getName() = node.getBindingPath().getControlQualifiedType()
@@ -87,22 +87,22 @@ module UI5DataFlow {
8787
UI5BindingPath getBindingPath() { result = bindingPath }
8888

8989
UI5BoundNode() {
90+
exists(WebApp webApp | webApp.getAResource() = this.getFile() and
91+
webApp.getAResource() = bindingPath.getFile() |
9092
/* The relevant portion of the content of a JSONModel */
9193
exists(Property p, JsonModel model |
9294
// The property bound to an UI5View source
9395
this.(DataFlow::PropRef).getPropertyNameExpr() = p.getNameExpr() and
9496
// The binding path refers to this model
95-
bindingPath.getAbsolutePath() = model.getPathString(p) and
96-
inSameUI5Project(this.getFile(), bindingPath.getFile())
97+
bindingPath.getAbsolutePath() = model.getPathString(p)
9798
)
9899
or
99100
/* The URI string to the JSONModel constructor call */
100101
exists(JsonModel model |
101102
this = model.getArgument(0) and
102103
this.asExpr() instanceof StringLiteral and
103-
bindingPath.getAbsolutePath() = model.getPathString() and
104-
inSameUI5Project(this.getFile(), bindingPath.getFile())
105-
)
104+
bindingPath.getAbsolutePath() = model.getPathString()
105+
))
106106
}
107107
}
108108

‎javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5HTML.qll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,8 @@ class FrameOptions extends TFrameOptions {
8787
/**
8888
* Holds if the frame options are left untouched as the default value `trusted`.
8989
*/
90-
predicate thereIsNoFrameOptionSet(UI5::Project p) {
91-
not exists(FrameOptions frameOptions | p.isInThisProject(frameOptions.getLocation().getFile()) |
90+
predicate thereIsNoFrameOptionSet(UI5::WebApp webapp) {
91+
not exists(FrameOptions frameOptions | webapp.getAResource() = frameOptions.getLocation().getFile() |
9292
frameOptions.allowsSharedOriginEmbedding() or
9393
frameOptions.deniesEmbedding() or
9494
frameOptions.allowsAllOriginEmbedding()

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ abstract class UI5BindingPath extends Locatable {
9898
// The property bound to an UI5View source
9999
result.getPropertyNameExpr() = p.getNameExpr() and
100100
this.getAbsolutePath() = model.getPathString(p) and
101-
//restrict search inside the same project
102-
inSameUI5Project(this.getFile(), result.getFile())
101+
//restrict search inside the same webapp
102+
exists(WebApp webApp | webApp.getAResource() = this.getFile() and webApp.getAResource() = result.getFile())
103103
)
104104
// TODO
105105
/*
@@ -142,8 +142,8 @@ abstract class UI5View extends File {
142142
CustomController getController() {
143143
// The controller name should match
144144
result.getName() = this.getControllerName() and
145-
// The View and the Controller are in a same project
146-
inSameUI5Project(this, result.getFile())
145+
// The View and the Controller are in a same webapp
146+
exists(WebApp webApp | webApp.getAResource() = this and webApp.getAResource() = result.getFile())
147147
}
148148

149149
abstract UI5BindingPath getASource();
@@ -490,10 +490,10 @@ class XmlView extends UI5View, XmlFile {
490490
(
491491
builtInControl(element.getNamespace())
492492
or
493-
// or a custom control with implementation code found in the project
493+
// or a custom control with implementation code found in the webapp
494494
exists(CustomControl control |
495495
control.getName() = element.getNamespace().getUri() + "." + element.getName() and
496-
inSameUI5Project(control.getFile(), element.getFile())
496+
exists(WebApp webApp | webApp.getAResource() = control.getFile() and webApp.getAResource() = element.getFile())
497497
)
498498
)
499499
)
@@ -563,20 +563,20 @@ class XmlControl extends UI5Control instanceof XmlElement {
563563

564564
override CustomControl getDefinition() {
565565
result.getName() = this.getQualifiedType() and
566-
inSameUI5Project(this.getFile(), result.getFile())
566+
exists(WebApp webApp | webApp.getAResource() = this.getFile() and webApp.getAResource() = result.getFile())
567567
}
568568

569569
bindingset[propName]
570570
override MethodCallNode getARead(string propName) {
571571
// TODO: in same view
572-
inSameUI5Project(this.getFile(), result.getFile()) and
572+
exists(WebApp webApp | webApp.getAResource() = this.getFile() and webApp.getAResource() = result.getFile()) and
573573
result.getMethodName() = "get" + capitalize(propName)
574574
}
575575

576576
bindingset[propName]
577577
override MethodCallNode getAWrite(string propName) {
578578
// TODO: in same view
579-
inSameUI5Project(this.getFile(), result.getFile()) and
579+
exists(WebApp webApp | webApp.getAResource() = this.getFile() and webApp.getAResource() = result.getFile()) and
580580
result.getMethodName() = "set" + capitalize(propName)
581581
}
582582

‎javascript/frameworks/ui5/src/UI5Clickjacking/UI5Clickjacking.ql‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ private import advanced_security.javascript.frameworks.ui5.UI5
1717

1818
class FirstLineOfMainHtml extends HTML::DocumentElement, FirstLineOf {
1919
FirstLineOfMainHtml() {
20-
exists(UI5::Project p | this.getFile().(FirstLineOf).getFile() = p.getMainHTML())
20+
exists(UI5::WebApp app | this.getFile().(FirstLineOf).getFile() = app)
2121
}
2222
}
2323

@@ -51,8 +51,8 @@ where
5151
" being set to `allow`."
5252
)
5353
or
54-
exists(UI5::Project p | thereIsNoFrameOptionSet(p) |
55-
alert.asFirstLineOfMainHtml().getFile() = p.getMainHTML() and
54+
exists(UI5::WebApp app | thereIsNoFrameOptionSet(app) |
55+
alert.asFirstLineOfMainHtml().getFile() = app and
5656
message = "Possible clickjacking vulnerability due to missing frame options."
5757
)
5858
select alert, message

0 commit comments

Comments
 (0)