From 5bb09b5f95e014c8e2c684bb6a4b00115b66ebc1 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Fri, 2 Dec 2016 16:57:19 +0100 Subject: [Backport] Use safe math in ValidateCopyBufferSubData. This should fix any potential out of bounds reads/writes. BUG=chromium:660854 Change-Id: Iffa00e4551d7362115cbf023a09b1d0e15f724c8 Reviewed-on: https://chromium-review.googlesource.com/405816 Commit-Queue: Jamie Madill Reviewed-by: Corentin Wallez (CVE-2016-5221) Reviewed-by: Alexandru Croitor --- .../angle/src/libGLESv2/entry_points_gles_3_0.cpp | 44 ++++++++++++++++++++-- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/chromium/third_party/angle/src/libGLESv2/entry_points_gles_3_0.cpp b/chromium/third_party/angle/src/libGLESv2/entry_points_gles_3_0.cpp index 8ef131417a7..c1bbc98de8b 100644 --- a/chromium/third_party/angle/src/libGLESv2/entry_points_gles_3_0.cpp +++ b/chromium/third_party/angle/src/libGLESv2/entry_points_gles_3_0.cpp @@ -23,7 +23,10 @@ #include "libANGLE/validationES3.h" #include "libANGLE/queryconversions.h" +#include "base/numerics/safe_conversions.h" #include "common/debug.h" +#include "common/mathutil.h" +#include "common/utilities.h" namespace gl { @@ -1881,6 +1884,8 @@ const GLubyte *GL_APIENTRY GetStringi(GLenum name, GLuint index) void GL_APIENTRY CopyBufferSubData(GLenum readTarget, GLenum writeTarget, GLintptr readOffset, GLintptr writeOffset, GLsizeiptr size) { + using base::IsValueInRangeForNumericType; + using base::internal::CheckedNumeric; EVENT("(GLenum readTarget = 0x%X, GLenum writeTarget = 0x%X, GLintptr readOffset = %d, GLintptr writeOffset = %d, GLsizeiptr size = %d)", readTarget, writeTarget, readOffset, writeOffset, size); @@ -1915,20 +1920,51 @@ void GL_APIENTRY CopyBufferSubData(GLenum readTarget, GLenum writeTarget, GLintp return; } - if (readOffset < 0 || writeOffset < 0 || size < 0 || - static_cast(readOffset + size) > readBuffer->getSize() || - static_cast(writeOffset + size) > writeBuffer->getSize()) + CheckedNumeric checkedReadOffset(readOffset); + CheckedNumeric checkedWriteOffset(writeOffset); + CheckedNumeric checkedSize(size); + auto checkedReadSum = checkedReadOffset + checkedSize; + auto checkedWriteSum = checkedWriteOffset + checkedSize; + + if (!checkedReadSum.IsValid() || !checkedWriteSum.IsValid() || + !IsValueInRangeForNumericType(readBuffer->getSize()) || + !IsValueInRangeForNumericType(writeBuffer->getSize())) + { + context->handleError(Error(GL_INVALID_VALUE)); + return; + } + + if (readOffset < 0 || writeOffset < 0 || size < 0) { context->recordError(Error(GL_INVALID_VALUE)); return; } - if (readBuffer == writeBuffer && std::abs(readOffset - writeOffset) < size) + if (checkedReadSum.ValueOrDie() > readBuffer->getSize() || + checkedWriteSum.ValueOrDie() > writeBuffer->getSize()) { context->recordError(Error(GL_INVALID_VALUE)); return; } + if (readBuffer == writeBuffer) + { + auto checkedOffsetDiff = (checkedReadOffset - checkedWriteOffset).Abs(); + if (!checkedOffsetDiff.IsValid()) + { + // This shold not be possible. + UNREACHABLE(); + context->handleError(Error(GL_INVALID_VALUE)); + return; + } + + if (checkedOffsetDiff.ValueOrDie() < size) + { + context->handleError(Error(GL_INVALID_VALUE)); + return; + } + } + // if size is zero, the copy is a successful no-op if (size > 0) { -- cgit v1.2.3