aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPierre Ossman <ossman@cendio.se>2019-09-10 16:07:50 +0200
committerPierre Ossman <ossman@cendio.se>2019-12-20 07:29:00 +0100
commitdd4ccd13ff5ecf64c913f09558733e593556e1da (patch)
tree6bb7c09b97e48e246bd1459a3d970fb7b3dcd648
parent6d71fe854b7b518c59f59c18d94edfd839043650 (diff)
downloadtigervnc-dd4ccd13ff5ecf64c913f09558733e593556e1da.tar.gz
tigervnc-dd4ccd13ff5ecf64c913f09558733e593556e1da.zip
Add sanity checks for PixelFormat shift values
Otherwise we might be tricked in to reading and writing things at incorrect offsets for pixels which ultimately could result in an attacker writing things to the stack or heap and executing things they shouldn't. This only affects the server as the client never uses the pixel format suggested by th server. Issue found by Pavel Cheremushkin from Kaspersky Lab. (cherry picked from commit cd1d650c532a46e95a1229dffaf281c76a50cdfe)
-rw-r--r--common/rfb/PixelFormat.cxx7
-rw-r--r--tests/unit/pixelformat.cxx6
2 files changed, 13 insertions, 0 deletions
diff --git a/common/rfb/PixelFormat.cxx b/common/rfb/PixelFormat.cxx
index 2d8142d1..789c43ed 100644
--- a/common/rfb/PixelFormat.cxx
+++ b/common/rfb/PixelFormat.cxx
@@ -682,6 +682,13 @@ bool PixelFormat::isSane(void)
if (totalBits > depth)
return false;
+ if ((bits(redMax) + redShift) > bpp)
+ return false;
+ if ((bits(greenMax) + greenShift) > bpp)
+ return false;
+ if ((bits(blueMax) + blueShift) > bpp)
+ return false;
+
if (((redMax << redShift) & (greenMax << greenShift)) != 0)
return false;
if (((redMax << redShift) & (blueMax << blueShift)) != 0)
diff --git a/tests/unit/pixelformat.cxx b/tests/unit/pixelformat.cxx
index 7b6087f7..46fecfb4 100644
--- a/tests/unit/pixelformat.cxx
+++ b/tests/unit/pixelformat.cxx
@@ -108,6 +108,12 @@ int main(int argc, char** argv)
doTest(true, 32, 16, false, true, 255, 255, 255, 0, 8, 16);
+ /* Invalid shift values */
+
+ doTest(true, 32, 24, false, true, 255, 255, 255, 25, 8, 16);
+ doTest(true, 32, 24, false, true, 255, 255, 255, 0, 25, 16);
+ doTest(true, 32, 24, false, true, 255, 255, 255, 0, 8, 25);
+
/* Overlapping channels */
doTest(true, 32, 24, false, true, 255, 255, 255, 0, 7, 16);