Validate that unpack skip is in bounds

src/libANGLE/formatutils.cpp

@@ -763,15 +763,28 @@ gl::ErrorOrResult<GLuint> InternalFormat::computeCompressedImageSize(GLenum form
     return bytes.ValueOrDie();
 }
 
-GLuint InternalFormat::computeSkipPixels(GLint rowPitch,
-                                         GLint depthPitch,
-                                         GLint skipImages,
-                                         GLint skipRows,
-                                         GLint skipPixels,
-                                         bool applySkipImages) const
+gl::ErrorOrResult<GLuint> InternalFormat::computeSkipBytes(GLuint rowPitch,
+                                                           GLuint depthPitch,
+                                                           GLint skipImages,
+                                                           GLint skipRows,
+                                                           GLint skipPixels,
+                                                           bool applySkipImages) const
 {
-    GLuint skipImagesBytes = applySkipImages ? skipImages * depthPitch : 0;
-    return skipImagesBytes + skipRows * rowPitch + skipPixels * pixelBytes;
+    CheckedNumeric<GLuint> checkedRowPitch(rowPitch);
+    CheckedNumeric<GLuint> checkedDepthPitch(depthPitch);
+    CheckedNumeric<GLuint> checkedSkipImages(static_cast<GLuint>(skipImages));
+    CheckedNumeric<GLuint> checkedSkipRows(static_cast<GLuint>(skipRows));
+    CheckedNumeric<GLuint> checkedSkipPixels(static_cast<GLuint>(skipPixels));
+    CheckedNumeric<GLuint> checkedPixelBytes(pixelBytes);
+    auto checkedSkipImagesBytes = checkedSkipImages * checkedDepthPitch;
+    if (!applySkipImages)
+    {
+        checkedSkipImagesBytes = 0;
+    }
+    auto skipBytes = checkedSkipImagesBytes + checkedSkipRows * checkedRowPitch +
+                     checkedSkipPixels * checkedPixelBytes;
+    ANGLE_TRY_CHECKED_MATH(skipBytes);
+    return skipBytes.ValueOrDie();
 }
 
 gl::ErrorOrResult<GLuint> InternalFormat::computeUnpackSize(

src/libANGLE/formatutils.h

@@ -59,12 +59,12 @@ struct InternalFormat
                                                 GLint imageHeight) const;
     gl::ErrorOrResult<GLuint> computeCompressedImageSize(GLenum formatType,
                                                          const gl::Extents &size) const;
-    GLuint computeSkipPixels(GLint rowPitch,
-                             GLint depthPitch,
-                             GLint skipImages,
-                             GLint skipRows,
-                             GLint skipPixels,
-                             bool applySkipImages) const;
+    gl::ErrorOrResult<GLuint> computeSkipBytes(GLuint rowPitch,
+                                               GLuint depthPitch,
+                                               GLint skipImages,
+                                               GLint skipRows,
+                                               GLint skipPixels,
+                                               bool applySkipImages) const;
     gl::ErrorOrResult<GLuint> computeUnpackSize(GLenum formatType,
                                                 const gl::Extents &size,
                                                 const gl::PixelUnpackState &unpack) const;

src/libANGLE/renderer/d3d/FramebufferD3D.cpp

@@ -246,8 +246,10 @@ gl::Error FramebufferD3D::readPixels(ContextImpl *context,
     ANGLE_TRY_RESULT(
         sizedFormatInfo.computeRowPitch(type, area.width, packState.alignment, packState.rowLength),
         outputPitch);
-    GLsizei outputSkipBytes = sizedFormatInfo.computeSkipPixels(
-        outputPitch, 0, 0, packState.skipRows, packState.skipPixels, false);
+    GLuint outputSkipBytes = 0;
+    ANGLE_TRY_RESULT(sizedFormatInfo.computeSkipBytes(outputPitch, 0, 0, packState.skipRows,
+                                                      packState.skipPixels, false),
+                     outputSkipBytes);
 
     return readPixelsImpl(area, format, type, outputPitch, packState,
                           reinterpret_cast<uint8_t *>(pixels) + outputSkipBytes);

src/libANGLE/renderer/d3d/d3d11/Image11.cpp

@@ -252,17 +252,19 @@ gl::Error Image11::loadData(const gl::Box &area,
                             bool applySkipImages)
 {
     const gl::InternalFormat &formatInfo = gl::GetInternalFormatInfo(mInternalFormat);
-    GLsizei inputRowPitch                = 0;
+    GLuint inputRowPitch                 = 0;
     ANGLE_TRY_RESULT(
         formatInfo.computeRowPitch(type, area.width, unpack.alignment, unpack.rowLength),
         inputRowPitch);
-    GLsizei inputDepthPitch = 0;
+    GLuint inputDepthPitch = 0;
     ANGLE_TRY_RESULT(formatInfo.computeDepthPitch(type, area.width, area.height, unpack.alignment,
                                                   unpack.rowLength, unpack.imageHeight),
                      inputDepthPitch);
-    GLsizei inputSkipBytes =
-        formatInfo.computeSkipPixels(inputRowPitch, inputDepthPitch, unpack.skipImages,
-                                     unpack.skipRows, unpack.skipPixels, applySkipImages);
+    GLuint inputSkipBytes = 0;
+    ANGLE_TRY_RESULT(
+        formatInfo.computeSkipBytes(inputRowPitch, inputDepthPitch, unpack.skipImages,
+                                    unpack.skipRows, unpack.skipPixels, applySkipImages),
+        inputSkipBytes);
 
     const d3d11::DXGIFormatSize &dxgiFormatInfo = d3d11::GetDXGIFormatSizeInfo(mDXGIFormat);
     GLuint outputPixelSize = dxgiFormatInfo.pixelBytes;

src/libANGLE/renderer/d3d/d3d11/TextureStorage11.cpp

@@ -647,17 +647,19 @@ gl::Error TextureStorage11::setData(const gl::ImageIndex &index,
     const int width  = destBox ? destBox->width : static_cast<int>(image->getWidth());
     const int height = destBox ? destBox->height : static_cast<int>(image->getHeight());
     const int depth = destBox ? destBox->depth : static_cast<int>(image->getDepth());
-    UINT srcRowPitch = 0;
+    GLuint srcRowPitch = 0;
     ANGLE_TRY_RESULT(
         internalFormatInfo.computeRowPitch(type, width, unpack.alignment, unpack.rowLength),
         srcRowPitch);
-    UINT srcDepthPitch = 0;
+    GLuint srcDepthPitch = 0;
     ANGLE_TRY_RESULT(internalFormatInfo.computeDepthPitch(type, width, height, unpack.alignment,
                                                           unpack.rowLength, unpack.imageHeight),
                      srcDepthPitch);
-    const GLsizei srcSkipBytes =
-        internalFormatInfo.computeSkipPixels(srcRowPitch, srcDepthPitch, unpack.skipImages,
-                                             unpack.skipRows, unpack.skipPixels, index.is3D());
+    GLuint srcSkipBytes = 0;
+    ANGLE_TRY_RESULT(
+        internalFormatInfo.computeSkipBytes(srcRowPitch, srcDepthPitch, unpack.skipImages,
+                                            unpack.skipRows, unpack.skipPixels, index.is3D()),
+        srcSkipBytes);
 
     const d3d11::TextureFormat &d3d11Format = d3d11::GetTextureFormatInfo(
         image->getInternalFormat(), mRenderer->getRenderer11DeviceCaps());

src/libANGLE/renderer/d3d/d3d9/Image9.cpp

@@ -483,13 +483,13 @@ gl::Error Image9::loadData(const gl::Box &area,
     ASSERT(area.z == 0 && area.depth == 1);
 
     const gl::InternalFormat &formatInfo = gl::GetInternalFormatInfo(mInternalFormat);
-    GLsizei inputRowPitch                = 0;
+    GLuint inputRowPitch                 = 0;
     ANGLE_TRY_RESULT(
         formatInfo.computeRowPitch(type, area.width, unpack.alignment, unpack.rowLength),
         inputRowPitch);
     ASSERT(!applySkipImages);
-    GLsizei inputSkipBytes = formatInfo.computeSkipPixels(
-        inputRowPitch, 0, unpack.skipImages, unpack.skipRows, unpack.skipPixels, false);
+    ASSERT(unpack.skipPixels == 0);
+    ASSERT(unpack.skipRows == 0);
 
     const d3d9::TextureFormat &d3dFormatInfo = d3d9::GetTextureFormatInfo(mInternalFormat);
     ASSERT(d3dFormatInfo.loadFunction != NULL);
@@ -508,9 +508,8 @@ gl::Error Image9::loadData(const gl::Box &area,
     }
 
     d3dFormatInfo.loadFunction(area.width, area.height, area.depth,
-                               reinterpret_cast<const uint8_t *>(input) + inputSkipBytes,
-                               inputRowPitch, 0, reinterpret_cast<uint8_t *>(locked.pBits),
-                               locked.Pitch, 0);
+                               reinterpret_cast<const uint8_t *>(input), inputRowPitch, 0,
+                               reinterpret_cast<uint8_t *>(locked.pBits), locked.Pitch, 0);
 
     unlock();
 

src/libANGLE/validationES3.cpp

@@ -487,6 +487,32 @@ bool ValidateES3TexImageParametersBase(Context *context,
         CheckedNumeric<size_t> checkedOffset(reinterpret_cast<size_t>(pixels));
         checkedCopyBytes += checkedOffset;
 
+        auto rowPitchOrErr =
+            formatInfo.computeRowPitch(type, width, unpack.alignment, unpack.rowLength);
+        if (rowPitchOrErr.isError())
+        {
+            context->handleError(rowPitchOrErr.getError());
+            return false;
+        }
+        auto depthPitchOrErr = formatInfo.computeDepthPitch(type, width, height, unpack.alignment,
+                                                            unpack.rowLength, unpack.imageHeight);
+        if (depthPitchOrErr.isError())
+        {
+            context->handleError(depthPitchOrErr.getError());
+            return false;
+        }
+
+        bool targetIs3D     = target == GL_TEXTURE_3D || target == GL_TEXTURE_2D_ARRAY;
+        auto skipBytesOrErr = formatInfo.computeSkipBytes(
+            rowPitchOrErr.getResult(), depthPitchOrErr.getResult(), unpack.skipImages,
+            unpack.skipRows, unpack.skipPixels, targetIs3D);
+        if (skipBytesOrErr.isError())
+        {
+            context->handleError(skipBytesOrErr.getError());
+            return false;
+        }
+        checkedCopyBytes += skipBytesOrErr.getResult();
+
         if (!checkedCopyBytes.IsValid() ||
             (checkedCopyBytes.ValueOrDie() > static_cast<size_t>(pixelUnpackBuffer->getSize())))
         {

src/tests/gl_tests/TextureTest.cpp

@@ -5,6 +5,7 @@
 //
 
 #include "test_utils/ANGLETest.h"
+#include "test_utils/gl_raii.h"
 
 using namespace angle;
 
@@ -3354,6 +3355,39 @@ TEST_P(Texture2DTestES3, UnpackSkipImages2D)
     EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
 }
 
+// Test that skip defined in unpack parameters is taken into account when determining whether
+// unpacking source extends outside unpack buffer bounds.
+TEST_P(Texture2DTestES3, UnpackSkipPixelsOutOfBounds)
+{
+    glBindTexture(GL_TEXTURE_2D, mTexture2D);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
+    ASSERT_GL_NO_ERROR();
+
+    GLBuffer buf;
+    glBindBuffer(GL_PIXEL_UNPACK_BUFFER, buf.get());
+    std::vector<GLColor> pixelsGreen(128u * 128u, GLColor::green);
+    glBufferData(GL_PIXEL_UNPACK_BUFFER, pixelsGreen.size() * 4u, pixelsGreen.data(),
+                 GL_DYNAMIC_COPY);
+    ASSERT_GL_NO_ERROR();
+
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 128, 128, 0, GL_RGBA, GL_UNSIGNED_BYTE, 0);
+    ASSERT_GL_NO_ERROR();
+
+    glPixelStorei(GL_UNPACK_SKIP_PIXELS, 1);
+    ASSERT_GL_NO_ERROR();
+
+    glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 128, 128, GL_RGBA, GL_UNSIGNED_BYTE, 0);
+    EXPECT_GL_ERROR(GL_INVALID_OPERATION);
+
+    glPixelStorei(GL_UNPACK_SKIP_PIXELS, 0);
+    glPixelStorei(GL_UNPACK_SKIP_ROWS, 1);
+    ASSERT_GL_NO_ERROR();
+
+    glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 128, 128, GL_RGBA, GL_UNSIGNED_BYTE, 0);
+    EXPECT_GL_ERROR(GL_INVALID_OPERATION);
+}
+
 // Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against.
 // TODO(oetuaho): Enable all below tests on OpenGL. Requires a fix for ANGLE bug 1278.
 ANGLE_INSTANTIATE_TEST(Texture2DTest,