Bug#334089: remotely segfaultable, DOS

[Steve Langasek]

tags 334089 patch
thanks

Hello,

I've tracked this bug in centericq down to a failure to deal with short
packets (or packets declaring their own length to be zero).  The attached
patch fixes this segfault, by stopping without further processing of the
packet when its length is determined to be zero.

Someone should also check what happens when the parser reads a packet length
value of 1 or 2; there may be other bugs handling those cases as well.

I don't see any obvious way that this bug could be exploited to gain remote
access, but unfortunately there may be a non-obvious way...  I've cc:ed the
security team, so they can evaluate whether this warrants a security upload
-- perhaps the DoS alone is enough reason for an update.

Also, I've attached a second patch, unrelated to any known crasher bugs,
that includes some fixes for memory handling which turned up when trying to
valgrind centericq.  I don't suspect that it's relevant to a stable security
update, but the maintainer may want to consider including it in his next
upload to unstable.

Thanks,
-- 
Steve Langasek                   Give me a lever long enough and a Free OS
Debian Developer                   to set it on, and I can move the world.
[EMAIL PROTECTED]                                   http://www.debian.org/

diff -u centericq-4.21.0/debian/changelog centericq-4.21.0/debian/changelog
--- centericq-4.21.0/debian/changelog
+++ centericq-4.21.0/debian/changelog
@@ -1,3 +1,11 @@
+centericq (4.21.0-3.1) unstable; urgency=low
+
+  * Non-maintainer upload.
+  * Fix for ICQ direct client handler, which fails to handle undersized
+    requests from remote hosts, leading to a segfault (closes: #334089).
+
+ -- Steve Langasek <[EMAIL PROTECTED]>  Sat, 19 Nov 2005 05:16:12 -0800
+
 centericq (4.21.0-3) unstable; urgency=low
 
   * Applied patch from Guillaume Libersat <[EMAIL PROTECTED]>
only in patch2:
unchanged:
--- centericq-4.21.0.orig/libicq2000-0.1/src/DirectClient.cpp
+++ centericq-4.21.0/libicq2000-0.1/src/DirectClient.cpp
@@ -162,6 +162,7 @@
 
       m_recv.setLittleEndian();
       m_recv >> length;
+      if (length == 0) return; // short read, toss it back (nothing to do)
       if (length > Incoming_Packet_Limit) throw ParseException("Received too 
long incoming packet");
       if (m_recv.remains() < length) return; // waiting for more of the packet
 

diff -u centericq-4.21.0/debian/changelog centericq-4.21.0/debian/changelog
--- centericq-4.21.0/debian/changelog
+++ centericq-4.21.0/debian/changelog
@@ -3,6 +3,7 @@
   * Non-maintainer upload.
   * Fix for ICQ direct client handler, which fails to handle undersized
     requests from remote hosts, leading to a segfault (closes: #334089).
+  * Miscellaneous other memory handling clean-ups
 
  -- Steve Langasek <[EMAIL PROTECTED]>  Sat, 19 Nov 2005 05:16:12 -0800
 
only in patch2:
unchanged:
--- centericq-4.21.0.orig/src/icqface.cc
+++ centericq-4.21.0/src/icqface.cc
@@ -2003,7 +2003,7 @@
            mvaddchnstr(i-1, sizeWArea.x1+1, logline, 
sizeWArea.x2-sizeWArea.x1);
        }
 
-       delete logline;
+       delete[] logline;
 
        if(text.size() > sizeWArea.x2-sizeWArea.x1-2) 
text.resize(sizeWArea.x2-sizeWArea.x1-2);
        mvhline(LINES-3, sizeWArea.x1+2, ' ', sizeWArea.x2-sizeWArea.x1-2);
only in patch2:
unchanged:
--- centericq-4.21.0.orig/kkstrtext-0.1/kkstrtext.cc
+++ centericq-4.21.0/kkstrtext-0.1/kkstrtext.cc
@@ -907,8 +907,8 @@
            r += soutbuf;
            text.erase(0, text.size()-inleft);
 
-           delete soutbuf;
-           delete sinbuf;
+           delete[] soutbuf;
+           free(sinbuf);
 
            if(res == -1 && errno != EILSEQ)
                break;

signature.asc
Description: Digital signature