|
|
12537a |
From d61a767d6842b530ffb532ddd5a3d233119aad40 Mon Sep 17 00:00:00 2001
|
|
|
12537a |
From: Pierre Ossman <ossman@cendio.se>
|
|
|
12537a |
Date: Tue, 10 Sep 2019 11:05:48 +0200
|
|
|
12537a |
Subject: [PATCH] Make ZlibInStream more robust against failures
|
|
|
12537a |
|
|
|
12537a |
Move the checks around to avoid missing cases where we might access
|
|
|
12537a |
memory that is no longer valid. Also avoid touching the underlying
|
|
|
12537a |
stream implicitly (e.g. via the destructor) as it might also no
|
|
|
12537a |
longer be valid.
|
|
|
12537a |
|
|
|
12537a |
A malicious server could theoretically use this for remote code
|
|
|
12537a |
execution in the client.
|
|
|
12537a |
|
|
|
12537a |
Issue found by Pavel Cheremushkin from Kaspersky Lab
|
|
|
12537a |
---
|
|
|
12537a |
common/rdr/ZlibInStream.cxx | 13 +++++++------
|
|
|
12537a |
common/rdr/ZlibInStream.h | 2 +-
|
|
|
12537a |
common/rfb/CMsgReader.cxx | 3 ++-
|
|
|
12537a |
common/rfb/SMsgReader.cxx | 3 ++-
|
|
|
12537a |
common/rfb/TightDecoder.cxx | 3 ++-
|
|
|
12537a |
common/rfb/zrleDecode.h | 3 ++-
|
|
|
12537a |
6 files changed, 16 insertions(+), 11 deletions(-)
|
|
|
12537a |
|
|
|
12537a |
diff --git a/common/rdr/ZlibInStream.cxx b/common/rdr/ZlibInStream.cxx
|
|
|
12537a |
index 4053bd1..a361010 100644
|
|
|
12537a |
--- a/common/rdr/ZlibInStream.cxx
|
|
|
12537a |
+++ b/common/rdr/ZlibInStream.cxx
|
|
|
12537a |
@@ -52,16 +52,16 @@ int ZlibInStream::pos()
|
|
|
12537a |
return offset + ptr - start;
|
|
|
12537a |
}
|
|
|
12537a |
|
|
|
12537a |
-void ZlibInStream::removeUnderlying()
|
|
|
12537a |
+void ZlibInStream::flushUnderlying()
|
|
|
12537a |
{
|
|
|
12537a |
ptr = end = start;
|
|
|
12537a |
- if (!underlying) return;
|
|
|
12537a |
|
|
|
12537a |
while (bytesIn > 0) {
|
|
|
12537a |
decompress(true);
|
|
|
12537a |
end = start; // throw away any data
|
|
|
12537a |
}
|
|
|
12537a |
- underlying = 0;
|
|
|
12537a |
+
|
|
|
12537a |
+ setUnderlying(NULL, 0);
|
|
|
12537a |
}
|
|
|
12537a |
|
|
|
12537a |
void ZlibInStream::reset()
|
|
|
12537a |
@@ -90,7 +90,7 @@ void ZlibInStream::init()
|
|
|
12537a |
void ZlibInStream::deinit()
|
|
|
12537a |
{
|
|
|
12537a |
assert(zs != NULL);
|
|
|
12537a |
- removeUnderlying();
|
|
|
12537a |
+ setUnderlying(NULL, 0);
|
|
|
12537a |
inflateEnd(zs);
|
|
|
12537a |
delete zs;
|
|
|
12537a |
zs = NULL;
|
|
|
12537a |
@@ -100,8 +100,6 @@ int ZlibInStream::overrun(int itemSize, int nItems, bool wait)
|
|
|
12537a |
{
|
|
|
12537a |
if (itemSize > bufSize)
|
|
|
12537a |
throw Exception("ZlibInStream overrun: max itemSize exceeded");
|
|
|
12537a |
- if (!underlying)
|
|
|
12537a |
- throw Exception("ZlibInStream overrun: no underlying stream");
|
|
|
12537a |
|
|
|
12537a |
if (end - ptr != 0)
|
|
|
12537a |
memmove(start, ptr, end - ptr);
|
|
|
12537a |
@@ -127,6 +125,9 @@ int ZlibInStream::overrun(int itemSize, int nItems, bool wait)
|
|
|
12537a |
|
|
|
12537a |
bool ZlibInStream::decompress(bool wait)
|
|
|
12537a |
{
|
|
|
12537a |
+ if (!underlying)
|
|
|
12537a |
+ throw Exception("ZlibInStream overrun: no underlying stream");
|
|
|
12537a |
+
|
|
|
12537a |
zs->next_out = (U8*)end;
|
|
|
12537a |
zs->avail_out = start + bufSize - end;
|
|
|
12537a |
|
|
|
12537a |
diff --git a/common/rdr/ZlibInStream.h b/common/rdr/ZlibInStream.h
|
|
|
12537a |
index 6bd4da4..86ba1ff 100644
|
|
|
12537a |
--- a/common/rdr/ZlibInStream.h
|
|
|
12537a |
+++ b/common/rdr/ZlibInStream.h
|
|
|
12537a |
@@ -38,7 +38,7 @@ namespace rdr {
|
|
|
12537a |
virtual ~ZlibInStream();
|
|
|
12537a |
|
|
|
12537a |
void setUnderlying(InStream* is, int bytesIn);
|
|
|
12537a |
- void removeUnderlying();
|
|
|
12537a |
+ void flushUnderlying();
|
|
|
12537a |
int pos();
|
|
|
12537a |
void reset();
|
|
|
12537a |
|
|
|
12537a |
diff --git a/common/rfb/TightDecoder.cxx b/common/rfb/TightDecoder.cxx
|
|
|
12537a |
index 3a1254a..4273eb7 100644
|
|
|
12537a |
--- a/common/rfb/TightDecoder.cxx
|
|
|
12537a |
+++ b/common/rfb/TightDecoder.cxx
|
|
|
12537a |
@@ -340,7 +340,8 @@ void TightDecoder::decodeRect(const Rect& r, const void* buffer,
|
|
|
12537a |
|
|
|
12537a |
zis[streamId].readBytes(netbuf, dataSize);
|
|
|
12537a |
|
|
|
12537a |
- zis[streamId].removeUnderlying();
|
|
|
12537a |
+ zis[streamId].flushUnderlying();
|
|
|
12537a |
+ zis[streamId].setUnderlying(NULL, 0);
|
|
|
12537a |
delete ms;
|
|
|
12537a |
|
|
|
12537a |
bufptr = netbuf;
|
|
|
12537a |
diff --git a/common/rfb/zrleDecode.h b/common/rfb/zrleDecode.h
|
|
|
12537a |
index 0bfbbe1..a69ca67 100644
|
|
|
12537a |
--- a/common/rfb/zrleDecode.h
|
|
|
12537a |
+++ b/common/rfb/zrleDecode.h
|
|
|
12537a |
@@ -178,7 +178,8 @@ void ZRLE_DECODE (const Rect& r, rdr::InStream* is,
|
|
|
12537a |
}
|
|
|
12537a |
}
|
|
|
12537a |
|
|
|
12537a |
- zis->removeUnderlying();
|
|
|
12537a |
+ zis->flushUnderlying();
|
|
|
12537a |
+ zis->setUnderlying(NULL, 0);
|
|
|
12537a |
}
|
|
|
12537a |
|
|
|
12537a |
#undef ZRLE_DECODE
|