From 2b1df39aaac8cd8bd1faec136fa9768c5a88045b Mon Sep 17 00:00:00 2001
From: Christian Hoffmann <mail@hoffmann-christian.info>
Date: Tue, 27 Apr 2021 23:16:42 +0200
Subject: [PATCH] Server: Permit simple HTML in chat messages again

PR #939 disabled HTML usage in chat messages by escaping all user input
at the server side for security reasons.

There is demand to keep basic text formatting working for sharing
lyrics/chords in a sane way.

This PR re-enables certain safe tags again to enable such use cases.

This works by keeping the existing message escaping, but converting
selected safe HTML tags back into their parsable form.

To avoid worsening security again, this PR deliberately
- does not permit CSS or any color changes which could enable user
  impersonation,
- does not accept any attributes in HTML tags,
- does not accept invalid HTML (e.g. start tags without end tags),
- does not accept nested HTML tags (except for <br>s within
  <pre>...</pre>).

References:
https://github.com/jamulussoftware/jamulus/discussions/1021
https://github.com/jamulussoftware/jamulus/discussions/1524
---
 src/server.cpp | 24 +++++++++++++++++++++++-
 src/server.h   |  2 ++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/src/server.cpp b/src/server.cpp
index 99c7ab5c48..6e95d1535f 100644
--- a/src/server.cpp
+++ b/src/server.cpp
@@ -426,6 +426,20 @@ CServer::CServer ( const int          iNewMaxNumChan,
         QThreadPool::globalInstance()->setMaxThreadCount ( QThread::idealThreadCount() * 4 );
     }
 
+    // initialize chat html sanitization patterns:
+    static QList<QString> qlPermittedChatTagNames = {
+        "b",
+        "i",
+        "p",
+        "pre",
+        "u",
+    };
+    foreach ( auto tag, qlPermittedChatTagNames )
+    {
+        QRegExp pattern = QRegExp ( "&lt;(" + tag + ")&gt;([^<>]+)&lt;/" + tag + "&gt;" );
+        pattern.setMinimal ( true ); // non-greedy matching
+        qlPermittedChatTagPatterns << pattern;
+    }
 
     // Connections -------------------------------------------------------------
     // connect timer timeout signal
@@ -1475,11 +1489,19 @@ void CServer::CreateAndSendChatTextForAllConChannels ( const int      iCurChanID
     // use different colors
     QString sCurColor = vstrChatColors[iCurChanID % vstrChatColors.Size()];
 
+    // escape all html tags, but selectively allow safe tags again:
+    QString strChatTextFiltered = strChatText.toHtmlEscaped();
+    foreach ( auto pattern, qlPermittedChatTagPatterns )
+    {
+        strChatTextFiltered = strChatTextFiltered.replace ( pattern, "<\\1>\\2</\\1>" );
+    }
+    strChatTextFiltered = strChatTextFiltered.replace ( "&lt;br&gt;", "<br>" );
+
     const QString strActualMessageText =
         "<font color=\"" + sCurColor + "\">(" +
         QTime::currentTime().toString ( "hh:mm:ss AP" ) + ") <b>" +
         ChanName.toHtmlEscaped() +
-        "</b></font> " + strChatText.toHtmlEscaped();
+        "</b></font> " + strChatTextFiltered;
 
 
     // Send chat text to all connected clients ---------------------------------
diff --git a/src/server.h b/src/server.h
index 0bc8ecc57e..49879dece5 100644
--- a/src/server.h
+++ b/src/server.h
@@ -415,6 +415,8 @@ class CServer :
 
     CSignalHandler*            pSignalHandler;
 
+    QList<QRegExp>             qlPermittedChatTagPatterns;
+
 signals:
     void Started();
     void Stopped();