Dear maintainer,
I've been using the patched build locally for 2 working days without
issues, so I think it's time to push it to unstable for wider testing.
This is quite a complex patch for this late in the release cycle, but
I really don't see an option for a less complex one. But I suggest we
let it spend a few days in unstable before seeking an unblock request.
Attached is the nmudiff.
Cheers,
Olly
diff -Nru quassel-0.10.0/debian/changelog quassel-0.10.0/debian/changelog
--- quassel-0.10.0/debian/changelog 2014-11-09 02:29:37.000000000 +1300
+++ quassel-0.10.0/debian/changelog 2015-04-01 11:44:18.000000000 +1300
@@ -1,3 +1,12 @@
+quassel (1:0.10.0-2.3) unstable; urgency=high
+
+ * Non-maintainer upload with maintainer's permission.
+ * Improve the message-splitting algorithm for PRIVMSG and CTCP. Original
+ patch from Michael Marley, backported by Steinar H. Gunderson. Fixes
+ CVE-2015-2778 and CVE-2015-2779. (Closes: #781024)
+
+ -- Olly Betts <[email protected]> Wed, 01 Apr 2015 11:41:28 +1300
+
quassel (1:0.10.0-2.2) unstable; urgency=high
* Non-maintainer upload.
diff -Nru quassel-0.10.0/debian/patches/CVE-2015-2778.patch quassel-0.10.0/debian/patches/CVE-2015-2778.patch
--- quassel-0.10.0/debian/patches/CVE-2015-2778.patch 1970-01-01 12:00:00.000000000 +1200
+++ quassel-0.10.0/debian/patches/CVE-2015-2778.patch 2015-04-01 11:37:08.000000000 +1300
@@ -0,0 +1,421 @@
+>From b5e38970ffd55e2dd9f706ce75af9a8d7730b1b8 Mon Sep 17 00:00:00 2001
+From: Michael Marley <[email protected]>
+Date: Sat, 21 Feb 2015 07:33:57 -0500
+Subject: [PATCH] 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.
+---
+ src/core/corebasichandler.cpp | 3 ++
+ src/core/corebasichandler.h | 1 +
+ src/core/corenetwork.cpp | 86 +++++++++++++++++++++++++++++++++++++++
+ src/core/corenetwork.h | 5 +++
+ src/core/coreuserinputhandler.cpp | 72 +++++++++++---------------------
+ src/core/coreuserinputhandler.h | 2 +-
+ src/core/ctcpparser.cpp | 26 +++---------
+ 7 files changed, 124 insertions(+), 71 deletions(-)
+
+Index: quassel-0.10.0/src/core/corebasichandler.cpp
+===================================================================
+--- quassel-0.10.0.orig/src/core/corebasichandler.cpp
++++ quassel-0.10.0/src/core/corebasichandler.cpp
+@@ -33,6 +33,9 @@ CoreBasicHandler::CoreBasicHandler(CoreN
+ connect(this, SIGNAL(putCmd(QString, const QList<QByteArray> &, const QByteArray &)),
+ network(), SLOT(putCmd(QString, const QList<QByteArray> &, const QByteArray &)));
+
++ connect(this, SIGNAL(putCmd(QString, const QList<QList<QByteArray> > &, const QByteArray &)),
++ network(), SLOT(putCmd(QString, const QList<QList<QByteArray> > &, const QByteArray &)));
++
+ connect(this, SIGNAL(putRawLine(const QByteArray &)),
+ network(), SLOT(putRawLine(const QByteArray &)));
+ }
+Index: quassel-0.10.0/src/core/corebasichandler.h
+===================================================================
+--- quassel-0.10.0.orig/src/core/corebasichandler.h
++++ quassel-0.10.0/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<QByteArray> ¶ms, const QByteArray &prefix = QByteArray());
++ void putCmd(const QString &cmd, const QList<QList<QByteArray> > ¶ms, const QByteArray &prefix = QByteArray());
+ void putRawLine(const QByteArray &msg);
+
+ protected:
+Index: quassel-0.10.0/src/core/corenetwork.cpp
+===================================================================
+--- quassel-0.10.0.orig/src/core/corenetwork.cpp
++++ quassel-0.10.0/src/core/corenetwork.cpp
+@@ -283,6 +283,16 @@ void CoreNetwork::putCmd(const QString &
+ }
+
+
++void CoreNetwork::putCmd(const QString &cmd, const QList<QList<QByteArray> > ¶ms, const QByteArray &prefix)
++{
++ QListIterator<QList<QByteArray> > i(params);
++ while (i.hasNext()) {
++ QList<QByteArray> 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
+@@ -979,3 +989,82 @@ void CoreNetwork::requestSetNetworkInfo(
+ }
+ }
+ }
++
++
++CoreNetwork::SplitGenerator::~SplitGenerator() {}
++
++
++QList<QList<QByteArray> > CoreNetwork::splitMessage(const QString &cmd, const QString &message, SplitGenerator *cmdGenerator)
++{
++ QString wrkMsg(message);
++ QList<QList<QByteArray> > 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<QByteArray> 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<QByteArray> 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;
++}
+Index: quassel-0.10.0/src/core/corenetwork.h
+===================================================================
+--- quassel-0.10.0.orig/src/core/corenetwork.h
++++ quassel-0.10.0/src/core/corenetwork.h
+@@ -93,6 +93,13 @@ public:
+ inline quint16 localPort() const { return socket.localPort(); }
+ inline quint16 peerPort() const { return socket.peerPort(); }
+
++ // Interface equivalent to a std::function<QList<QByteArray>(QString &)>.
++ struct SplitGenerator {
++ virtual ~SplitGenerator() = 0;
++ virtual QList<QByteArray> operator() (QString &) = 0;
++ };
++ QList<QList<QByteArray> > splitMessage(const QString &cmd, const QString &message, SplitGenerator *cmdGenerator);
++
+ public slots:
+ virtual void setMyNick(const QString &mynick);
+
+@@ -112,6 +119,7 @@ public slots:
+ void userInput(BufferInfo bufferInfo, QString msg);
+ void putRawLine(QByteArray input);
+ void putCmd(const QString &cmd, const QList<QByteArray> ¶ms, const QByteArray &prefix = QByteArray());
++ void putCmd(const QString &cmd, const QList<QList<QByteArray> > ¶ms, const QByteArray &prefix = QByteArray());
+
+ void setChannelJoined(const QString &channel);
+ void setChannelParted(const QString &channel);
+Index: quassel-0.10.0/src/core/coreuserinputhandler.cpp
+===================================================================
+--- quassel-0.10.0.orig/src/core/coreuserinputhandler.cpp
++++ quassel-0.10.0/src/core/coreuserinputhandler.cpp
+@@ -468,7 +468,6 @@ void CoreUserInputHandler::handleMode(co
+ emit putCmd("MODE", serverEncode(params));
+ }
+
+-
+ // TODO: show privmsgs
+ void CoreUserInputHandler::handleMsg(const BufferInfo &bufferInfo, const QString &msg)
+ {
+@@ -477,16 +476,15 @@ void CoreUserInputHandler::handleMsg(con
+ return;
+
+ QString target = msg.section(' ', 0, 0);
+- QByteArray encMsg = userEncode(target, msg.section(' ', 1));
++ QString msgSection = msg.section(' ', 1);
+
+ #ifdef HAVE_QCA2
+- putPrivmsg(serverEncode(target), encMsg, network()->cipher(target));
++ putPrivmsg(target, msgSection, /*encodeForChannel=*/false, network()->cipher(target));
+ #else
+- putPrivmsg(serverEncode(target), encMsg);
++ putPrivmsg(target, msgSection, /*encodeForChannel=*/false);
+ #endif
+ }
+
+-
+ void CoreUserInputHandler::handleNick(const BufferInfo &bufferInfo, const QString &msg)
+ {
+ Q_UNUSED(bufferInfo)
+@@ -588,11 +586,10 @@ void CoreUserInputHandler::handleSay(con
+ if (bufferInfo.bufferName().isEmpty() || !bufferInfo.acceptsRegularMessages())
+ return; // server buffer
+
+- QByteArray encMsg = channelEncode(bufferInfo.bufferName(), msg);
+ #ifdef HAVE_QCA2
+- putPrivmsg(serverEncode(bufferInfo.bufferName()), encMsg, network()->cipher(bufferInfo.bufferName()));
++ putPrivmsg(bufferInfo.bufferName(), msg, /*encodeForChannel=*/true, network()->cipher(bufferInfo.bufferName()));
+ #else
+- putPrivmsg(serverEncode(bufferInfo.bufferName()), encMsg);
++ putPrivmsg(bufferInfo.bufferName(), msg, /*encodeForChannel=*/true);
+ #endif
+ emit displayMsg(Message::Plain, bufferInfo.type(), bufferInfo.bufferName(), msg, network()->myNick(), Message::Self);
+ }
+@@ -757,56 +754,37 @@ void CoreUserInputHandler::defaultHandle
+ }
+
+
+-void CoreUserInputHandler::putPrivmsg(const QByteArray &target, const QByteArray &message, 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.
+-
+- static const char *cmd = "PRIVMSG";
+- static const char *splitter = " .,-";
+-
+- int maxSplitPos = message.count();
+- int splitPos = maxSplitPos;
+- forever {
+- QByteArray crypted = message.left(splitPos);
+- bool isEncrypted = false;
++CoreUserInputHandler::MsgSplitGenerator::MsgSplitGenerator(CoreUserInputHandler *parent, const QString &target, bool encodeForChannel, Cipher *cipher)
++ : _parent(parent), _target(target), _encodeForChannel(encodeForChannel), _cipher(cipher) {}
++
++
++CoreUserInputHandler::MsgSplitGenerator::~MsgSplitGenerator() {}
++
++
++QList<QByteArray> CoreUserInputHandler::CoreUserInputHandler::MsgSplitGenerator::operator() (QString &splitMsg) {
++ QByteArray splitMsgEnc;
++ if (_encodeForChannel) {
++ splitMsgEnc = _parent->channelEncode(_target, splitMsg);
++ } else {
++ splitMsgEnc = _parent->userEncode(_target, splitMsg);
++ }
++
+ #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<QByteArray>() << 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<QByteArray>() << target << crypted);
+- if (splitPos < message.count())
+- putPrivmsg(target, message.mid(splitPos), cipher);
++ QByteArray targetEnc = _parent->serverEncode(_target);
++ return QList<QByteArray>() << targetEnc << splitMsgEnc;
++}
+
+- return;
+- }
++
++void CoreUserInputHandler::putPrivmsg(const QString &target, const QString &message, bool encodeForChannel, Cipher *cipher)
++{
++ QString cmd("PRIVMSG");
++
++ MsgSplitGenerator cmdGenerator(this, target, encodeForChannel, cipher);
++ putCmd(cmd, network()->splitMessage(cmd, message, &cmdGenerator));
+ }
+
+
+Index: quassel-0.10.0/src/core/coreuserinputhandler.h
+===================================================================
+--- quassel-0.10.0.orig/src/core/coreuserinputhandler.h
++++ quassel-0.10.0/src/core/coreuserinputhandler.h
+@@ -36,6 +36,7 @@ public:
+ inline CoreNetwork *coreNetwork() const { return qobject_cast<CoreNetwork *>(parent()); }
+
+ void handleUserInput(const BufferInfo &bufferInfo, const QString &text);
++ int lastParamOverrun(const QString &cmd, const QList<QByteArray> ¶ms);
+
+ public slots:
+ void handleAway(const BufferInfo &bufferInfo, const QString &text);
+@@ -86,8 +87,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);
+- int lastParamOverrun(const QString &cmd, const QList<QByteArray> ¶ms);
++ void putPrivmsg(const QString &target, const QString &message, bool encodeForChannel, Cipher *cipher = 0);
+
+ #ifdef HAVE_QCA2
+ QByteArray encrypt(const QString &target, const QByteArray &message, bool *didEncrypt = 0) const;
+@@ -100,6 +100,20 @@ private:
+ Command() {}
+ };
+
++ // A SplitGenerator that generates privmsgs.
++ class MsgSplitGenerator : public CoreNetwork::SplitGenerator {
++ public:
++ MsgSplitGenerator(CoreUserInputHandler *parent, const QString &target, bool encodeForChannel, Cipher *cipher);
++ virtual ~MsgSplitGenerator();
++ virtual QList<QByteArray> operator() (QString &splitMsg);
++
++ private:
++ CoreUserInputHandler *_parent;
++ const QString &_target;
++ bool _encodeForChannel;
++ Cipher *_cipher;
++ };
++
+ QHash<int, Command> _delayedCommands;
+ };
+
+Index: quassel-0.10.0/src/core/ctcpparser.cpp
+===================================================================
+--- quassel-0.10.0.orig/src/core/ctcpparser.cpp
++++ quassel-0.10.0/src/core/ctcpparser.cpp
+@@ -309,11 +309,21 @@ QByteArray CtcpParser::pack(const QByteA
+ }
+
+
++CtcpParser::CtcpSplitGenerator::CtcpSplitGenerator(
++ CtcpParser *parent, CoreNetwork *net, const QString &bufname, const QString &ctcpTag)
++ : _parent(parent), _net(net), _bufname(bufname), _ctcpTag(ctcpTag) {}
++
++CtcpParser::CtcpSplitGenerator::~CtcpSplitGenerator() {}
++
++QList<QByteArray> CtcpParser::CtcpSplitGenerator::operator() (QString &splitMsg) {
++ return QList<QByteArray>() << _net->serverEncode(_bufname) << _parent->lowLevelQuote(_parent->pack(_net->serverEncode(_ctcpTag), _net->userEncode(_bufname, splitMsg)));
++}
++
+ void CtcpParser::query(CoreNetwork *net, const QString &bufname, const QString &ctcpTag, const QString &message)
+ {
+- QList<QByteArray> params;
+- params << net->serverEncode(bufname) << lowLevelQuote(pack(net->serverEncode(ctcpTag), net->userEncode(bufname, message)));
+- net->putCmd("PRIVMSG", params);
++ QString cmd("PRIVMSG");
++ CtcpSplitGenerator cmdGenerator(this, net, bufname, ctcpTag);
++ net->putCmd(cmd, net->splitMessage(cmd, message, &cmdGenerator));
+ }
+
+
+Index: quassel-0.10.0/src/core/ctcpparser.h
+===================================================================
+--- quassel-0.10.0.orig/src/core/ctcpparser.h
++++ quassel-0.10.0/src/core/ctcpparser.h
+@@ -92,6 +92,20 @@ private:
+ CtcpReply(CoreNetwork *net, const QString &buf) : network(net), bufferName(buf) {}
+ };
+
++ // A SplitGenerator that generates CTCP messages.
++ class CtcpSplitGenerator : public CoreNetwork::SplitGenerator {
++ public:
++ CtcpSplitGenerator(CtcpParser *parent, CoreNetwork *net, const QString &bufname, const QString &ctcpTag);
++ virtual ~CtcpSplitGenerator();
++ virtual QList<QByteArray> operator() (QString &splitMsg);
++
++ private:
++ CtcpParser *_parent;
++ CoreNetwork *_net;
++ const QString &_bufname;
++ const QString &_ctcpTag;
++ };
++
+ QHash<QUuid, CtcpReply> _replies;
+
+ QHash<QByteArray, QByteArray> _ctcpMDequoteHash;
diff -Nru quassel-0.10.0/debian/patches/series quassel-0.10.0/debian/patches/series
--- quassel-0.10.0/debian/patches/series 2014-10-29 05:16:01.000000000 +1300
+++ quassel-0.10.0/debian/patches/series 2015-04-01 11:37:31.000000000 +1300
@@ -1,2 +1,3 @@
01_default_network_channel.patch
CVE-2014-8483.patch
+CVE-2015-2778.patch
