[Konversation-devel] [Bug 246576] Crash on invalid input data from server

[Eike Hein]

https://bugs.kde.org/show_bug.cgi?id=246576


Eike Hein <hein at kde.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED




--- Comment #2 from Eike Hein <hein kde org>  2010-08-03 11:30:28 ---
commit a2acea6319ec0308527ee656b1df200f1fe00f28
Author: Eike Hein <hein at kde.org>
Date:   Tue Aug 3 11:29:37 2010 +0200

    Don't crash on Jonas Thiem's fuzzer.

    Fixed a small loop logic error in incoming(), a missing length
    sanity check after the encoding pass and a source of QList::last()
    calls on an empty list.

    BUG:246576

diff --git a/ChangeLog b/ChangeLog
index 396004b..3c10d30 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -115,6 +115,7 @@ Changes since 1.3.1:
     tection of email addresses.
   - Average performance of link detection has improved slightly.
 * Incoming actions (i.e. "/me") without an argument are now handled properly.
+* Fixed a number of crashes on illegal data from the server.


 Changes from 1.3 to 1.3.1:
diff --git a/src/commit.h b/src/commit.h
index 2946f3b..8a5cc24 100644
--- a/src/commit.h
+++ b/src/commit.h
@@ -1,4 +1,4 @@
 // This COMMIT number is added to version string to be used as "patch level"
 #ifndef COMMIT
-#define COMMIT 4068
+#define COMMIT 4069
 #endif
diff --git a/src/irc/inputfilter.cpp b/src/irc/inputfilter.cpp
index ba1828a..7948c63 100644
--- a/src/irc/inputfilter.cpp
+++ b/src/irc/inputfilter.cpp
@@ -153,7 +153,7 @@ void InputFilter::parseLine(const QString& line)
     }
 }

-#define trailing parameterList.last()
+#define trailing (parameterList.isEmpty() ? QString() : parameterList.last())
 #define plHas(x) _plHas(parameterList.count(), (x))

 bool _plHad=false;
diff --git a/src/irc/server.cpp b/src/irc/server.cpp
index 8166332..72a3bba 100644
--- a/src/irc/server.cpp
+++ b/src/irc/server.cpp
@@ -1100,13 +1100,15 @@ void Server::incoming()
         QTextCodec* codec = getIdentity()->getCodec();
         QByteArray first = bufferLines.first();

+        bufferLines.removeFirst();
+
         QStringList lineSplit = codec->toUnicode(first).split(' ',
QString::SkipEmptyParts);

-        if( lineSplit.count() >= 1 )
+        if (lineSplit.count() >= 1)
         {
-            if( lineSplit[0][0] == ':' )          // does this message have a
prefix?
+            if (lineSplit[0][0] == ':')          // does this message have a
prefix?
             {
-                if( !lineSplit[0].contains('!') ) // is this a server(global)
message?
+                if(!lineSplit[0].contains('!')) // is this a server(global)
message?
                     isServerMessage = true;
                 else
                     senderNick = lineSplit[0].mid(1,
lineSplit[0].indexOf('!')-1);
@@ -1114,13 +1116,9 @@ void Server::incoming()
                 lineSplit.removeFirst();          // remove prefix
             }
         }
-        else
-        {
-            // The line contained only spaces (other than CRLF, removed above)
-            // and thus there's nothing more we can do with it.
-            bufferLines.removeFirst();
+
+        if (lineSplit.isEmpty())
             continue;
-        }

         // BEGIN pre-parse to know where the message belongs to
         QString command = lineSplit[0].toLower();
@@ -1200,8 +1198,10 @@ void Server::incoming()
         #endif
         bool isUtf8 = Konversation::isUtf8(first);

+        QString encoded;
+
         if (isUtf8)
-            m_inputBuffer << QString::fromUtf8(first);
+            encoded = QString::fromUtf8(first);
         else
         {
             // check setting
@@ -1223,15 +1223,16 @@ void Server::incoming()
             if (codec->mibEnum() == 106)
                 codec = QTextCodec::codecForMib( 4 /* iso-8859-1 */ );

-            m_inputBuffer << codec->toUnicode(first);
+            encoded = codec->toUnicode(first);
         }

-        bufferLines.removeFirst();
-
         // Qt uses 0xFDD0 and 0xFDD1 to mark the beginning and end of text
frames. Remove
         // these here to avoid fatal errors encountered in QText* and the
event loop pro-
         // cessing.
-        sterilizeUnicode(m_inputBuffer.back());
+        sterilizeUnicode(encoded);
+
+        if (!encoded.isEmpty())
+            m_inputBuffer << encoded;

         //FIXME: This has nothing to do with bytes, and it's not raw received
bytes either. Bogus number.
         //m_bytesReceived+=m_inputBuffer.back().length();

-- 
Configure bugmail: https://bugs.kde.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.