From 49ada81d84287aa60d7755bae1ab936c130c291e Mon Sep 17 00:00:00 2001 From: Adam Miller Date: Tue, 24 Mar 2015 09:11:52 -0500 Subject: [PATCH 1/8] Fix BZ1205130 - CTCP DoS --- quassel-0.11.0-CTCP-query-crash.patch | 347 ++++++++++++++++++++++++++ quassel.spec | 11 +- 2 files changed, 357 insertions(+), 1 deletion(-) create mode 100644 quassel-0.11.0-CTCP-query-crash.patch diff --git a/quassel-0.11.0-CTCP-query-crash.patch b/quassel-0.11.0-CTCP-query-crash.patch new file mode 100644 index 0000000..caf8f7f --- /dev/null +++ b/quassel-0.11.0-CTCP-query-crash.patch @@ -0,0 +1,347 @@ +commit b5e38970ffd55e2dd9f706ce75af9a8d7730b1b8 +Author: Michael Marley +Date: Sat Feb 21 07:33:57 2015 -0500 + + Improve the message-splitting algorithm for PRIVMSG and CTCP + + This introduces a new message splitting algorithm based on + QTextBoundaryFinder. It works by first starting with the entire + message to be sent, encoding it, and checking to see if it is over + the maximum message length. If it is, it uses QTBF to find the + word boundary most immediately preceding the maximum length. If no + suitable boundary can be found, it falls back to searching for + grapheme boundaries. It repeats this process until the entire + message has been sent. + + Unlike what it replaces, the new splitting code is not recursive + and cannot cause stack overflows. Additionally, if it is unable + to split a string, it will give up gracefully and not crash the + core or cause a thread to run away. + + This patch fixes two bugs. The first is garbage characters caused + by accidentally splitting the string in the middle of a multibyte + character. Since the new code splits at a character level instead + of a byte level, this will no longer be an issue. The second is + the core crash caused by sending an overlength CTCP query ("/me") + containing only multibyte characters. This bug was caused by the + old CTCP splitter using the byte index from lastParamOverrun() as + a character index for a QString. + +diff --git a/src/core/corebasichandler.cpp b/src/core/corebasichandler.cpp +index dfa8a99..fbfc76c 100644 +--- a/src/core/corebasichandler.cpp ++++ b/src/core/corebasichandler.cpp +@@ -33,6 +33,9 @@ CoreBasicHandler::CoreBasicHandler(CoreNetwork *parent) + connect(this, SIGNAL(putCmd(QString, const QList &, const QByteArray &)), + network(), SLOT(putCmd(QString, const QList &, const QByteArray &))); + ++ connect(this, SIGNAL(putCmd(QString, const QList> &, const QByteArray &)), ++ network(), SLOT(putCmd(QString, const QList> &, const QByteArray &))); ++ + connect(this, SIGNAL(putRawLine(const QByteArray &)), + network(), SLOT(putRawLine(const QByteArray &))); + } +diff --git a/src/core/corebasichandler.h b/src/core/corebasichandler.h +index 20d057f..a4b5a7f 100644 +--- a/src/core/corebasichandler.h ++++ b/src/core/corebasichandler.h +@@ -55,6 +55,7 @@ public: + signals: + void displayMsg(Message::Type, BufferInfo::Type, const QString &target, const QString &text, const QString &sender = "", Message::Flags flags = Message::None); + void putCmd(const QString &cmd, const QList ¶ms, const QByteArray &prefix = QByteArray()); ++ void putCmd(const QString &cmd, const QList> ¶ms, const QByteArray &prefix = QByteArray()); + void putRawLine(const QByteArray &msg); + + protected: +diff --git a/src/core/corenetwork.cpp b/src/core/corenetwork.cpp +index 7e9ce26..932af6f 100644 +--- a/src/core/corenetwork.cpp ++++ b/src/core/corenetwork.cpp +@@ -284,6 +284,16 @@ void CoreNetwork::putCmd(const QString &cmd, const QList ¶ms, co + } + + ++void CoreNetwork::putCmd(const QString &cmd, const QList> ¶ms, const QByteArray &prefix) ++{ ++ QListIterator> i(params); ++ while (i.hasNext()) { ++ QList msg = i.next(); ++ putCmd(cmd, msg, prefix); ++ } ++} ++ ++ + void CoreNetwork::setChannelJoined(const QString &channel) + { + _autoWhoQueue.prepend(channel.toLower()); // prepend so this new chan is the first to be checked +@@ -980,3 +990,79 @@ void CoreNetwork::requestSetNetworkInfo(const NetworkInfo &info) + } + } + } ++ ++ ++QList> CoreNetwork::splitMessage(const QString &cmd, const QString &message, std::function(QString &)> cmdGenerator) ++{ ++ QString wrkMsg(message); ++ QList> msgsToSend; ++ ++ // do while (wrkMsg.size() > 0) ++ do { ++ // First, check to see if the whole message can be sent at once. The ++ // cmdGenerator function is passed in by the caller and is used to encode ++ // and encrypt (if applicable) the message, since different callers might ++ // want to use different encoding or encode different values. ++ int splitPos = wrkMsg.size(); ++ QList initialSplitMsgEnc = cmdGenerator(wrkMsg); ++ int initialOverrun = userInputHandler()->lastParamOverrun(cmd, initialSplitMsgEnc); ++ ++ if (initialOverrun) { ++ // If the message was too long to be sent, first try splitting it along ++ // word boundaries with QTextBoundaryFinder. ++ QString splitMsg(wrkMsg); ++ QTextBoundaryFinder qtbf(QTextBoundaryFinder::Word, splitMsg); ++ qtbf.setPosition(initialSplitMsgEnc[1].size() - initialOverrun); ++ QList splitMsgEnc; ++ int overrun = initialOverrun; ++ ++ while (overrun) { ++ splitPos = qtbf.toPreviousBoundary(); ++ ++ // splitPos==-1 means the QTBF couldn't find a split point at all and ++ // splitPos==0 means the QTBF could only find a boundary at the beginning of ++ // the string. Neither one of these works for us. ++ if (splitPos > 0) { ++ // If a split point could be found, split the message there, calculate the ++ // overrun, and continue with the loop. ++ splitMsg = splitMsg.left(splitPos); ++ splitMsgEnc = cmdGenerator(splitMsg); ++ overrun = userInputHandler()->lastParamOverrun(cmd, splitMsgEnc); ++ } ++ else { ++ // If a split point could not be found (the beginning of the message ++ // is reached without finding a split point short enough to send) and we ++ // are still in Word mode, switch to Grapheme mode. We also need to restore ++ // the full wrkMsg to splitMsg, since splitMsg may have been cut down during ++ // the previous attempt to find a split point. ++ if (qtbf.type() == QTextBoundaryFinder::Word) { ++ splitMsg = wrkMsg; ++ splitPos = splitMsg.size(); ++ QTextBoundaryFinder graphemeQtbf(QTextBoundaryFinder::Grapheme, splitMsg); ++ graphemeQtbf.setPosition(initialSplitMsgEnc[1].size() - initialOverrun); ++ qtbf = graphemeQtbf; ++ } ++ else { ++ // If the QTBF fails to find a split point in Grapheme mode, we give up. ++ // This should never happen, but it should be handled anyway. ++ qWarning() << "Unexpected failure to split message!"; ++ return msgsToSend; ++ } ++ } ++ } ++ ++ // Once a message of sendable length has been found, remove it from the wrkMsg and ++ // add it to the list of messages to be sent. ++ wrkMsg.remove(0, splitPos); ++ msgsToSend.append(splitMsgEnc); ++ } ++ else{ ++ // If the entire remaining message is short enough to be sent all at once, remove ++ // it from the wrkMsg and add it to the list of messages to be sent. ++ wrkMsg.remove(0, splitPos); ++ msgsToSend.append(initialSplitMsgEnc); ++ } ++ } while (wrkMsg.size() > 0); ++ ++ return msgsToSend; ++} +diff --git a/src/core/corenetwork.h b/src/core/corenetwork.h +index 87121ba..05565a4 100644 +--- a/src/core/corenetwork.h ++++ b/src/core/corenetwork.h +@@ -40,6 +40,8 @@ + + #include "coresession.h" + ++#include ++ + class CoreIdentity; + class CoreUserInputHandler; + class CoreIgnoreListManager; +@@ -93,6 +95,8 @@ public: + inline quint16 localPort() const { return socket.localPort(); } + inline quint16 peerPort() const { return socket.peerPort(); } + ++ QList> splitMessage(const QString &cmd, const QString &message, std::function(QString &)> cmdGenerator); ++ + public slots: + virtual void setMyNick(const QString &mynick); + +@@ -112,6 +116,7 @@ public slots: + void userInput(BufferInfo bufferInfo, QString msg); + void putRawLine(QByteArray input); + void putCmd(const QString &cmd, const QList ¶ms, const QByteArray &prefix = QByteArray()); ++ void putCmd(const QString &cmd, const QList> ¶ms, const QByteArray &prefix = QByteArray()); + + void setChannelJoined(const QString &channel); + void setChannelParted(const QString &channel); +diff --git a/src/core/coreuserinputhandler.cpp b/src/core/coreuserinputhandler.cpp +index 33d1f67..72ac996 100644 +--- a/src/core/coreuserinputhandler.cpp ++++ b/src/core/coreuserinputhandler.cpp +@@ -473,12 +473,16 @@ void CoreUserInputHandler::handleMsg(const BufferInfo &bufferInfo, const QString + return; + + QString target = msg.section(' ', 0, 0); +- QByteArray encMsg = userEncode(target, msg.section(' ', 1)); ++ QString msgSection = msg.section(' ', 1); ++ ++ std::function encodeFunc = [this] (const QString &target, const QString &message) -> QByteArray { ++ return userEncode(target, message); ++ }; + + #ifdef HAVE_QCA2 +- putPrivmsg(serverEncode(target), encMsg, network()->cipher(target)); ++ putPrivmsg(target, msgSection, encodeFunc, network()->cipher(target)); + #else +- putPrivmsg(serverEncode(target), encMsg); ++ putPrivmsg(target, msgSection, encodeFunc); + #endif + } + +@@ -594,11 +598,14 @@ void CoreUserInputHandler::handleSay(const BufferInfo &bufferInfo, const QString + if (bufferInfo.bufferName().isEmpty() || !bufferInfo.acceptsRegularMessages()) + return; // server buffer + +- QByteArray encMsg = channelEncode(bufferInfo.bufferName(), msg); ++ std::function encodeFunc = [this] (const QString &target, const QString &message) -> QByteArray { ++ return channelEncode(target, message); ++ }; ++ + #ifdef HAVE_QCA2 +- putPrivmsg(serverEncode(bufferInfo.bufferName()), encMsg, network()->cipher(bufferInfo.bufferName())); ++ putPrivmsg(bufferInfo.bufferName(), msg, encodeFunc, network()->cipher(bufferInfo.bufferName())); + #else +- putPrivmsg(serverEncode(bufferInfo.bufferName()), encMsg); ++ putPrivmsg(bufferInfo.bufferName(), msg, encodeFunc); + #endif + emit displayMsg(Message::Plain, bufferInfo.type(), bufferInfo.bufferName(), msg, network()->myNick(), Message::Self); + } +@@ -763,56 +770,23 @@ void CoreUserInputHandler::defaultHandler(QString cmd, const BufferInfo &bufferI + } + + +-void CoreUserInputHandler::putPrivmsg(const QByteArray &target, const QByteArray &message, Cipher *cipher) ++void CoreUserInputHandler::putPrivmsg(const QString &target, const QString &message, std::function encodeFunc, Cipher *cipher) + { +- // Encrypted messages need special care. There's no clear relation between cleartext and encrypted message length, +- // so we can't just compute the maxSplitPos. Instead, we need to loop through the splitpoints until the crypted +- // version is short enough... +- // TODO: check out how the various possible encryption methods behave length-wise and make +- // this clean by predicting the length of the crypted msg. +- // For example, blowfish-ebc seems to create 8-char chunks. ++ QString cmd("PRIVMSG"); ++ QByteArray targetEnc = serverEncode(target); + +- static const char *cmd = "PRIVMSG"; +- static const char *splitter = " .,-!?"; ++ std::function(QString &)> cmdGenerator = [&] (QString &splitMsg) -> QList { ++ QByteArray splitMsgEnc = encodeFunc(target, splitMsg); + +- int maxSplitPos = message.count(); +- int splitPos = maxSplitPos; +- forever { +- QByteArray crypted = message.left(splitPos); +- bool isEncrypted = false; + #ifdef HAVE_QCA2 +- if (cipher && !cipher->key().isEmpty() && !message.isEmpty()) { +- isEncrypted = cipher->encrypt(crypted); ++ if (cipher && !cipher->key().isEmpty() && !splitMsg.isEmpty()) { ++ cipher->encrypt(splitMsgEnc); + } + #endif +- int overrun = lastParamOverrun(cmd, QList() << target << crypted); +- if (overrun) { +- // In case this is not an encrypted msg, we can just cut off at the end +- if (!isEncrypted) +- maxSplitPos = message.count() - overrun; +- +- splitPos = -1; +- for (const char *splitChar = splitter; *splitChar != 0; splitChar++) { +- splitPos = qMax(splitPos, message.lastIndexOf(*splitChar, maxSplitPos) + 1); // keep split char on old line +- } +- if (splitPos <= 0 || splitPos > maxSplitPos) +- splitPos = maxSplitPos; +- +- maxSplitPos = splitPos - 1; +- if (maxSplitPos <= 0) { // this should never happen, but who knows... +- qWarning() << tr("[Error] Could not encrypt your message: %1").arg(message.data()); +- return; +- } +- continue; // we never come back here for !encrypted! +- } +- +- // now we have found a valid splitpos (or didn't need to split to begin with) +- putCmd(cmd, QList() << target << crypted); +- if (splitPos < message.count()) +- putPrivmsg(target, message.mid(splitPos), cipher); ++ return QList() << targetEnc << splitMsgEnc; ++ }; + +- return; +- } ++ putCmd(cmd, network()->splitMessage(cmd, message, cmdGenerator)); + } + + +diff --git a/src/core/coreuserinputhandler.h b/src/core/coreuserinputhandler.h +index 69a429e..6e69ce6 100644 +--- a/src/core/coreuserinputhandler.h ++++ b/src/core/coreuserinputhandler.h +@@ -88,7 +88,7 @@ protected: + private: + void doMode(const BufferInfo& bufferInfo, const QChar &addOrRemove, const QChar &mode, const QString &nickList); + void banOrUnban(const BufferInfo &bufferInfo, const QString &text, bool ban); +- void putPrivmsg(const QByteArray &target, const QByteArray &message, Cipher *cipher = 0); ++ void putPrivmsg(const QString &target, const QString &message, std::function encodeFunc, Cipher *cipher = 0); + + #ifdef HAVE_QCA2 + QByteArray encrypt(const QString &target, const QByteArray &message, bool *didEncrypt = 0) const; +diff --git a/src/core/ctcpparser.cpp b/src/core/ctcpparser.cpp +index fba3d13..37b0af3 100644 +--- a/src/core/ctcpparser.cpp ++++ b/src/core/ctcpparser.cpp +@@ -312,29 +312,13 @@ QByteArray CtcpParser::pack(const QByteArray &ctcpTag, const QByteArray &message + + void CtcpParser::query(CoreNetwork *net, const QString &bufname, const QString &ctcpTag, const QString &message) + { +- QList params; +- params << net->serverEncode(bufname) << lowLevelQuote(pack(net->serverEncode(ctcpTag), net->userEncode(bufname, message))); +- +- static const char *splitter = " .,-!?"; +- int maxSplitPos = message.count(); +- int splitPos = maxSplitPos; ++ QString cmd("PRIVMSG"); + +- int overrun = net->userInputHandler()->lastParamOverrun("PRIVMSG", params); +- if (overrun) { +- maxSplitPos = message.count() - overrun -2; +- splitPos = -1; +- for (const char *splitChar = splitter; *splitChar != 0; splitChar++) { +- splitPos = qMax(splitPos, message.lastIndexOf(*splitChar, maxSplitPos) + 1); // keep split char on old line +- } +- if (splitPos <= 0 || splitPos > maxSplitPos) +- splitPos = maxSplitPos; +- +- params = params.mid(0, 1) << lowLevelQuote(pack(net->serverEncode(ctcpTag), net->userEncode(bufname, message.left(splitPos)))); +- } +- net->putCmd("PRIVMSG", params); ++ std::function(QString &)> cmdGenerator = [&] (QString &splitMsg) -> QList { ++ return QList() << net->serverEncode(bufname) << lowLevelQuote(pack(net->serverEncode(ctcpTag), net->userEncode(bufname, splitMsg))); ++ }; + +- if (splitPos < message.count()) +- query(net, bufname, ctcpTag, message.mid(splitPos)); ++ net->putCmd(cmd, net->splitMessage(cmd, message, cmdGenerator)); + } + + diff --git a/quassel.spec b/quassel.spec index 24f4b03..7b942c4 100755 --- a/quassel.spec +++ b/quassel.spec @@ -1,7 +1,7 @@ Name: quassel Summary: A modern distributed IRC system Version: 0.11.0 -Release: 1%{?dist} +Release: 2%{?dist} License: GPLv2 or GPLv3 Group: Applications/Internet @@ -20,6 +20,10 @@ Provides: %{name}-gui = %{version}-%{release} Requires: %{name}-common = %{version}-%{release} +# BZ1205130 - CTCP query Denial of Service +## Upstream patch git commit id b5e38970ffd55e2dd9f706ce75af9a8d7730b1b8 +Patch0: quassel-0.11.0-CTCP-query-crash.patch + %description Quassel IRC is a modern, distributed IRC client, meaning that one (or multiple) client(s) can attach @@ -60,6 +64,8 @@ Quassel client %prep %setup -q -n %{name}-%{version} +%patch0 -p1 + %build mkdir build pushd build @@ -116,6 +122,9 @@ gtk-update-icon-cache %{_kde4_iconsdir}/hicolor &> /dev/null || : %changelog +* Tue Mar 24 2015 Adam Miller - 0.11.0-2 +- BZ1205130 - patch for CTCP Denial of Service + * Wed Sep 24 2014 Adam Miller - 0.11.0-1 - Update to latest upstream From 95efb5ac885bcebb1cb208e2ea3265fc15f5b1fa Mon Sep 17 00:00:00 2001 From: Richard Hughes Date: Thu, 26 Mar 2015 16:42:06 +0000 Subject: [PATCH 2/8] Add an AppData file for the software center --- quassel.spec | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/quassel.spec b/quassel.spec index 7b942c4..dd98c5d 100755 --- a/quassel.spec +++ b/quassel.spec @@ -1,7 +1,7 @@ Name: quassel Summary: A modern distributed IRC system Version: 0.11.0 -Release: 2%{?dist} +Release: 3%{?dist} License: GPLv2 or GPLv3 Group: Applications/Internet @@ -79,6 +79,20 @@ rm -rf $RPM_BUILD_ROOT make install/fast DESTDIR=${RPM_BUILD_ROOT} -C build +# Merge applications into one software center item +mkdir -p $RPM_BUILD_ROOT%{_datadir}/appdata +cat > $RPM_BUILD_ROOT%{_datadir}/appdata/quasselclient.appdata.xml < + + + CC0-1.0 + quasselclient.desktop + + quassel.desktop + + +EOF + # unpackaged files rm -f $RPM_BUILD_ROOT%{_datadir}/pixmaps/quassel.png @@ -100,6 +114,7 @@ gtk-update-icon-cache %{_kde4_iconsdir}/hicolor &> /dev/null || : %files %defattr(-,root,root,-) %{_kde4_bindir}/quassel +%{_kde4_datadir}/appdata/quassel.appdata.xml %{_kde4_datadir}/applications/kde4/quassel.desktop %files common @@ -122,6 +137,9 @@ gtk-update-icon-cache %{_kde4_iconsdir}/hicolor &> /dev/null || : %changelog +* Thu Mar 26 2015 Richard Hughes - 0.11.0-3 +- Add an AppData file for the software center + * Tue Mar 24 2015 Adam Miller - 0.11.0-2 - BZ1205130 - patch for CTCP Denial of Service From 0ee8d27b53156ef6345fffb77ba7a170813cb420 Mon Sep 17 00:00:00 2001 From: Richard Hughes Date: Thu, 26 Mar 2015 17:28:56 +0000 Subject: [PATCH 3/8] trivial: Fix a typo in the last commit --- quassel.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quassel.spec b/quassel.spec index dd98c5d..d2efd50 100755 --- a/quassel.spec +++ b/quassel.spec @@ -114,7 +114,7 @@ gtk-update-icon-cache %{_kde4_iconsdir}/hicolor &> /dev/null || : %files %defattr(-,root,root,-) %{_kde4_bindir}/quassel -%{_kde4_datadir}/appdata/quassel.appdata.xml +%{_kde4_datadir}/appdata/quasselclient.appdata.xml %{_kde4_datadir}/applications/kde4/quassel.desktop %files common From 0f8a51efdb51fb16371481a4d46b2e42e01d7e0f Mon Sep 17 00:00:00 2001 From: Kalev Lember Date: Sat, 2 May 2015 17:20:48 +0200 Subject: [PATCH 4/8] Rebuilt for GCC 5 C++11 ABI change --- quassel.spec | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/quassel.spec b/quassel.spec index d2efd50..e9eec7d 100755 --- a/quassel.spec +++ b/quassel.spec @@ -1,7 +1,7 @@ Name: quassel Summary: A modern distributed IRC system Version: 0.11.0 -Release: 3%{?dist} +Release: 4%{?dist} License: GPLv2 or GPLv3 Group: Applications/Internet @@ -137,6 +137,9 @@ gtk-update-icon-cache %{_kde4_iconsdir}/hicolor &> /dev/null || : %changelog +* Sat May 02 2015 Kalev Lember - 0.11.0-4 +- Rebuilt for GCC 5 C++11 ABI change + * Thu Mar 26 2015 Richard Hughes - 0.11.0-3 - Add an AppData file for the software center From 4ffcbaac2907e1e0fce97b3327ffe4ce5bf58a35 Mon Sep 17 00:00:00 2001 From: Dennis Gilmore Date: Thu, 18 Jun 2015 21:56:22 +0000 Subject: [PATCH 5/8] - Rebuilt for https://fedoraproject.org/wiki/Fedora_23_Mass_Rebuild --- quassel.spec | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/quassel.spec b/quassel.spec index e9eec7d..e1e9da2 100755 --- a/quassel.spec +++ b/quassel.spec @@ -1,7 +1,7 @@ Name: quassel Summary: A modern distributed IRC system Version: 0.11.0 -Release: 4%{?dist} +Release: 5%{?dist} License: GPLv2 or GPLv3 Group: Applications/Internet @@ -137,6 +137,9 @@ gtk-update-icon-cache %{_kde4_iconsdir}/hicolor &> /dev/null || : %changelog +* Thu Jun 18 2015 Fedora Release Engineering - 0.11.0-5 +- Rebuilt for https://fedoraproject.org/wiki/Fedora_23_Mass_Rebuild + * Sat May 02 2015 Kalev Lember - 0.11.0-4 - Rebuilt for GCC 5 C++11 ABI change From 0836b99dcea8d6d7d5f7fa2b7e87081277485ec4 Mon Sep 17 00:00:00 2001 From: Adam Miller Date: Mon, 3 Aug 2015 11:42:52 -0500 Subject: [PATCH 6/8] update to latest upstream - 0.12.2 --- .gitignore | 1 + quassel-0.11.0-CTCP-query-crash.patch | 347 -------------------------- quassel.spec | 11 +- sources | 2 +- 4 files changed, 9 insertions(+), 352 deletions(-) delete mode 100644 quassel-0.11.0-CTCP-query-crash.patch diff --git a/.gitignore b/.gitignore index 76e5eac..acfd6ee 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ /quassel-0.9.2.tar.bz2 /quassel-0.10.0.tar.bz2 /quassel-0.11.0.tar.bz2 +/quassel-0.12.2.tar.bz2 diff --git a/quassel-0.11.0-CTCP-query-crash.patch b/quassel-0.11.0-CTCP-query-crash.patch deleted file mode 100644 index caf8f7f..0000000 --- a/quassel-0.11.0-CTCP-query-crash.patch +++ /dev/null @@ -1,347 +0,0 @@ -commit b5e38970ffd55e2dd9f706ce75af9a8d7730b1b8 -Author: Michael Marley -Date: Sat Feb 21 07:33:57 2015 -0500 - - Improve the message-splitting algorithm for PRIVMSG and CTCP - - This introduces a new message splitting algorithm based on - QTextBoundaryFinder. It works by first starting with the entire - message to be sent, encoding it, and checking to see if it is over - the maximum message length. If it is, it uses QTBF to find the - word boundary most immediately preceding the maximum length. If no - suitable boundary can be found, it falls back to searching for - grapheme boundaries. It repeats this process until the entire - message has been sent. - - Unlike what it replaces, the new splitting code is not recursive - and cannot cause stack overflows. Additionally, if it is unable - to split a string, it will give up gracefully and not crash the - core or cause a thread to run away. - - This patch fixes two bugs. The first is garbage characters caused - by accidentally splitting the string in the middle of a multibyte - character. Since the new code splits at a character level instead - of a byte level, this will no longer be an issue. The second is - the core crash caused by sending an overlength CTCP query ("/me") - containing only multibyte characters. This bug was caused by the - old CTCP splitter using the byte index from lastParamOverrun() as - a character index for a QString. - -diff --git a/src/core/corebasichandler.cpp b/src/core/corebasichandler.cpp -index dfa8a99..fbfc76c 100644 ---- a/src/core/corebasichandler.cpp -+++ b/src/core/corebasichandler.cpp -@@ -33,6 +33,9 @@ CoreBasicHandler::CoreBasicHandler(CoreNetwork *parent) - connect(this, SIGNAL(putCmd(QString, const QList &, const QByteArray &)), - network(), SLOT(putCmd(QString, const QList &, const QByteArray &))); - -+ connect(this, SIGNAL(putCmd(QString, const QList> &, const QByteArray &)), -+ network(), SLOT(putCmd(QString, const QList> &, const QByteArray &))); -+ - connect(this, SIGNAL(putRawLine(const QByteArray &)), - network(), SLOT(putRawLine(const QByteArray &))); - } -diff --git a/src/core/corebasichandler.h b/src/core/corebasichandler.h -index 20d057f..a4b5a7f 100644 ---- a/src/core/corebasichandler.h -+++ b/src/core/corebasichandler.h -@@ -55,6 +55,7 @@ public: - signals: - void displayMsg(Message::Type, BufferInfo::Type, const QString &target, const QString &text, const QString &sender = "", Message::Flags flags = Message::None); - void putCmd(const QString &cmd, const QList ¶ms, const QByteArray &prefix = QByteArray()); -+ void putCmd(const QString &cmd, const QList> ¶ms, const QByteArray &prefix = QByteArray()); - void putRawLine(const QByteArray &msg); - - protected: -diff --git a/src/core/corenetwork.cpp b/src/core/corenetwork.cpp -index 7e9ce26..932af6f 100644 ---- a/src/core/corenetwork.cpp -+++ b/src/core/corenetwork.cpp -@@ -284,6 +284,16 @@ void CoreNetwork::putCmd(const QString &cmd, const QList ¶ms, co - } - - -+void CoreNetwork::putCmd(const QString &cmd, const QList> ¶ms, const QByteArray &prefix) -+{ -+ QListIterator> i(params); -+ while (i.hasNext()) { -+ QList msg = i.next(); -+ putCmd(cmd, msg, prefix); -+ } -+} -+ -+ - void CoreNetwork::setChannelJoined(const QString &channel) - { - _autoWhoQueue.prepend(channel.toLower()); // prepend so this new chan is the first to be checked -@@ -980,3 +990,79 @@ void CoreNetwork::requestSetNetworkInfo(const NetworkInfo &info) - } - } - } -+ -+ -+QList> CoreNetwork::splitMessage(const QString &cmd, const QString &message, std::function(QString &)> cmdGenerator) -+{ -+ QString wrkMsg(message); -+ QList> msgsToSend; -+ -+ // do while (wrkMsg.size() > 0) -+ do { -+ // First, check to see if the whole message can be sent at once. The -+ // cmdGenerator function is passed in by the caller and is used to encode -+ // and encrypt (if applicable) the message, since different callers might -+ // want to use different encoding or encode different values. -+ int splitPos = wrkMsg.size(); -+ QList initialSplitMsgEnc = cmdGenerator(wrkMsg); -+ int initialOverrun = userInputHandler()->lastParamOverrun(cmd, initialSplitMsgEnc); -+ -+ if (initialOverrun) { -+ // If the message was too long to be sent, first try splitting it along -+ // word boundaries with QTextBoundaryFinder. -+ QString splitMsg(wrkMsg); -+ QTextBoundaryFinder qtbf(QTextBoundaryFinder::Word, splitMsg); -+ qtbf.setPosition(initialSplitMsgEnc[1].size() - initialOverrun); -+ QList splitMsgEnc; -+ int overrun = initialOverrun; -+ -+ while (overrun) { -+ splitPos = qtbf.toPreviousBoundary(); -+ -+ // splitPos==-1 means the QTBF couldn't find a split point at all and -+ // splitPos==0 means the QTBF could only find a boundary at the beginning of -+ // the string. Neither one of these works for us. -+ if (splitPos > 0) { -+ // If a split point could be found, split the message there, calculate the -+ // overrun, and continue with the loop. -+ splitMsg = splitMsg.left(splitPos); -+ splitMsgEnc = cmdGenerator(splitMsg); -+ overrun = userInputHandler()->lastParamOverrun(cmd, splitMsgEnc); -+ } -+ else { -+ // If a split point could not be found (the beginning of the message -+ // is reached without finding a split point short enough to send) and we -+ // are still in Word mode, switch to Grapheme mode. We also need to restore -+ // the full wrkMsg to splitMsg, since splitMsg may have been cut down during -+ // the previous attempt to find a split point. -+ if (qtbf.type() == QTextBoundaryFinder::Word) { -+ splitMsg = wrkMsg; -+ splitPos = splitMsg.size(); -+ QTextBoundaryFinder graphemeQtbf(QTextBoundaryFinder::Grapheme, splitMsg); -+ graphemeQtbf.setPosition(initialSplitMsgEnc[1].size() - initialOverrun); -+ qtbf = graphemeQtbf; -+ } -+ else { -+ // If the QTBF fails to find a split point in Grapheme mode, we give up. -+ // This should never happen, but it should be handled anyway. -+ qWarning() << "Unexpected failure to split message!"; -+ return msgsToSend; -+ } -+ } -+ } -+ -+ // Once a message of sendable length has been found, remove it from the wrkMsg and -+ // add it to the list of messages to be sent. -+ wrkMsg.remove(0, splitPos); -+ msgsToSend.append(splitMsgEnc); -+ } -+ else{ -+ // If the entire remaining message is short enough to be sent all at once, remove -+ // it from the wrkMsg and add it to the list of messages to be sent. -+ wrkMsg.remove(0, splitPos); -+ msgsToSend.append(initialSplitMsgEnc); -+ } -+ } while (wrkMsg.size() > 0); -+ -+ return msgsToSend; -+} -diff --git a/src/core/corenetwork.h b/src/core/corenetwork.h -index 87121ba..05565a4 100644 ---- a/src/core/corenetwork.h -+++ b/src/core/corenetwork.h -@@ -40,6 +40,8 @@ - - #include "coresession.h" - -+#include -+ - class CoreIdentity; - class CoreUserInputHandler; - class CoreIgnoreListManager; -@@ -93,6 +95,8 @@ public: - inline quint16 localPort() const { return socket.localPort(); } - inline quint16 peerPort() const { return socket.peerPort(); } - -+ QList> splitMessage(const QString &cmd, const QString &message, std::function(QString &)> cmdGenerator); -+ - public slots: - virtual void setMyNick(const QString &mynick); - -@@ -112,6 +116,7 @@ public slots: - void userInput(BufferInfo bufferInfo, QString msg); - void putRawLine(QByteArray input); - void putCmd(const QString &cmd, const QList ¶ms, const QByteArray &prefix = QByteArray()); -+ void putCmd(const QString &cmd, const QList> ¶ms, const QByteArray &prefix = QByteArray()); - - void setChannelJoined(const QString &channel); - void setChannelParted(const QString &channel); -diff --git a/src/core/coreuserinputhandler.cpp b/src/core/coreuserinputhandler.cpp -index 33d1f67..72ac996 100644 ---- a/src/core/coreuserinputhandler.cpp -+++ b/src/core/coreuserinputhandler.cpp -@@ -473,12 +473,16 @@ void CoreUserInputHandler::handleMsg(const BufferInfo &bufferInfo, const QString - return; - - QString target = msg.section(' ', 0, 0); -- QByteArray encMsg = userEncode(target, msg.section(' ', 1)); -+ QString msgSection = msg.section(' ', 1); -+ -+ std::function encodeFunc = [this] (const QString &target, const QString &message) -> QByteArray { -+ return userEncode(target, message); -+ }; - - #ifdef HAVE_QCA2 -- putPrivmsg(serverEncode(target), encMsg, network()->cipher(target)); -+ putPrivmsg(target, msgSection, encodeFunc, network()->cipher(target)); - #else -- putPrivmsg(serverEncode(target), encMsg); -+ putPrivmsg(target, msgSection, encodeFunc); - #endif - } - -@@ -594,11 +598,14 @@ void CoreUserInputHandler::handleSay(const BufferInfo &bufferInfo, const QString - if (bufferInfo.bufferName().isEmpty() || !bufferInfo.acceptsRegularMessages()) - return; // server buffer - -- QByteArray encMsg = channelEncode(bufferInfo.bufferName(), msg); -+ std::function encodeFunc = [this] (const QString &target, const QString &message) -> QByteArray { -+ return channelEncode(target, message); -+ }; -+ - #ifdef HAVE_QCA2 -- putPrivmsg(serverEncode(bufferInfo.bufferName()), encMsg, network()->cipher(bufferInfo.bufferName())); -+ putPrivmsg(bufferInfo.bufferName(), msg, encodeFunc, network()->cipher(bufferInfo.bufferName())); - #else -- putPrivmsg(serverEncode(bufferInfo.bufferName()), encMsg); -+ putPrivmsg(bufferInfo.bufferName(), msg, encodeFunc); - #endif - emit displayMsg(Message::Plain, bufferInfo.type(), bufferInfo.bufferName(), msg, network()->myNick(), Message::Self); - } -@@ -763,56 +770,23 @@ void CoreUserInputHandler::defaultHandler(QString cmd, const BufferInfo &bufferI - } - - --void CoreUserInputHandler::putPrivmsg(const QByteArray &target, const QByteArray &message, Cipher *cipher) -+void CoreUserInputHandler::putPrivmsg(const QString &target, const QString &message, std::function encodeFunc, Cipher *cipher) - { -- // Encrypted messages need special care. There's no clear relation between cleartext and encrypted message length, -- // so we can't just compute the maxSplitPos. Instead, we need to loop through the splitpoints until the crypted -- // version is short enough... -- // TODO: check out how the various possible encryption methods behave length-wise and make -- // this clean by predicting the length of the crypted msg. -- // For example, blowfish-ebc seems to create 8-char chunks. -+ QString cmd("PRIVMSG"); -+ QByteArray targetEnc = serverEncode(target); - -- static const char *cmd = "PRIVMSG"; -- static const char *splitter = " .,-!?"; -+ std::function(QString &)> cmdGenerator = [&] (QString &splitMsg) -> QList { -+ QByteArray splitMsgEnc = encodeFunc(target, splitMsg); - -- int maxSplitPos = message.count(); -- int splitPos = maxSplitPos; -- forever { -- QByteArray crypted = message.left(splitPos); -- bool isEncrypted = false; - #ifdef HAVE_QCA2 -- if (cipher && !cipher->key().isEmpty() && !message.isEmpty()) { -- isEncrypted = cipher->encrypt(crypted); -+ if (cipher && !cipher->key().isEmpty() && !splitMsg.isEmpty()) { -+ cipher->encrypt(splitMsgEnc); - } - #endif -- int overrun = lastParamOverrun(cmd, QList() << target << crypted); -- if (overrun) { -- // In case this is not an encrypted msg, we can just cut off at the end -- if (!isEncrypted) -- maxSplitPos = message.count() - overrun; -- -- splitPos = -1; -- for (const char *splitChar = splitter; *splitChar != 0; splitChar++) { -- splitPos = qMax(splitPos, message.lastIndexOf(*splitChar, maxSplitPos) + 1); // keep split char on old line -- } -- if (splitPos <= 0 || splitPos > maxSplitPos) -- splitPos = maxSplitPos; -- -- maxSplitPos = splitPos - 1; -- if (maxSplitPos <= 0) { // this should never happen, but who knows... -- qWarning() << tr("[Error] Could not encrypt your message: %1").arg(message.data()); -- return; -- } -- continue; // we never come back here for !encrypted! -- } -- -- // now we have found a valid splitpos (or didn't need to split to begin with) -- putCmd(cmd, QList() << target << crypted); -- if (splitPos < message.count()) -- putPrivmsg(target, message.mid(splitPos), cipher); -+ return QList() << targetEnc << splitMsgEnc; -+ }; - -- return; -- } -+ putCmd(cmd, network()->splitMessage(cmd, message, cmdGenerator)); - } - - -diff --git a/src/core/coreuserinputhandler.h b/src/core/coreuserinputhandler.h -index 69a429e..6e69ce6 100644 ---- a/src/core/coreuserinputhandler.h -+++ b/src/core/coreuserinputhandler.h -@@ -88,7 +88,7 @@ protected: - private: - void doMode(const BufferInfo& bufferInfo, const QChar &addOrRemove, const QChar &mode, const QString &nickList); - void banOrUnban(const BufferInfo &bufferInfo, const QString &text, bool ban); -- void putPrivmsg(const QByteArray &target, const QByteArray &message, Cipher *cipher = 0); -+ void putPrivmsg(const QString &target, const QString &message, std::function encodeFunc, Cipher *cipher = 0); - - #ifdef HAVE_QCA2 - QByteArray encrypt(const QString &target, const QByteArray &message, bool *didEncrypt = 0) const; -diff --git a/src/core/ctcpparser.cpp b/src/core/ctcpparser.cpp -index fba3d13..37b0af3 100644 ---- a/src/core/ctcpparser.cpp -+++ b/src/core/ctcpparser.cpp -@@ -312,29 +312,13 @@ QByteArray CtcpParser::pack(const QByteArray &ctcpTag, const QByteArray &message - - void CtcpParser::query(CoreNetwork *net, const QString &bufname, const QString &ctcpTag, const QString &message) - { -- QList params; -- params << net->serverEncode(bufname) << lowLevelQuote(pack(net->serverEncode(ctcpTag), net->userEncode(bufname, message))); -- -- static const char *splitter = " .,-!?"; -- int maxSplitPos = message.count(); -- int splitPos = maxSplitPos; -+ QString cmd("PRIVMSG"); - -- int overrun = net->userInputHandler()->lastParamOverrun("PRIVMSG", params); -- if (overrun) { -- maxSplitPos = message.count() - overrun -2; -- splitPos = -1; -- for (const char *splitChar = splitter; *splitChar != 0; splitChar++) { -- splitPos = qMax(splitPos, message.lastIndexOf(*splitChar, maxSplitPos) + 1); // keep split char on old line -- } -- if (splitPos <= 0 || splitPos > maxSplitPos) -- splitPos = maxSplitPos; -- -- params = params.mid(0, 1) << lowLevelQuote(pack(net->serverEncode(ctcpTag), net->userEncode(bufname, message.left(splitPos)))); -- } -- net->putCmd("PRIVMSG", params); -+ std::function(QString &)> cmdGenerator = [&] (QString &splitMsg) -> QList { -+ return QList() << net->serverEncode(bufname) << lowLevelQuote(pack(net->serverEncode(ctcpTag), net->userEncode(bufname, splitMsg))); -+ }; - -- if (splitPos < message.count()) -- query(net, bufname, ctcpTag, message.mid(splitPos)); -+ net->putCmd(cmd, net->splitMessage(cmd, message, cmdGenerator)); - } - - diff --git a/quassel.spec b/quassel.spec index e1e9da2..0ed124b 100755 --- a/quassel.spec +++ b/quassel.spec @@ -1,7 +1,7 @@ Name: quassel Summary: A modern distributed IRC system -Version: 0.11.0 -Release: 5%{?dist} +Version: 0.12.2 +Release: 1%{?dist} License: GPLv2 or GPLv3 Group: Applications/Internet @@ -22,7 +22,7 @@ Requires: %{name}-common = %{version}-%{release} # BZ1205130 - CTCP query Denial of Service ## Upstream patch git commit id b5e38970ffd55e2dd9f706ce75af9a8d7730b1b8 -Patch0: quassel-0.11.0-CTCP-query-crash.patch +#Patch0: quassel-0.11.0-CTCP-query-crash.patch %description Quassel IRC is a modern, distributed IRC client, @@ -64,7 +64,7 @@ Quassel client %prep %setup -q -n %{name}-%{version} -%patch0 -p1 +#%patch0 -p1 %build mkdir build @@ -137,6 +137,9 @@ gtk-update-icon-cache %{_kde4_iconsdir}/hicolor &> /dev/null || : %changelog +* Mon Aug 03 2015 Adam Miller - 0.12.2-1 +- Update to latest upstream release. + * Thu Jun 18 2015 Fedora Release Engineering - 0.11.0-5 - Rebuilt for https://fedoraproject.org/wiki/Fedora_23_Mass_Rebuild diff --git a/sources b/sources index 5d2f8e2..2ea948b 100644 --- a/sources +++ b/sources @@ -1 +1 @@ -b6a89db333eb225760f95becd3680ca8 quassel-0.11.0.tar.bz2 +f5473a9c5927a0e8cb3a204ced887aa8 quassel-0.12.2.tar.bz2 From cd55ae5ec99022cde2abe50e4d1a6199914d9b2b Mon Sep 17 00:00:00 2001 From: Richard Hughes Date: Wed, 16 Sep 2015 17:37:44 +0100 Subject: [PATCH 7/8] Remove the AppData file as the desktop file is no longer valid --- quassel.spec | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/quassel.spec b/quassel.spec index 0ed124b..0133183 100755 --- a/quassel.spec +++ b/quassel.spec @@ -1,7 +1,7 @@ Name: quassel Summary: A modern distributed IRC system Version: 0.12.2 -Release: 1%{?dist} +Release: 2%{?dist} License: GPLv2 or GPLv3 Group: Applications/Internet @@ -79,20 +79,6 @@ rm -rf $RPM_BUILD_ROOT make install/fast DESTDIR=${RPM_BUILD_ROOT} -C build -# Merge applications into one software center item -mkdir -p $RPM_BUILD_ROOT%{_datadir}/appdata -cat > $RPM_BUILD_ROOT%{_datadir}/appdata/quasselclient.appdata.xml < - - - CC0-1.0 - quasselclient.desktop - - quassel.desktop - - -EOF - # unpackaged files rm -f $RPM_BUILD_ROOT%{_datadir}/pixmaps/quassel.png @@ -114,7 +100,6 @@ gtk-update-icon-cache %{_kde4_iconsdir}/hicolor &> /dev/null || : %files %defattr(-,root,root,-) %{_kde4_bindir}/quassel -%{_kde4_datadir}/appdata/quasselclient.appdata.xml %{_kde4_datadir}/applications/kde4/quassel.desktop %files common @@ -137,6 +122,9 @@ gtk-update-icon-cache %{_kde4_iconsdir}/hicolor &> /dev/null || : %changelog +* Thu Mar 26 2015 Richard Hughes - 0.12.2-2 +- Remove the AppData file as the desktop file is no longer valid. + * Mon Aug 03 2015 Adam Miller - 0.12.2-1 - Update to latest upstream release. From f1f43971a3969207bc6fd275fa89bb3c90ff1aee Mon Sep 17 00:00:00 2001 From: Richard Hughes Date: Wed, 16 Sep 2015 17:42:36 +0100 Subject: [PATCH 8/8] Fix date --- quassel.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quassel.spec b/quassel.spec index 0133183..2046890 100755 --- a/quassel.spec +++ b/quassel.spec @@ -122,7 +122,7 @@ gtk-update-icon-cache %{_kde4_iconsdir}/hicolor &> /dev/null || : %changelog -* Thu Mar 26 2015 Richard Hughes - 0.12.2-2 +* Wed Sep 16 2015 Richard Hughes - 0.12.2-2 - Remove the AppData file as the desktop file is no longer valid. * Mon Aug 03 2015 Adam Miller - 0.12.2-1