commit 16a2a922cfd8251a462c9ea5f90105e4d686c211
author Nicolas Capens <capn@google.com> Thu Mar 22 00:44:07 2018 -0400
committer Nicolas Capens <nicolascapens@google.com> Mon Mar 26 20:23:58 2018 +0000
tree 23186c32c5a231ba49b2e753aaf87ad77cb3ba43
parent 9869bedb56822d19c9bfbdb33f6af061da8e6106

Validate pixel unpack buffer offset.

When a pixel unpack buffer is bound, glTexImage calls interpret the
<pixels> parameter as an offset into the pixel buffer. We weren't
validating that the accessed data falls within the buffer, when taking
the offset into account.

Bug chromium:822976

Change-Id: I3ab23e3b135fd4ad1e55555eec95d584684f5d82
Reviewed-on: https://swiftshader-review.googlesource.com/17928
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>

src/OpenGL/libGLESv2/Context.cpp

diff --git a/src/OpenGL/libGLESv2/Context.cpp b/src/OpenGL/libGLESv2/Context.cpp
index f495e8a..39e8c96 100644
--- a/src/OpenGL/libGLESv2/Context.cpp
+++ b/src/OpenGL/libGLESv2/Context.cpp
@@ -1542,22 +1542,37 @@
 	return inputPitch * inputHeight * depth;
 }
 
-GLenum Context::getPixels(const GLvoid **data, GLenum type, GLsizei imageSize) const
+GLenum Context::getPixels(const GLvoid **pixels, GLenum type, GLsizei imageSize) const
 {
 	if(mState.pixelUnpackBuffer)
 	{
-		if(mState.pixelUnpackBuffer->name)
+		ASSERT(mState.pixelUnpackBuffer->name != 0);
+
+		if(mState.pixelUnpackBuffer->isMapped())
 		{
-			if(mState.pixelUnpackBuffer->isMapped() ||
-			   (mState.pixelUnpackBuffer->size() < static_cast<size_t>(imageSize)) ||
-			   (static_cast<GLsizei>((ptrdiff_t)(*data)) % GetTypeSize(type)))
-			{
-				return GL_INVALID_OPERATION;
-			}
+			return GL_INVALID_OPERATION;
 		}
 
-		*data = static_cast<const unsigned char*>(mState.pixelUnpackBuffer->data()) + (ptrdiff_t)(*data);
+		size_t offset = static_cast<size_t>((ptrdiff_t)(*pixels));
+
+		if(offset % GetTypeSize(type) != 0)
+		{
+			return GL_INVALID_OPERATION;
+		}
+
+		if(offset > mState.pixelUnpackBuffer->size())
+		{
+			return GL_INVALID_OPERATION;
+		}
+
+		if(mState.pixelUnpackBuffer->size() - offset < static_cast<size_t>(imageSize))
+		{
+			return GL_INVALID_OPERATION;
+		}
+
+		*pixels = static_cast<const unsigned char*>(mState.pixelUnpackBuffer->data()) + offset;
 	}
+
 	return GL_NO_ERROR;
 }
 

src/OpenGL/libGLESv2/utilities.cpp

diff --git a/src/OpenGL/libGLESv2/utilities.cpp b/src/OpenGL/libGLESv2/utilities.cpp
index facc550..cdd1e05 100644
--- a/src/OpenGL/libGLESv2/utilities.cpp
+++ b/src/OpenGL/libGLESv2/utilities.cpp
@@ -1121,7 +1121,7 @@
 		return GL_NO_ERROR;
 	}
 
-	GLsizei GetTypeSize(GLenum type)
+	size_t GetTypeSize(GLenum type)
 	{
 		switch(type)
 		{

src/OpenGL/libGLESv2/utilities.h

diff --git a/src/OpenGL/libGLESv2/utilities.h b/src/OpenGL/libGLESv2/utilities.h
index 5959751..f0fdb4b 100644
--- a/src/OpenGL/libGLESv2/utilities.h
+++ b/src/OpenGL/libGLESv2/utilities.h
@@ -58,7 +58,7 @@
 	int CubeFaceIndex(GLenum cubeTarget);
 	bool IsTextureTarget(GLenum target);
 	GLenum ValidateTextureFormatType(GLenum format, GLenum type, GLint internalformat, GLenum target, GLint clientVersion);
-	GLsizei GetTypeSize(GLenum type);
+	size_t GetTypeSize(GLenum type);
 
 	bool IsColorRenderable(GLint internalformat, GLint clientVersion);
 	bool IsDepthRenderable(GLint internalformat, GLint clientVersion);