From 01fc884265b1124182fee74d28fd182cb773123c Mon Sep 17 00:00:00 2001 From: Mauro Baluda Date: Thu, 5 Mar 2026 20:26:34 +0100 Subject: [PATCH 1/4] Refactor UI5 dataflow imports and rename UI5DataFlow module --- javascript/frameworks/ui5/ext/ui5.model.yml | 1 - .../frameworks/ui5/RemoteFlowSources.qll | 11 - .../javascript/frameworks/ui5/UI5Control.qll | 297 +++++++++++++++++ .../ui5/UI5FormulaInjectionQuery.qll | 2 +- .../frameworks/ui5/UI5LogInjectionQuery.qll | 2 +- .../frameworks/ui5/UI5LogsToHttpQuery.qll | 2 +- .../ui5/UI5UnsafeLogAccessQuery.qll | 2 +- .../javascript/frameworks/ui5/UI5View.qll | 299 +----------------- .../javascript/frameworks/ui5/UI5XssQuery.qll | 2 +- .../frameworks/ui5/dataflow/TypeTrackers.qll | 2 +- .../{DataFlow.qll => UI5DataFlow.qll} | 14 + .../ui5/src/Diagnostics/ListHtmlSinks.ql | 2 +- .../src/Diagnostics/ListRemoteFlowSources.ql | 2 +- .../UI5FormulaInjection.ql | 1 - .../src/UI5LogInjection/UI5LogInjection.ql | 1 - .../ui5/src/UI5LogInjection/UI5LogsToHttp.ql | 1 - .../src/UI5LogInjection/UI5UnsafeLogAccess.ql | 1 - .../src/UI5PathInjection/UI5PathInjection.ql | 2 +- .../frameworks/ui5/src/UI5Xss/UI5Xss.ql | 2 +- .../ui5/test/models/sink/formulaSinkTest.ql | 2 +- .../ui5/test/models/sink/pathSinkTest.ql | 2 +- .../ui5/test/models/sink/xssSinkTest.ql | 2 +- .../UI5Xss.expected | 1 - 23 files changed, 326 insertions(+), 327 deletions(-) create mode 100644 javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5Control.qll rename javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/{DataFlow.qll => UI5DataFlow.qll} (96%) diff --git a/javascript/frameworks/ui5/ext/ui5.model.yml b/javascript/frameworks/ui5/ext/ui5.model.yml index 4112a1cab..a17302ede 100644 --- a/javascript/frameworks/ui5/ext/ui5.model.yml +++ b/javascript/frameworks/ui5/ext/ui5.model.yml @@ -115,7 +115,6 @@ extensions: data: - ["UI5InputControl", "Member[value]", "remote"] - ["UI5InputControl", "Member[getValue].ReturnValue", "remote"] - - ["UI5HTMLControl", "Member[getContent].ReturnValue", "remote"] - ["UI5CodeEditor", "Member[value]", "remote"] - ["UI5CodeEditor", "Member[getCurrentValue].ReturnValue", "remote"] - ["global", "Member[jQuery].Member[sap].Member[syncHead,syncGet,syncGetText,syncPost,syncPostText].ReturnValue", "remote"] 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 b3d38669b..e78632936 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 @@ -203,14 +203,3 @@ private class DisplayEventHandlerParameterAccess extends RemoteFlowSource instan ) } } - -/** - * Method calls that fetch a piece of data either from a library control capable of accepting user input, or from a URI parameter. - */ -private class UI5ExtRemoteSource extends RemoteFlowSource { - UI5ExtRemoteSource() { this = ModelOutput::getASourceNode("remote").asSource() } - - override string getSourceType() { - result = "Remote flow" // Don't discriminate between UI5-specific remote flows and vanilla ones - } -} diff --git a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5Control.qll b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5Control.qll new file mode 100644 index 000000000..81cc9d8b5 --- /dev/null +++ b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5Control.qll @@ -0,0 +1,297 @@ +import advanced_security.javascript.frameworks.ui5.UI5 + +private newtype TUI5Control = + TXmlControl(XmlElement control) { + control + .(Locatable) + .getFile() + .getBaseName() + .matches(["%.view.xml", "%.view.html", "%.fragment.xml"]) + } or + TJsonControl(JsonObject control) { + exists(JsonView view | control.getParent() = view.getRoot().getPropValue("content")) + } or + TJsControl(NewNode control) { + exists(JsView view | + control.asExpr().getParentExpr() = + view.getRoot() + .getArgument(1) + .getALocalSource() + .(ObjectLiteralNode) + .getAPropertyWrite("createContent") + .getRhs() + .(FunctionNode) + .getReturnNode() + .getALocalSource() + .(ArrayLiteralNode) + .asExpr() + ) + or + control = ModelOutput::getATypeNode("Control").getAnInvocation() + } + +class UI5Control extends TUI5Control { + XmlElement asXmlControl() { this = TXmlControl(result) } + + JsonObject asJsonControl() { this = TJsonControl(result) } + + NewNode asJsControl() { this = TJsControl(result) } + + string toString() { + result = this.asXmlControl().toString() + or + result = this.asJsonControl().toString() + or + result = this.asJsControl().toString() + } + + predicate hasLocationInfo( + string filepath, int startcolumn, int startline, int endcolumn, int endline + ) { + this.asXmlControl().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) + or + /* Since JsonValue does not implement `hasLocationInfo`, we use `getLocation` instead. */ + exists(Location location | location = this.asJsonControl().getLocation() | + location.getFile().getAbsolutePath() = filepath and + location.getStartColumn() = startcolumn and + location.getStartLine() = startline and + location.getEndColumn() = endcolumn and + location.getEndLine() = endline + ) + or + this.asJsControl().hasLocationInfo(filepath, startcolumn, startline, endcolumn, endline) + } + + /** + * Gets the qualified type string, e.g. `sap.m.SearchField`. + */ + string getQualifiedType() { + exists(XmlElement control | control = this.asXmlControl() | + result = control.getNamespace().getUri() + "." + control.getName() + ) + or + exists(JsonObject control | control = this.asJsonControl() | + result = control.getPropStringValue("Type") + ) + or + exists(NewNode control | control = this.asJsControl() | + result = control.asExpr().getAChildExpr().(PropAccess).getQualifiedName() + or + control = API::moduleImport(result).getAnInvocation() + ) + } + + File getFile() { + result = this.asXmlControl().getFile() or + result = this.asJsonControl().getFile() or + result = this.asJsControl().getFile() + } + + /** + * Gets the `id` property of this control. + */ + string getId() { result = this.getProperty("id").getValue() } + + /** + * Gets the qualified type name, e.g. `sap/m/SearchField`. + */ + string getImportPath() { result = this.getQualifiedType().replaceAll(".", "/") } + + /** + * Gets the definition of this control if this is a custom one. + */ + CustomControl getDefinition() { + result.getName() = this.getQualifiedType() and + inSameWebApp(this.getFile(), result.getFile()) + } + + /** + * Gets a reference to this control. Currently supports only such references made through `byId`. + * Handles both: + * - `this.byId("controlId")` or `this.getView().byId("controlId")` - ID in argument 0 + * - `Fragment.byId(viewId, "controlId")` - ID in argument 1 + */ + ControlReference getAReference() { + result.getMethodName() = "byId" and + ( + // Standard byId: ID in first argument + result.getNumArgument() = 1 and + result.getArgument(0).getALocalSource().asExpr().(StringLiteral).getValue() = + this.getProperty("id").getValue() + or + // Fragment.byId: ID in second argument + result.getNumArgument() = 2 and + result.getArgument(1).getALocalSource().asExpr().(StringLiteral).getValue() = + this.getProperty("id").getValue() + ) + } + + /** Gets a property of this control having the name. */ + UI5ControlProperty getProperty(string propName) { + result.asXmlControlProperty() = this.asXmlControl().getAttribute(propName) + or + result.asJsonControlProperty() = this.asJsonControl().getPropValue(propName) + or + result.asJsControlProperty() = + this.asJsControl() + .getArgument(0) + .getALocalSource() + .asExpr() + .(ObjectExpr) + .getPropertyByName(propName) + .getAChildExpr() + .flow() and + not exists(Property property | result.asJsControlProperty() = property.getNameExpr().flow()) + } + + /** Gets a property of this control. */ + UI5ControlProperty getAProperty() { result = this.getProperty(_) } + + bindingset[propName] + MethodCallNode getARead(string propName) { + // TODO: in same view + inSameWebApp(this.getFile(), result.getFile()) and + result.getMethodName() = "get" + capitalize(propName) + } + + bindingset[propName] + MethodCallNode getAWrite(string propName) { + // TODO: in same view + inSameWebApp(this.getFile(), result.getFile()) and + result.getMethodName() = "set" + capitalize(propName) + } + + /** Holds if this control reads from or writes to a model. */ + predicate accessesModel(UI5Model model) { this.accessesModel(model, _) } + + /** Holds if this control reads from or writes to a model with regards to a binding path. */ + predicate accessesModel(UI5Model model, XmlBindingPath bindingPath) { + // Verify that the controller's model has the referenced property + exists(XmlView view | + // Both this control and the model belong to the same view + this = view.getControl() and + model = view.getController().getModel() and + model.(UI5InternalModel).getPathString() = bindingPath.getPath() and + bindingPath.getBindingTarget() = this.asXmlControl().getAnAttribute() + ) + } + + /** Get the view that this control is part of. */ + UI5View getView() { result = this.asXmlControl().getFile() } + + /** Get the controller that manages this control. */ + CustomController getController() { result = this.getView().getController() } + + /** + * Gets the full import path of the associated control. + */ + string getControlTypeName() { result = this.getQualifiedType().replaceAll(".", "/") } + + /** + * Holds if the control content is sanitized for HTML + * 'sap/ui/core/HTML' sanitized using the property 'sanitizeContent' + * 'sap/ui/richttexteditor/RichTextEditor' sanitized using the property 'sanitizeValue' + */ + predicate isHTMLSanitized() { + this.getControlTypeName() = "sap/ui/richtexteditor/RichTextEditor" and + not this.isSanitizePropertySetTo("sanitizeValue", false) + or + this.getControlTypeName() = "sap/ui/core/HTML" and + this.isSanitizePropertySetTo("sanitizeContent", true) and + not this.isSanitizePropertySetTo("sanitizeContent", false) + } + + bindingset[propName, val] + private predicate isSanitizePropertySetTo(string propName, boolean val) { + /* 1. `sanitizeContent` attribute is set declaratively. */ + this.getProperty(propName).toString() = val.toString() + or + /* 2. `sanitizeContent` attribute is set programmatically using setProperty(). */ + exists(CallNode node | node = this.getAReference().getAMemberCall("setProperty") | + node.getArgument(0).getStringValue() = propName and + not node.getArgument(1).mayHaveBooleanValue(val.booleanNot()) + ) + or + /* 3. `sanitizeContent` attribute is set programmatically using a setter. */ + exists(CallNode node, string setterName | + setterName = "set" + propName.prefix(1).toUpperCase() + propName.suffix(1) and + not node.getArgument(0).mayHaveBooleanValue(val.booleanNot()) + | + node = this.getAReference().getAMemberCall(setterName) or + node = this.asJsControl().getAMemberCall(setterName) + ) + } +} + +private newtype TUI5ControlProperty = + TXmlControlProperty(XmlAttribute property) or + TJsonControlProperty(JsonValue property) or + TJsControlProperty(ValueNode property) + +class UI5ControlProperty extends TUI5ControlProperty { + XmlAttribute asXmlControlProperty() { this = TXmlControlProperty(result) } + + JsonValue asJsonControlProperty() { this = TJsonControlProperty(result) } + + ValueNode asJsControlProperty() { this = TJsControlProperty(result) } + + string toString() { + result = this.asXmlControlProperty().getValue().toString() or + result = this.asJsonControlProperty().toString() or + result = this.asJsControlProperty().toString() + } + + UI5Control getControl() { + result.asXmlControl() = this.asXmlControlProperty().getElement() or + result.asJsonControl() = this.asJsonControlProperty().getParent() or + result.asJsControl().getArgument(0).asExpr() = this.asJsControlProperty().getEnclosingExpr() + } + + string getName() { + result = this.asXmlControlProperty().getName() + or + exists(JsonValue parent | parent.getPropValue(result) = this.asJsonControlProperty()) + or + exists(Property property | + property.getAChildExpr() = this.asJsControlProperty().asExpr() and result = property.getName() + ) + } + + string getValue() { + result = this.asXmlControlProperty().getValue() or + result = this.asJsonControlProperty().getStringValue() or + result = this.asJsControlProperty().asExpr().(StringLiteral).getValue() + } +} + +/** + * Utility predicate capturing the handler name. + */ +bindingset[notation] +private string handlerNotationCaptureName(string notation) { + result = + notation.replaceAll(" ", "").regexpCapture("\\.([\\w-]+)(?:\\([^)]*\\$(\\{[^}]+}).*)?", 1) +} + +/** + * A function mentioned in a property of a UI5Control, usually an event handler. + * + * e.g. The function referred to by `doSomething()` as in `