From a24ea05596030ef654689886d84800d9bf20d9e1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 19 Aug 2026 10:04:34 +0300 Subject: [PATCH 1/6] security(core): HTML-encode the application name in the graph note tooltip The graph-note hover tooltip builds its content as an HTML string that includes the application name from countlyGlobal, whose values are raw at runtime, and renders it via tipsy html:true. Encode the application name with countlyCommon.encodeHtml so it renders as text; the other values in the tooltip are API-encoded or i18n. Display is unchanged. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 1 + frontend/express/public/javascripts/countly/countly.common.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37ace8a79ae..4e0970edfdc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ 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 - [hooks] Internal event hooks are now scoped to the apps the hook belongs to: app creation is a global-admin-only event, and remote-config, cohort, alert and hook-chaining events are only delivered when the event's app is one the hook is scoped to - [compliance-hub] The consents table now returns a fixed set of fields; a projection supplied on the request is no longer used to widen the response beyond the consent columns - [dashboards] Widgets are no longer copied when the copying user has no access to the apps they reference, and widget app ids are validated on widget create and update diff --git a/frontend/express/public/javascripts/countly/countly.common.js b/frontend/express/public/javascripts/countly/countly.common.js index 5c3ebc5297c..3846434dd7a 100644 --- a/frontend/express/public/javascripts/countly/countly.common.js +++ b/frontend/express/public/javascripts/countly/countly.common.js @@ -1115,7 +1115,7 @@ var noteTime = moment(notes[0].ts).format("D MMM, HH:mm"); var noteId = notes[0].app_id; var app = countlyGlobal.apps[noteId] || {}; - titleDom = "
" + noteTime + "
" + app.name + "
" + + titleDom = "
" + noteTime + "
" + countlyCommon.encodeHtml(app.name) + "
" + "
" + notes[0].note + "
" + "
" + (notes[0].owner_name) + " | " + (jQuery.i18n.map["notes.note-" + notes[0].noteType] || notes[0].noteType) + "
" + "
"; From 937fbc8c437c545e3444d41696c990e77f79431e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 19 Aug 2026 10:11:50 +0300 Subject: [PATCH 2/6] security(push): sanitize message-editor content before it enters the contenteditable The push message editor set the composed message as innerHTML on a live contenteditable. Sanitize that content with countlyCommon.encodeSomeHtml, allowing only the user-property token span (and the attributes it relies on: class, id, contenteditable, data-user-property-*) and escaping any other markup to inert text. The message body is user text and the token element is the only legitimate markup, so display is unchanged for normal messages; the token id is preserved so the editor's per-token event wiring keeps working. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 1 + .../javascripts/countly.views.component.common.js | 11 ++++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e0970edfdc..99a9847ebbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Enterprise Fixes: 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 - [hooks] Internal event hooks are now scoped to the apps the hook belongs to: app creation is a global-admin-only event, and remote-config, cohort, alert and hook-chaining events are only delivered when the event's app is one the hook is scoped to - [compliance-hub] The consents table now returns a fixed set of fields; a projection supplied on the request is no longer used to widen the response beyond the consent columns - [dashboards] Widgets are no longer copied when the copying user has no access to the apps they reference, and widget app ids are validated on widget create and update 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(); }, From a6a8dbf91b3b01db0f3d80e1627231b8edd46125 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 14:54:34 +0300 Subject: [PATCH 3/6] [security][compliance-hub] escape interpolated values in the export history actions column The export/purge history datatable builds an html string in onReady and the template renders it, so every value interpolated into it has to be html-safe before it gets there. Values taken from the row arrive through common.returnOutput, which escape_html_entities has already escaped, so they are deliberately left alone: escaping them a second time would surface the entities literally in the ui. One value in that string comes from countlyGlobal instead. That object is serialized into the dashboard by express-expose, whose escaping is for the javascript string context and is value-preserving by design, so the api's html escaping never applied to it. It is now escaped where it is interpolated. Adds test/unit-tests/plugins.compliance-hub.actions-escaping.js, which loads the real module in a sandbox and exercises the actual onReady builder. It pins both directions: a value carrying markup is neutralized, and an already-escaped api value is not double-escaped. Co-Authored-By: Claude Opus 4.8 --- .../public/javascripts/countly.models.js | 8 +- ...plugins.compliance-hub.actions-escaping.js | 132 ++++++++++++++++++ 2 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 test/unit-tests/plugins.compliance-hub.actions-escaping.js diff --git a/plugins/compliance-hub/frontend/public/javascripts/countly.models.js b/plugins/compliance-hub/frontend/public/javascripts/countly.models.js index bd758822308..c4e6f858295 100644 --- a/plugins/compliance-hub/frontend/public/javascripts/countly.models.js +++ b/plugins/compliance-hub/frontend/public/javascripts/countly.models.js @@ -194,7 +194,13 @@ var ret = "

" + ((jQuery.i18n.map["systemlogs.action." + row.a]) ? jQuery.i18n.map["systemlogs.action." + row.a] : row.a) + "

"; if (typeof row.i === "object") { if (typeof row.i.app_id !== "undefined" && countlyGlobal.apps[row.i.app_id]) { - ret += "

" + jQuery.i18n.map["systemlogs.for-app"] + ": " + countlyGlobal.apps[row.i.app_id].name + "

"; + //this string is rendered with v-html, so every interpolated value must already be + //HTML-safe. Everything taken from "row" arrives through common.returnOutput, which + //escape_html_entities has already escaped, so it must NOT be escaped again here or + //the entities would show up literally. The app name is the exception: it comes from + //countlyGlobal, which is serialized into the dashboard's script island by + //express-expose and is never HTML-escaped, so it reaches us raw and is escaped here. + ret += "

" + jQuery.i18n.map["systemlogs.for-app"] + ": " + countlyCommon.encodeHtml(countlyGlobal.apps[row.i.app_id].name) + "

"; } if (typeof row.i.appuser_id !== "undefined") { ret += "

" + jQuery.i18n.map["systemlogs.for-appuser"] + ": " + row.i.appuser_id + "

"; 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(""); + // the only markup left must be the

wrappers this builder emits itself + actions.replace(/<\/?p[^>]*>/g, "").indexOf("<").should.equal(-1); + done(); + }); + + it("keeps an ordinary app name readable", function(done) { + var actions = actionsFor("My Application"); + actions.should.containEql("For app: My Application"); + done(); + }); + + it("does not double-escape values the API already escaped", function(done) { + // returnOutput turns ' into '; escaping again would surface "&#39;" in the UI + var actions = actionsFor("My Application", { app_id: APP_ID, appuser_id: "user's-id" }); + actions.should.containEql("user's-id"); + actions.indexOf("&#39;").should.equal(-1); + done(); + }); +}); From 9af2b50354ec69a819d0ebf71ea5462383b12541 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 19 Aug 2026 09:52:50 +0300 Subject: [PATCH 4/6] [security][core] render populator confirm dialog bodies as text instead of html The populator template and environment delete confirmations rendered their body with v-html, but the value bound there is a localized sentence with a name substituted into a {0} placeholder. Both names are html-decoded on the way in, so the escaping the api applied no longer held by the time they were rendered. Text interpolation removes the sink instead of filtering what reaches it. Checked every assignment to this dialog object and every populator locale file that supplies these strings, in all 26 languages present: none contain tags, so nothing renders differently. Left the plugins plugin's dependency confirmation on v-html deliberately. Its strings do carry markup: plugins.confirm has a

in all 24 translated locale files, even though the default locale no longer has it. The values interpolated into that one are plugin titles from package metadata rather than anything a dashboard account can write. Co-Authored-By: Claude Opus 4.8 --- .../frontend/public/templates/environment_detail.html | 3 ++- plugins/populator/frontend/public/templates/populator.html | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/populator/frontend/public/templates/environment_detail.html b/plugins/populator/frontend/public/templates/environment_detail.html index 28d5ea365fc..4395bf63b4a 100644 --- a/plugins/populator/frontend/public/templates/environment_detail.html +++ b/plugins/populator/frontend/public/templates/environment_detail.html @@ -33,7 +33,8 @@

diff --git a/plugins/populator/frontend/public/templates/populator.html b/plugins/populator/frontend/public/templates/populator.html index 0ec4dbfd018..82e0c955f29 100644 --- a/plugins/populator/frontend/public/templates/populator.html +++ b/plugins/populator/frontend/public/templates/populator.html @@ -178,7 +178,8 @@

From c387c0133f52b9c83844b4a55906a94f184401f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Aug 2026 23:54:54 +0300 Subject: [PATCH 5/6] security(core): escape "<" in the res.expose script island (stored XSS via app name) The dashboard serialises the exposed countlyGlobal object into an inline ). The serialiser only neutralised the exact sequence "", but the HTML tokeniser also ends a script element at "", "" and other whitespace/slash spellings, so an application name containing one of those broke out of the script block. An app admin of a single app could store such a name; any global admin who then loaded the dashboard (which lists every app) executed the attacker's markup in their own session, escalating an app-admin account to global-admin control. Escape every "<" as < in both serialisation paths: string values (the primitive branch, replacing the exact-match ""/"