From c43c9e6244f1e0eed2ed8d42e7b1ecadef2d98e6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 12:40:06 +0300 Subject: [PATCH 1/2] [fix][push] render message text as text in the notification preview The details view builds its phone-style preview by splicing personalization elements into the message and handing the result to innerHTML, so it can walk the nodes back out and turn each into a component. The message text travels inside that string, so a tag in the message was parsed as markup and its handlers ran. The decode in front of it is not the mistake and cannot simply go. The indexes in messagePers and titlePers are offsets into the message as it was typed, and the api escapes on output, so the string has to be decoded before those offsets line up with anything. Removing the decode would misplace every element that follows an escaped character. So the placeholders now go in as opaque tokens, the whole string is escaped, and the tokens are swapped for their elements afterwards. Only markup generated here survives the escape. The offset arithmetic is untouched: a token stands in for an element and is measured the same way, so every position it computes is the position it computed before. Escaping is a plain string replacement rather than countlyCommon.encodeHtml, which round-trips through innerText and would fold newlines into
, changing both the text and the offsets. This covers the title as well as the message, since both go through buildMessageText. The element markup itself was already safe, built with setAttribute and innerText and serialized with outerHTML. Verified against the pre-change source: the payload no longer produces an element or an event-handler attribute and instead shows as the text that was typed, and the preview components for ordinary messages are byte-identical across single, leading, multiple and adjacent placeholders, text containing an ampersand, and messages with no personalization at all. --- .../public/javascripts/countly.models.js | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/plugins/push/frontend/public/javascripts/countly.models.js b/plugins/push/frontend/public/javascripts/countly.models.js index e3d7eb6c646..a09baf1a9ee 100644 --- a/plugins/push/frontend/public/javascripts/countly.models.js +++ b/plugins/push/frontend/public/javascripts/countly.models.js @@ -900,6 +900,24 @@ return map[m]; }); }, + /** + * Escape text so it cannot contribute markup once the result reaches innerHTML. + * + * Deliberately a string replacement rather than countlyCommon.encodeHtml, which + * round-trips through innerText and would normalise newlines into
, changing + * the text and the offsets the personalization indexes rely on. + * + * @param {string} str - untrusted text + * @returns {string} the text, safe to place in an html string + */ + escapeMessageText: function(str) { + return String(str) + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); + }, buildMessageText: function(message, userPropertiesDto) { var self = this; if (!message) { @@ -936,11 +954,23 @@ // } // }); // return html; + //The indexes in userPropertiesDto are offsets into the message as it was typed, + //so the escaping the api applied has to come off before they can line up. The + //result is handed to innerHTML further along, in + //getPreviewMessageComponentsList, which is what made a tag in the message run. + // + //So the placeholders go in as opaque tokens first, the whole string is escaped, + //and the tokens are swapped for their elements afterwards. The arithmetic below + //is untouched: a token takes the element's place and is the same string for + //length purposes, so every position it computes is the position it computed + //before. Only markup generated here survives the escape. var messageInHTMLString = this.decodeHtml(message); + var placeholderElements = []; var buildMessageLength = 0; var previousIndex = undefined; this.sortUserProperties(userPropertiesDto).forEach(function(currentUserPropertyIndex, index) { - var userPropertyStringElement = self.getUserPropertyElement(currentUserPropertyIndex, userPropertiesDto[currentUserPropertyIndex]); + var userPropertyStringElement = "@@CLY_PERS_" + placeholderElements.length + "@@"; + placeholderElements.push(self.getUserPropertyElement(currentUserPropertyIndex, userPropertiesDto[currentUserPropertyIndex])); if (index === 0) { messageInHTMLString = self.insertUserPropertyAtIndex(messageInHTMLString, currentUserPropertyIndex, userPropertyStringElement); buildMessageLength = Number(currentUserPropertyIndex) + userPropertyStringElement.length; @@ -956,7 +986,10 @@ } previousIndex = currentUserPropertyIndex; }); - return messageInHTMLString; + //The token has no character the escape touches, so it comes through intact. + return self.escapeMessageText(messageInHTMLString).replace(/@@CLY_PERS_(\d+)@@/g, function(match, tokenIndex) { + return placeholderElements[Number(tokenIndex)] || ""; + }); }, mapType: function(dto) { if (dto.triggers[0].kind === 'plain') { From bf7d37068e2835d0b602e64e19b01b31ae5b9eac Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 11:06:49 +0300 Subject: [PATCH 2/2] fix(push): escape the preview message on every path, and make the placeholder token uncollidable (24.05) Answers both review findings on the preview builder. **A notification without personalization skipped the escaping entirely.** buildMessageText returned early before any of it ran, and the result goes to innerHTML in getPreviewMessageComponentsList. It happened to be safe for a stored notification, because the api escapes what it returns - but the preview also renders the editor's live state, and text typed into the editor has been through no escaping at all. That path now makes the same round trip the personalization path makes: decode, then escape. Not escape alone, which is what the note suggested: the stored message is already escaped, so escaping it again would show a user <b> where they wrote something else. Decoding first makes the result exactly one level of escaping whichever way the message arrived, and it is idempotent for the stored case. **The placeholder token could be typed.** @@CLY_PERS_0@@ in the message body was indistinguishable from the token this function inserts, so an author's literal text became an extra personalization span, or vanished when the index was out of range - and the edit state carries that corruption back on save. The token now carries a per call nonce, which cannot be guessed or typed. Length is not constrained here, since the arithmetic measures the token it actually inserted, so none of the offset handling changes. Checked the round trip against both shapes rather than reasoning about it: an api-escaped message comes back byte-identical, and a raw typed tag comes back neutralized. --- .../public/javascripts/countly.models.js | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/plugins/push/frontend/public/javascripts/countly.models.js b/plugins/push/frontend/public/javascripts/countly.models.js index a09baf1a9ee..4280c81c63e 100644 --- a/plugins/push/frontend/public/javascripts/countly.models.js +++ b/plugins/push/frontend/public/javascripts/countly.models.js @@ -924,7 +924,15 @@ message = ""; } if (!userPropertiesDto) { - return message; + //the same round trip the personalization path makes below, for the same + //reason: the result is handed to innerHTML in + //getPreviewMessageComponentsList. Returning the value untouched relied on + //the api having escaped it, which holds for a stored notification and not + //for text typed into the editor, whose state this preview also renders. + // + //Decode then escape rather than escape alone: escaping an already escaped + //message would show <b> to the user instead of what they wrote. + return self.escapeMessageText(self.decodeHtml(message)); } // var html = '', // keys = this.sortUserProperties(userPropertiesDto), @@ -965,11 +973,19 @@ //length purposes, so every position it computes is the position it computed //before. Only markup generated here survives the escape. var messageInHTMLString = this.decodeHtml(message); + //A fixed token could be typed into the message. Then the replacement below + //cannot tell the author's text from ours: it becomes an extra personalization + //span, or disappears when the index is out of range, and the edit state carries + //that corruption back on save. A per call nonce cannot be guessed or typed. + // + //Length is not constrained - the arithmetic below measures the token it + //actually inserted - so this changes nothing about the offsets. + var tokenNonce = "CLYPERS" + Math.random().toString(36).slice(2, 10); var placeholderElements = []; var buildMessageLength = 0; var previousIndex = undefined; this.sortUserProperties(userPropertiesDto).forEach(function(currentUserPropertyIndex, index) { - var userPropertyStringElement = "@@CLY_PERS_" + placeholderElements.length + "@@"; + var userPropertyStringElement = "@@" + tokenNonce + "_" + placeholderElements.length + "@@"; placeholderElements.push(self.getUserPropertyElement(currentUserPropertyIndex, userPropertiesDto[currentUserPropertyIndex])); if (index === 0) { messageInHTMLString = self.insertUserPropertyAtIndex(messageInHTMLString, currentUserPropertyIndex, userPropertyStringElement); @@ -987,7 +1003,7 @@ previousIndex = currentUserPropertyIndex; }); //The token has no character the escape touches, so it comes through intact. - return self.escapeMessageText(messageInHTMLString).replace(/@@CLY_PERS_(\d+)@@/g, function(match, tokenIndex) { + return self.escapeMessageText(messageInHTMLString).replace(new RegExp("@@" + tokenNonce + "_(\\d+)@@", "g"), function(match, tokenIndex) { return placeholderElements[Number(tokenIndex)] || ""; }); },