There is use after free crash when client using zrle disconnects:
ZRLEEncoder contains zos variable (rdr::ZlibOutStream) and mos variable 
(pointer to rdr::MemOutStream).
mos is always allocated in constructor (it could be a copy of static sharedMos 
pointer if sharedMos != 0, but it is always 0). 
When ZRLEEncoder::writeRect is called, any of zrleEncode* functions sets mos as 
an underlying stream of zos.
When ZRLEEncoder is destructed, mos is deleted (sharedMos is always 0), then 
zos is implicitly destructed, but zos accesses it's underlying stream in it's 
destructor!

We need to destruct mos first and zos second when ZRLEEncoder is destructed.
As sharedMos is never used, we can remove that, simplify ZRLEEncoder and turn 
zos into a member variable same as mos. They will be both implicitly destructed 
in reverse order of declaration.

diff -ur tigervnc-1.3.0-orig/common/rfb/ZRLEEncoder.cxx 
tigervnc-1.3.0/common/rfb/ZRLEEncoder.cxx
--- tigervnc-1.3.0-orig/common/rfb/ZRLEEncoder.cxx      2013-09-17 
00:18:28.557911306 +0300
+++ tigervnc-1.3.0/common/rfb/ZRLEEncoder.cxx   2013-09-17 00:19:57.487915741 
+0300
@@ -26,7 +26,6 @@
 
 using namespace rfb;
 
-rdr::MemOutStream* ZRLEEncoder::sharedMos = 0;
 int ZRLEEncoder::maxLen = 4097 * 1024; // enough for width 16384 32-bit pixels
 
 IntParameter zlibLevel("ZlibLevel","Zlib compression level",-1);
@@ -55,33 +54,27 @@
 }
 
 ZRLEEncoder::ZRLEEncoder(SMsgWriter* writer_)
-  : writer(writer_), zos(0,0,zlibLevel)
+  : writer(writer_), zos(0,0,zlibLevel), mos(129*1024)
 {
-  if (sharedMos)
-    mos = sharedMos;
-  else
-    mos = new rdr::MemOutStream(129*1024);
 }
 
 ZRLEEncoder::~ZRLEEncoder()
 {
-  if (!sharedMos)
-    delete mos;
 }
 
 bool ZRLEEncoder::writeRect(const Rect& r, TransImageGetter* ig, Rect* actual)
 {
   rdr::U8* imageBuf = writer->getImageBuf(64 * 64 * 4 + 4);
-  mos->clear();
+  mos.clear();
   bool wroteAll = true;
   *actual = r;
 
   switch (writer->bpp()) {
   case 8:
-    wroteAll = zrleEncode8(r, mos, &zos, imageBuf, maxLen, actual, ig);
+    wroteAll = zrleEncode8(r, &mos, &zos, imageBuf, maxLen, actual, ig);
     break;
   case 16:
-    wroteAll = zrleEncode16(r, mos, &zos, imageBuf, maxLen, actual, ig);
+    wroteAll = zrleEncode16(r, &mos, &zos, imageBuf, maxLen, actual, ig);
     break;
   case 32:
     {
@@ -94,16 +87,16 @@
       if ((fitsInLS3Bytes && pf.isLittleEndian()) ||
           (fitsInMS3Bytes && pf.isBigEndian()))
       {
-        wroteAll = zrleEncode24A(r, mos, &zos, imageBuf, maxLen, actual, ig);
+        wroteAll = zrleEncode24A(r, &mos, &zos, imageBuf, maxLen, actual, ig);
       }
       else if ((fitsInLS3Bytes && pf.isBigEndian()) ||
                (fitsInMS3Bytes && pf.isLittleEndian()))
       {
-        wroteAll = zrleEncode24B(r, mos, &zos, imageBuf, maxLen, actual, ig);
+        wroteAll = zrleEncode24B(r, &mos, &zos, imageBuf, maxLen, actual, ig);
       }
       else
       {
-        wroteAll = zrleEncode32(r, mos, &zos, imageBuf, maxLen, actual, ig);
+        wroteAll = zrleEncode32(r, &mos, &zos, imageBuf, maxLen, actual, ig);
       }
       break;
     }
@@ -111,8 +104,8 @@
 
   writer->startRect(*actual, encodingZRLE);
   rdr::OutStream* os = writer->getOutStream();
-  os->writeU32(mos->length());
-  os->writeBytes(mos->data(), mos->length());
+  os->writeU32(mos.length());
+  os->writeBytes(mos.data(), mos.length());
   writer->endRect();
   return wroteAll;
 }
diff -ur tigervnc-1.3.0-orig/common/rfb/ZRLEEncoder.h 
tigervnc-1.3.0/common/rfb/ZRLEEncoder.h
--- tigervnc-1.3.0-orig/common/rfb/ZRLEEncoder.h        2013-09-17 
00:18:28.558911306 +0300
+++ tigervnc-1.3.0/common/rfb/ZRLEEncoder.h     2013-09-17 00:20:34.372917581 
+0300
@@ -38,16 +38,11 @@
     // width, in this example about 128 bytes).
     static void setMaxLen(int m) { maxLen = m; }
 
-    // setSharedMos() sets a MemOutStream to be shared amongst all
-    // ZRLEEncoders.  Should be called before any ZRLEEncoders are created.
-    static void setSharedMos(rdr::MemOutStream* mos_) { sharedMos = mos_; }
-
   private:
     ZRLEEncoder(SMsgWriter* writer);
     SMsgWriter* writer;
+    rdr::MemOutStream mos;
     rdr::ZlibOutStream zos;
-    rdr::MemOutStream* mos;
-    static rdr::MemOutStream* sharedMos;
     static int maxLen;
   };
 }


------------------------------------------------------------------------------
LIMITED TIME SALE - Full Year of Microsoft Training For Just $49.99!
1,500+ hours of tutorials including VisualStudio 2012, Windows 8, SharePoint
2013, SQL 2012, MVC 4, more. BEST VALUE: New Multi-Library Power Pack includes
Mobile, Cloud, Java, and UX Design. Lowest price ever! Ends 9/20/13. 
http://pubads.g.doubleclick.net/gampad/clk?id=58041151&iu=/4140/ostg.clktrk
_______________________________________________
Tigervnc-devel mailing list
Tigervnc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tigervnc-devel

Reply via email to