diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7703fab7885..389f793b873 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,13 @@ Fixes:
Enterprise Fixes:
- [data-manager] Fixed editing an event whose key contains `&` creating undeletable duplicate rows in the events table
+Security Fixes:
+- [core] The graph note tooltip now HTML-encodes the application name before rendering, so an application name is shown as text rather than markup
+- [push] The message editor now sanitizes message content before rendering it into the editor, allowing only the user-property token element and rendering any other markup as text
+- [compliance-hub] The export/purge history table now HTML-encodes the application name before it is placed in the action cell, so an application name is shown as text rather than markup
+- [populator] The populator confirmation dialog bodies are now rendered as text instead of HTML
+- [core] The dashboard escapes `<` when serializing the exposed `countlyGlobal` object into the inline page script, so an application name (or any exposed value) containing a `` end tag in any spelling can no longer break out of the script block and run in another user's session; the active-app name is now rendered with `.text()` instead of `.html()`
+
## Version 24.05.51
Fixes:
diff --git a/frontend/express/libs/express-expose.js b/frontend/express/libs/express-expose.js
index 34d662e58e3..5a160207e23 100644
--- a/frontend/express/libs/express-expose.js
+++ b/frontend/express/libs/express-expose.js
@@ -186,9 +186,13 @@ function string(obj) {
else {
obj = JSON.stringify(obj);
if (obj) {
- // Only escape things that could break out of script context
- obj = obj.replace(/<\/script>/ig, '');
- obj = obj.replace(/
+
{{dialog.text}}
diff --git a/plugins/push/frontend/public/javascripts/countly.views.component.common.js b/plugins/push/frontend/public/javascripts/countly.views.component.common.js
index 84082df30e1..765fd61b207 100644
--- a/plugins/push/frontend/public/javascripts/countly.views.component.common.js
+++ b/plugins/push/frontend/public/javascripts/countly.views.component.common.js
@@ -1,6 +1,15 @@
/* eslint-disable no-console */
/*global CV,countlyVue,countlyPushNotification,countlyGlobal,countlyCommon,moment*/
(function(countlyPushNotificationComponent) {
+ // The message editor is a live contenteditable. Its body is user-authored text; the
+ // only legitimate markup is the user-property token . Allow just that element
+ // (with the attributes the token relies on) and let everything else be escaped to inert
+ // text, so a stored message cannot introduce active markup when the editor is populated.
+ var PUSH_MESSAGE_EDITOR_XSS_OPTIONS = {
+ whiteList: {
+ span: ["class", "id", "contenteditable", "data-user-property-label", "data-user-property-value", "data-user-property-fallback"]
+ }
+ };
countlyPushNotificationComponent.LargeRadioButtonWithDescription = countlyVue.views.create({
props: {
value: {
@@ -706,7 +715,7 @@
},
reset: function(htmlContent, ids) {
this.disconnectMutationObserver();
- this.$refs.element.innerHTML = htmlContent;
+ this.$refs.element.innerHTML = countlyCommon.encodeSomeHtml(htmlContent, PUSH_MESSAGE_EDITOR_XSS_OPTIONS);
this.addEventListeners(ids);
this.startMutationObserver();
},
diff --git a/test/unit-tests/plugins.compliance-hub.actions-escaping.js b/test/unit-tests/plugins.compliance-hub.actions-escaping.js
new file mode 100644
index 00000000000..2c512055487
--- /dev/null
+++ b/test/unit-tests/plugins.compliance-hub.actions-escaping.js
@@ -0,0 +1,132 @@
+var should = require("should");
+var fs = require("fs");
+var path = require("path");
+var vm = require("vm");
+
+// The export/purge history datatable builds an HTML string in onReady and the template renders it
+// with v-html (plugins/compliance-hub/frontend/public/templates/exportHistory.html). Everything
+// interpolated into that string therefore has to be HTML-safe already.
+//
+// Two opposite mistakes are possible and this file guards both directions:
+//
+// 1. NOT escaping the app name. It is read from countlyGlobal, which express-expose serializes
+// into the dashboard's inline script island. That serializer escapes for the JavaScript string
+// context only and is deliberately value-preserving, so the value arrives raw. An app admin can
+// set their own app's name, and a global admin's dashboard lists every app, so an unescaped name
+// is a stored cross-user XSS.
+// 2. Escaping the values that came from the API. Those already went through
+// common.escape_html_entities in common.returnOutput, so escaping them again would render the
+// entities literally in the UI.
+var SRC = path.resolve(__dirname, "../../plugins/compliance-hub/frontend/public/javascripts/countly.models.js");
+
+/**
+ * Load countly.models.js in a sandbox and hand back the onReady callbacks it registers,
+ * keyed by data-table name.
+ * @param {object} apps - the countlyGlobal.apps map the module should see
+ * @returns {object} map of resource name to its onReady function
+ */
+function loadResources(apps) {
+ var resources = {};
+ var noop = function() {};
+ // matches countlyCommon.encodeHtml, which is `div.innerText = x; return div.innerHTML`:
+ // the text-node serializer escapes &, < and > and leaves quotes alone.
+ var encodeHtml = function(html) {
+ return (html + "").replace(/&/g, "&").replace(//g, ">");
+ };
+ var sandbox = {
+ window: {},
+ countlyCommon: {
+ encodeHtml: encodeHtml,
+ formatTimeAgoText: function() {
+ return { text: "just now" };
+ },
+ getDescendantProp: noop,
+ API_PARTS: { data: { r: "/o" } },
+ ACTIVE_APP_ID: "5f1a2b3c4d5e6f0011223344",
+ periodObj: {},
+ getPeriodForAjax: function() {
+ return "30days";
+ }
+ },
+ CountlyHelpers: { createMetricModel: noop },
+ jQuery: { i18n: { map: { "systemlogs.for-app": "For app", "systemlogs.for-appuser": "For app user", "systemlogs.action.export": "Data exported" } } },
+ CV: {
+ i18n: function(k) {
+ return k;
+ }
+ },
+ countlyGlobal: { apps: apps },
+ countlyTaskManager: {},
+ countlyVue: {
+ vuex: {
+ ServerDataTable: function(name, cfg) {
+ resources[name] = cfg.onReady;
+ return { name: name };
+ },
+ Module: function() {
+ return {};
+ },
+ MutationsFor: noop,
+ ActionsFor: noop
+ }
+ }
+ };
+ sandbox.global = sandbox;
+ vm.createContext(sandbox);
+ vm.runInContext(fs.readFileSync(SRC, "utf8"), sandbox, { filename: SRC });
+ return resources;
+}
+
+describe("compliance-hub export history actions escaping", function() {
+ var APP_ID = "5f1a2b3c4d5e6f0011223344";
+
+ /**
+ * Run the export-history onReady over a single row.
+ * @param {string} appName - the app name countlyGlobal should carry
+ * @param {object} i - the row's "i" payload
+ * @returns {string} the built actions HTML
+ */
+ function actionsFor(appName, i) {
+ var apps = {};
+ apps[APP_ID] = { name: appName };
+ var onReady = loadResources(apps).exportHistoryDataResource;
+ should.exist(onReady);
+ var rows = onReady({}, [{ a: "export", ts: 0, i: i || { app_id: APP_ID } }]);
+ return rows[0].actions;
+ }
+
+ it("escapes an app name that carries a tag", function(done) {
+ var actions = actionsFor('');
+ actions.indexOf("");
+ actions.indexOf("").should.equal(-1);
+ done();
+ });
+
+ it("leaves no raw angle bracket from the app name", function(done) {
+ var actions = actionsFor("