From 74554d36a528c4e44da47d0f5b4bd722f8bbb401 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Thu, 8 Apr 2021 21:39:11 -0400
Subject: [PATCH] chat: Allow `chatNewMessage` hook to modify more values

---
 doc/api/hooks_client-side.md |  5 ++++
 src/static/js/chat.js        | 44 +++++++++++++++++-------------------
 2 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/doc/api/hooks_client-side.md b/doc/api/hooks_client-side.md
index e87536cfe..2559a4e08 100755
--- a/doc/api/hooks_client-side.md
+++ b/doc/api/hooks_client-side.md
@@ -294,6 +294,11 @@ Things in context:
 
 This hook is called on the client side whenever a chat message is received from
 the server. It can be used to create different notifications for chat messages.
+Hoook functions can modify the `author`, `authorName`, `duration`, `sticky`,
+`text`, and `timeStr` context properties to change how the message is processed.
+The `text` and `timeStr` properties may contain HTML, but plugins should be
+careful to sanitize any added user input to avoid introducing an XSS
+vulnerability.
 
 ## collectContentPre
 
diff --git a/src/static/js/chat.js b/src/static/js/chat.js
index 00811ff44..c6b00af13 100755
--- a/src/static/js/chat.js
+++ b/src/static/js/chat.js
@@ -109,14 +109,6 @@ exports.chat = (() => {
       // correct the time
       msg.time += this._pad.clientTimeOffset;
 
-      // create the time string
-      let minutes = `${new Date(msg.time).getMinutes()}`;
-      let hours = `${new Date(msg.time).getHours()}`;
-      if (minutes.length === 1) minutes = `0${minutes}`;
-      if (hours.length === 1) hours = `0${hours}`;
-      const timeStr = `${hours}:${minutes}`;
-
-      // create the authorclass
       if (!msg.userId) {
         /*
          * If, for a bug or a database corruption, the message coming from the
@@ -129,25 +121,25 @@ exports.chat = (() => {
             'Replacing with "unknown". This may be a bug or a database corruption.');
       }
 
-      msg.userId = padutils.escapeHtml(msg.userId);
-      const authorClass = `author-${msg.userId.replace(/[^a-y0-9]/g, (c) => {
+      const authorClass = (authorId) => `author-${authorId.replace(/[^a-y0-9]/g, (c) => {
         if (c === '.') return '-';
         return `z${c.charCodeAt(0)}z`;
       })}`;
 
-      const text = padutils.escapeHtmlWithClickableLinks(msg.text, '_blank');
-
-      const authorName = msg.userName == null ? html10n.get('pad.userlist.unnamed')
-        : padutils.escapeHtml(msg.userName);
-
       // the hook args
       const ctx = {
-        authorName,
+        authorName: msg.userName != null ? msg.userName : html10n.get('pad.userlist.unnamed'),
         author: msg.userId,
-        text,
+        text: padutils.escapeHtmlWithClickableLinks(msg.text, '_blank'),
         sticky: false,
         timestamp: msg.time,
-        timeStr,
+        timeStr: (() => {
+          let minutes = `${new Date(msg.time).getMinutes()}`;
+          let hours = `${new Date(msg.time).getHours()}`;
+          if (minutes.length === 1) minutes = `0${minutes}`;
+          if (hours.length === 1) hours = `0${hours}`;
+          return `${hours}:${minutes}`;
+        })(),
         duration: 4000,
       };
 
@@ -160,7 +152,7 @@ exports.chat = (() => {
       // does this message contain this user's name? (is the curretn user mentioned?)
       const myName = $('#myusernameedit').val();
       const wasMentioned =
-          text.toLowerCase().indexOf(myName.toLowerCase()) !== -1 && myName !== 'undefined';
+          ctx.text.toLowerCase().indexOf(myName.toLowerCase()) !== -1 && myName !== 'undefined';
 
       // If the user was mentioned, make the message sticky
       if (wasMentioned && !alreadyFocused && !isHistoryAdd && !chatOpen) {
@@ -171,9 +163,14 @@ exports.chat = (() => {
 
       // Call chat message hook
       hooks.aCallAll('chatNewMessage', ctx, () => {
+        const cls = authorClass(ctx.author);
         const html =
-            `<p data-authorId='${msg.userId}' class='${authorClass}'><b>${authorName}:</b>` +
-            `<span class='time ${authorClass}'>${ctx.timeStr}</span> ${ctx.text}</p>`;
+            `<p data-authorId='${padutils.escapeHtml(ctx.author)}' class='${cls}'>` +
+            `<b>${padutils.escapeHtml(ctx.authorName)}:</b>` +
+            // ctx.text was HTML-escaped before calling the hook, and ctx.timeStr couldn't have had
+            // any HTML. Hook functions are trusted to not introduce an XSS vulnerability by adding
+            // unescaped user input to either ctx.text or ctx.timeStr.
+            `<span class='time ${cls}'>${ctx.timeStr}</span> ${ctx.text}</p>`;
         if (isHistoryAdd) $(html).insertAfter('#chatloadmessagesbutton');
         else $('#chattext').append(html);
 
@@ -186,9 +183,10 @@ exports.chat = (() => {
 
           if (!chatOpen && ctx.duration > 0) {
             $.gritter.add({
-              // Note: ctx.authorName and ctx.text are already HTML-escaped.
               text: $('<p>')
-                  .append($('<span>').addClass('author-name').html(ctx.authorName))
+                  .append($('<span>').addClass('author-name').text(ctx.authorName))
+                  // ctx.text was HTML-escaped before calling the hook. Hook functions are trusted
+                  // to not introduce an XSS vulnerability by adding unescaped user input.
                   .append(ctx.text),
               sticky: ctx.sticky,
               time: 5000,