Bugzilla – Attachment 558340 Details for
Bug 840433
Xvnc crashes
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Forgot Password
[patch]
Fix use after free.
u_tigervnc-1.3.0-fix-use-after-free.patch (text/plain), 4.29 KB, created by
Michal Srb
on 2013-09-16 22:15:04 UTC
(
hide
)
Description:
Fix use after free.
Filename:
MIME Type:
Creator:
Michal Srb
Created:
2013-09-16 22:15:04 UTC
Size:
4.29 KB
patch
obsolete
>Author: Michal Srb <msrb@suse.com> >Subject: Fix use after free in ZRLEEncoder. >Patch-Mainline: To be upstreamed >References: bnc#840433 > >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; > }; > }
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
Actions:
View
|
Diff
Attachments on
bug 840433
: 558340