Merge pull request #278 from advanced-security/mbaluda/get-sanitized-content

data-douser · web-flow · commit 7e2b76c0fcb2 · 2026-01-29T09:52:48.000-07:00
Model DF edge for sanitized HTML setContent-&gt;getContent
diff --git a/.github/workflows/javascript.sarif.expected b/.github/workflows/javascript.sarif.expected
diff --git a/javascript/frameworks/ui5/ext/ui5.model.yml b/javascript/frameworks/ui5/ext/ui5.model.yml
@@ -6,6 +6,9 @@ extensions:
       - ["SapUICoreInstance", "global", "Member[sap].Member[ui].Member[getCore].ReturnValue"]
       - ["Control", "Control", "Instance"]
       - ["Control", "sap/ui/core/Control", ""]
+      - ["Control", "UI5HTMLControl", ""]
+      - ["Control", "UI5InputControl", ""]
+      - ["Control", "CustomControl", ""]
       - ["Control", "global", "Member[sap].Member[ui].Member[core].Member[Control]"]
       - ["Controller", "Controller", "Instance"]
       - ["Controller", "sap/ui/core/mvc/Controller", ""]
@@ -112,6 +115,7 @@ 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/UI5View.qll b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5View.qll
@@ -805,6 +805,8 @@ private newtype TUI5Control =
             .(ArrayLiteralNode)
             .asExpr()
     )
+    or
+    control = ModelOutput::getATypeNode("Control").getAnInvocation()
   }
 
 class UI5Control extends TUI5Control {
@@ -852,7 +854,9 @@ class UI5Control extends TUI5Control {
     )
     or
     exists(NewNode control | control = this.asJsControl() |
-      result = this.asJsControl().asExpr().getAChildExpr().(DotExpr).getQualifiedName()
+      result = control.asExpr().getAChildExpr().(PropAccess).getQualifiedName()
+      or
+      control = API::moduleImport(result).getAnInvocation()
     )
   }
 
@@ -988,9 +992,12 @@ class UI5Control extends TUI5Control {
     )
     or
     /* 3. `sanitizeContent` attribute is set programmatically using a setter. */
-    exists(CallNode node |
-      node = this.getAReference().getAMemberCall("setS" + propName.suffix(1)) and
+    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)
     )
   }
 }
diff --git a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5XssQuery.qll b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5XssQuery.qll
@@ -31,6 +31,14 @@ module UI5Xss implements DataFlow::ConfigSig {
     node.(DataFlow::CallNode).getReceiver().asExpr().(PropAccess).getQualifiedName() = "jQuery.sap" and
     node.(DataFlow::CallNode).getCalleeName() =
       ["encodeCSS", "encodeJS", "encodeURL", "encodeURLParameters", "encodeXML", "encodeHTML"]
+    or
+    /* Block flow through setContent/getContent of a sanitized UI5Control */
+    exists(UI5Control control, DataFlow::MethodCallNode content |
+      control.asJsControl() = content.getReceiver().getALocalSource() and
+      control.isHTMLSanitized() and
+      content.getMethodName() = ["setContent", "getContent"] and
+      node = [content, content.getArgument(0)]
+    )
   }
 
   predicate isSink(DataFlow::Node node) {
@@ -56,6 +64,19 @@ module UI5Xss implements DataFlow::ConfigSig {
 
       end = h.getParameter(0)
     )
+    or
+    /* Flow from `setContent` to `getContent` of a control */
+    exists(
+      UI5Control control, DataFlow::MethodCallNode getContent, DataFlow::MethodCallNode setContent
+    |
+      control.asJsControl() = setContent.getReceiver().getALocalSource() and
+      control.asJsControl() = getContent.getReceiver().getALocalSource() and
+      setContent.getMethodName() = "setContent" and
+      getContent.getMethodName() = "getContent" and
+      start = setContent.getArgument(0) and
+      end = getContent and
+      not control.isHTMLSanitized()
+    )
   }
 }
 
@@ -93,10 +114,6 @@ private class UI5ExtHtmlISink extends DataFlow::Node {
   }
 }
 
-private class HTMLControlInstantiation extends ElementInstantiation {
-  HTMLControlInstantiation() { typeModel("UI5HTMLControl", this.getImportPath(), _) }
-}
-
 private module TrackPlaceAtCallConfigFlow = TaintTracking::Global<TrackPlaceAtCallConfig>;
 
 abstract class DynamicallySetElementValueOfHTML extends DataFlow::Node { }
diff --git a/javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/UI5Xss.expected b/javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/UI5Xss.expected
@@ -0,0 +1,22 @@
+nodes
+| webapp/controller/app.controller.js:10:17:10:27 | input: null |
+| webapp/controller/app.controller.js:16:35:16:62 | oModel. ... input') |
+| webapp/controller/app.controller.js:19:36:19:63 | oModel. ... input') |
+| webapp/controller/app.controller.js:20:35:20:58 | unsanit ... ntent() |
+| webapp/view/app.view.xml:5:5:7:28 | value={/input} |
+| webapp/view/app.view.xml:8:5:8:79 | content={/output} |
+edges
+| webapp/controller/app.controller.js:10:17:10:27 | input: null | webapp/controller/app.controller.js:16:35:16:62 | oModel. ... input') |
+| webapp/controller/app.controller.js:10:17:10:27 | input: null | webapp/controller/app.controller.js:19:36:19:63 | oModel. ... input') |
+| webapp/controller/app.controller.js:10:17:10:27 | input: null | webapp/view/app.view.xml:5:5:7:28 | value={/input} |
+| webapp/controller/app.controller.js:11:17:11:28 | output: null | webapp/view/app.view.xml:8:5:8:79 | content={/output} |
+| webapp/controller/app.controller.js:13:26:13:45 | new JSONModel(oData) | webapp/view/app.view.xml:8:5:8:79 | content={/output} |
+| webapp/controller/app.controller.js:19:36:19:63 | oModel. ... input') | webapp/controller/app.controller.js:20:35:20:58 | unsanit ... ntent() |
+| webapp/view/app.view.xml:5:5:7:28 | value={/input} | webapp/controller/app.controller.js:10:17:10:27 | input: null |
+| webapp/view/app.view.xml:5:5:7:28 | value={/input} | webapp/controller/app.controller.js:13:26:13:45 | new JSONModel(oData) |
+| webapp/view/app.view.xml:8:5:8:79 | content={/output} | webapp/controller/app.controller.js:11:17:11:28 | output: null |
+#select
+| webapp/controller/app.controller.js:16:35:16:62 | oModel. ... input') | webapp/view/app.view.xml:5:5:7:28 | value={/input} | webapp/controller/app.controller.js:16:35:16:62 | oModel. ... input') | XSS vulnerability due to $@. | webapp/view/app.view.xml:5:5:7:28 | value={/input} | user-provided value |
+| webapp/controller/app.controller.js:19:36:19:63 | oModel. ... input') | webapp/view/app.view.xml:5:5:7:28 | value={/input} | webapp/controller/app.controller.js:19:36:19:63 | oModel. ... input') | XSS vulnerability due to $@. | webapp/view/app.view.xml:5:5:7:28 | value={/input} | user-provided value |
+| webapp/controller/app.controller.js:20:35:20:58 | unsanit ... ntent() | webapp/controller/app.controller.js:20:35:20:58 | unsanit ... ntent() | webapp/controller/app.controller.js:20:35:20:58 | unsanit ... ntent() | XSS vulnerability due to $@. | webapp/controller/app.controller.js:20:35:20:58 | unsanit ... ntent() | user-provided value |
+| webapp/controller/app.controller.js:20:35:20:58 | unsanit ... ntent() | webapp/view/app.view.xml:5:5:7:28 | value={/input} | webapp/controller/app.controller.js:20:35:20:58 | unsanit ... ntent() | XSS vulnerability due to $@. | webapp/view/app.view.xml:5:5:7:28 | value={/input} | user-provided value |
diff --git a/javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/UI5Xss.qlref b/javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/UI5Xss.qlref
@@ -0,0 +1 @@
+UI5Xss/UI5Xss.ql
diff --git a/javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/package-lock.json b/javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/package-lock.json
diff --git a/javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/package.json b/javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/package.json
@@ -0,0 +1,5 @@
+{
+  "name": "sap-ui5-xss",
+  "version": "1.0.0",
+  "main": "index.js"
+}
diff --git a/javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/ui5.yaml b/javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/ui5.yaml
@@ -0,0 +1,7 @@
+specVersion: '3.0'
+metadata:
+  name: sap-ui5-xss
+type: application
+framework:
+  name: SAPUI5
+  version: "1.115.0"
diff --git a/javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/webapp/controller/app.controller.js b/javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/webapp/controller/app.controller.js
@@ -0,0 +1,32 @@
+sap.ui.define([
+    "sap/ui/core/mvc/Controller",
+    "sap/ui/model/json/JSONModel",
+    "sap/ui/core/HTML"
+], function (Controller, JSONModel, HTML) {
+    "use strict";
+    return Controller.extend("codeql-sap-js.controller.app", {
+        onInit: function () {
+            var oData = {
+                input: null,
+                output: null,
+            };
+            var oModel = new JSONModel(oData);
+            this.getView().setModel(oModel);
+
+            jQuery.sap.globalEval(oModel.getProperty('/input')); // UNSAFE: evaluating user input
+
+            var unsanitized = new HTML();
+            unsanitized.setContent(oModel.getProperty('/input')); // UNSAFE: evaluating user input
+            jQuery.sap.globalEval(unsanitized.getContent()); // UNSAFE: setContent->getContent flow step
+
+            var sanitized = new HTML();
+            sanitized.setSanitizeContent(true);
+            sanitized.setContent(oModel.getProperty('/input')); // SAFE: content is sanitized before eval
+            jQuery.sap.globalEval(sanitized.getContent()); // SAFE: content is sanitized before eval
+
+            let htmlSanitized = this.getView().byId("htmlSanitized");
+            jQuery.sap.globalEval(htmlSanitized.getContent()); // SAFE: content is sanitized declaratively in the view
+        }
+    });
+}
+);
diff --git a/javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/webapp/index.html b/javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/webapp/index.html
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+
+<head>
+
+    <meta charset="utf-8">
+    <title>SAPUI5 XSS</title>
+    <script src="https://sdk.openui5.org/resources/sap-ui-core.js" 
+        data-sap-ui-libs="sap.m"
+        data-sap-ui-onInit="module:codeql-sap-js/index" 
+        data-sap-ui-resourceroots='{
+            "codeql-sap-js": "./"
+        }'>
+        </script>
+</head>
+
+<body class="sapUiBody" id="content">
+
+</body>
+
+</html>
diff --git a/javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/webapp/index.js b/javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/webapp/index.js
@@ -0,0 +1,11 @@
+sap.ui.define([
+    "sap/ui/core/mvc/XMLView"
+], function (XMLView) {
+    "use strict";
+    XMLView.create({
+        viewName: "codeql-sap-js.view.app"
+    }).then(function (oView) {
+        oView.placeAt("content");
+    });
+
+}); 
diff --git a/javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/webapp/manifest.json b/javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/webapp/manifest.json
@@ -0,0 +1,5 @@
+{
+    "sap.app": {
+        "id": "sap-ui5-xss"
+    }
+}
diff --git a/javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/webapp/view/app.view.xml b/javascript/frameworks/ui5/test/queries/UI5Xss/xss-html-control-sanitized/webapp/view/app.view.xml
@@ -0,0 +1,9 @@
+<mvc:View controllerName="codeql-sap-js.controller.app"
+    xmlns="sap.m"
+    xmlns:core="sap.ui.core"
+    xmlns:mvc="sap.ui.core.mvc">
+    <Input placeholder="Enter Payload (will be sanitized)" 
+        description="Try: &lt;img src=x onerror=alert(&quot;XSS&quot;)&gt;" 
+        value="{/input}" />  <!--User input source sap.m.Input.value -->
+    <core:HTML id="htmlSanitized" content="{/output}" sanitizeContent="true"/> <!--sanitized XSS sink sap.ui.core.HTML.content -->
+</mvc:View>

Original file line number	Diff line number	Diff line change
`@@ -805,6 +805,8 @@ private newtype TUI5Control =`
`805`	`805`	`.(ArrayLiteralNode)`
`806`	`806`	`.asExpr()`
`807`	`807`	`)`
	`808`	`+ or`
	`809`	`+ control = ModelOutput::getATypeNode("Control").getAnInvocation()`
`808`	`810`	`}`
`809`	`811`
`810`	`812`	`class UI5Control extends TUI5Control {`
`@@ -852,7 +854,9 @@ class UI5Control extends TUI5Control {`
`852`	`854`	`)`
`853`	`855`	`or`
`854`	`856`	`exists(NewNode control \| control = this.asJsControl() \|`
`855`		`- result = this.asJsControl().asExpr().getAChildExpr().(DotExpr).getQualifiedName()`
	`857`	`+ result = control.asExpr().getAChildExpr().(PropAccess).getQualifiedName()`
	`858`	`+ or`
	`859`	`+ control = API::moduleImport(result).getAnInvocation()`
`856`	`860`	`)`
`857`	`861`	`}`
`858`	`862`
`@@ -988,9 +992,12 @@ class UI5Control extends TUI5Control {`
`988`	`992`	`)`
`989`	`993`	`or`
`990`	`994`	/* 3. `sanitizeContent` attribute is set programmatically using a setter. */
`991`		`- exists(CallNode node \|`
`992`		`- node = this.getAReference().getAMemberCall("setS" + propName.suffix(1)) and`
	`995`	`+ exists(CallNode node, string setterName \|`
	`996`	`+ setterName = "set" + propName.prefix(1).toUpperCase() + propName.suffix(1) and`
`993`	`997`	`not node.getArgument(0).mayHaveBooleanValue(val.booleanNot())`
	`998`	`+ \|`
	`999`	`+ node = this.getAReference().getAMemberCall(setterName) or`
	`1000`	`+ node = this.asJsControl().getAMemberCall(setterName)`
`994`	`1001`	`)`
`995`	`1002`	`}`
`996`	`1003`	`}`