Changeset 187189 in webkit

Timestamp:

Jul 22, 2015, 2:55:48 PM (11 years ago)

Author:

dino@apple.com

Message:

Out of bounds in WebGLRenderingContext::simulateVertexAttrib0
​https://bugs.webkit.org/show_bug.cgi?id=147176
<rdar://problem/21567767>

Reviewed by Oliver Hunt.

Source/WebCore:

Test: fast/canvas/webgl/out-of-bounds-simulated-vertexAttrib0-drawArrays.html

Add overflow checking for the drawing calls, specifically the way
they may simulate vertexAttrib0.

• html/canvas/WebGLRenderingContextBase.cpp:

(WebCore::WebGLRenderingContextBase::validateDrawArrays): Call new validation method.
(WebCore::WebGLRenderingContextBase::validateDrawElements): Ditto.
(WebCore::WebGLRenderingContextBase::validateSimulatedVertexAttrib0): New method that
validates the parameters used to create the simulated attribute.
(WebCore::WebGLRenderingContextBase::simulateVertexAttrib0): No need to do overflow
checking here now that the validation method does it for us.
(WebCore::WebGLRenderingContextBase::validateVertexAttributes): Deleted.

• html/canvas/WebGLRenderingContextBase.h: Add new validation method.

LayoutTests:

• fast/canvas/webgl/out-of-bounds-simulated-vertexAttrib0-drawArrays-expected.txt: Added.
• fast/canvas/webgl/out-of-bounds-simulated-vertexAttrib0-drawArrays.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/canvas/webgl/out-of-bounds-simulated-vertexAttrib0-drawArrays-expected.txt (added)
• LayoutTests/fast/canvas/webgl/out-of-bounds-simulated-vertexAttrib0-drawArrays.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp (modified) (8 diffs)
• Source/WebCore/html/canvas/WebGLRenderingContextBase.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2015-07-22  Dean Jackson  <dino@apple.com>
+
+        Out of bounds in WebGLRenderingContext::simulateVertexAttrib0
+        https://bugs.webkit.org/show_bug.cgi?id=147176
+        <rdar://problem/21567767>
+
+        Reviewed by Oliver Hunt.
+
+        * fast/canvas/webgl/out-of-bounds-simulated-vertexAttrib0-drawArrays-expected.txt: Added.
+        * fast/canvas/webgl/out-of-bounds-simulated-vertexAttrib0-drawArrays.html: Added.
+
 2015-07-22  Joseph Pecoraro  <pecoraro@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-07-22  Dean Jackson  <dino@apple.com>
+
+        Out of bounds in WebGLRenderingContext::simulateVertexAttrib0
+        https://bugs.webkit.org/show_bug.cgi?id=147176
+        <rdar://problem/21567767>
+
+        Reviewed by Oliver Hunt.
+
+        Test: fast/canvas/webgl/out-of-bounds-simulated-vertexAttrib0-drawArrays.html
+
+        Add overflow checking for the drawing calls, specifically the way
+        they may simulate vertexAttrib0.
+
+        * html/canvas/WebGLRenderingContextBase.cpp:
+        (WebCore::WebGLRenderingContextBase::validateDrawArrays): Call new validation method.
+        (WebCore::WebGLRenderingContextBase::validateDrawElements): Ditto.
+        (WebCore::WebGLRenderingContextBase::validateSimulatedVertexAttrib0): New method that
+        validates the parameters used to create the simulated attribute.
+        (WebCore::WebGLRenderingContextBase::simulateVertexAttrib0): No need to do overflow
+        checking here now that the validation method does it for us.
+        (WebCore::WebGLRenderingContextBase::validateVertexAttributes): Deleted.
+        * html/canvas/WebGLRenderingContextBase.h: Add new validation method.
+
 2015-07-22  Joseph Pecoraro  <pecoraro@apple.com>
 

trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp

@@ -1709 +1709 @@
         return true;
 
-
     // Look in each consumed vertex attrib (by the current program).
     bool sawNonInstancedAttrib = false;
@@ -1794 +1793 @@
             return false;
         }
+        if (!validateSimulatedVertexAttrib0(checkedSum.unsafeGet() - 1)) {
+            synthesizeGLError(GraphicsContext3D::INVALID_OPERATION, functionName, "attempt to access outside the bounds of the simulated vertexAttrib0 array");
+            return false;
+        }
     } else {
         if (!validateVertexAttributes(0)) {
@@ -1874 +1877 @@
             }
         }
+
+        if (!validateSimulatedVertexAttrib0(numElements)) {
+            synthesizeGLError(GraphicsContext3D::INVALID_OPERATION, functionName, "attempt to access outside the bounds of the simulated vertexAttrib0 array");
+            return false;
+        }
+
     } else {
         if (!validateVertexAttributes(0)) {
@@ -4670 +4679 @@
 }
 
+bool WebGLRenderingContextBase::validateSimulatedVertexAttrib0(GC3Dsizei numVertex)
+{
+    if (numVertex < 0)
+        return false;
+
+    if (!m_currentProgram)
+        return true;
+
+    bool usingVertexAttrib0 = m_currentProgram->isUsingVertexAttrib0();
+    if (!usingVertexAttrib0)
+        return true;
+
+    auto& state = m_boundVertexArrayObject->getVertexAttribState(0);
+    if (state.enabled)
+        return true;
+
+    Checked<GC3Dsizei, RecordOverflow> bufferSize(numVertex);
+    bufferSize += 1;
+    bufferSize *= Checked<GC3Dsizei>(4);
+    Checked<GC3Dsizeiptr, RecordOverflow> bufferDataSize(bufferSize);
+    bufferDataSize *= Checked<GC3Dsizeiptr>(sizeof(GC3Dfloat));
+    return !bufferDataSize.hasOverflowed();
+}
+
 bool WebGLRenderingContextBase::simulateVertexAttrib0(GC3Dsizei numVertex)
 {
-    const WebGLVertexArrayObjectBase::VertexAttribState& state = m_boundVertexArrayObject->getVertexAttribState(0);
-    const VertexAttribValue& attribValue = m_vertexAttribValue[0];
     if (!m_currentProgram)
         return false;
@@ -4679 +4710 @@
     if (usingVertexAttrib0)
         m_vertexAttrib0UsedBefore = true;
+
+    auto& state = m_boundVertexArrayObject->getVertexAttribState(0);
     if (state.enabled && usingVertexAttrib0)
         return false;
@@ -4685 +4718 @@
     m_vertexAttrib0UsedBefore = true;
     m_context->bindBuffer(GraphicsContext3D::ARRAY_BUFFER, m_vertexAttrib0Buffer->object());
-    Checked<GC3Dsizeiptr, RecordOverflow> bufferDataSize(numVertex);
-    bufferDataSize += 1;
-    bufferDataSize *= Checked<GC3Dsizeiptr, RecordOverflow>(4 * sizeof(GC3Dfloat));
-    if (bufferDataSize.hasOverflowed())
-        return false;
+
+    Checked<GC3Dsizei> bufferSize(numVertex);
+    bufferSize += 1;
+    bufferSize *= Checked<GC3Dsizei>(4);
+
+    Checked<GC3Dsizeiptr> bufferDataSize(bufferSize);
+    bufferDataSize *= Checked<GC3Dsizeiptr>(sizeof(GC3Dfloat));
+
     if (bufferDataSize.unsafeGet() > m_vertexAttrib0BufferSize) {
         m_context->bufferData(GraphicsContext3D::ARRAY_BUFFER, bufferDataSize.unsafeGet(), 0, GraphicsContext3D::DYNAMIC_DRAW);
@@ -4695 +4731 @@
         m_forceAttrib0BufferRefill = true;
     }
+
+    auto& attribValue = m_vertexAttribValue[0];
+
     if (usingVertexAttrib0
         && (m_forceAttrib0BufferRefill
@@ -4701 +4740 @@
             || attribValue.value[2] != m_vertexAttrib0BufferValue[2]
             || attribValue.value[3] != m_vertexAttrib0BufferValue[3])) {
-        auto bufferData = std::make_unique<GC3Dfloat[]>((numVertex + 1) * 4);
+
+        auto bufferData = std::make_unique<GC3Dfloat[]>(bufferSize.unsafeGet());
         for (GC3Dsizei ii = 0; ii < numVertex + 1; ++ii) {
             bufferData[ii * 4] = attribValue.value[0];

trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.h

@@ -791 +791 @@
     bool checkObjectToBeBound(const char* functionName, WebGLObject*, bool& deleted);
 
-    // Helpers for simulating vertexAttrib0
+    // Helpers for simulating vertexAttrib0.
     void initVertexAttrib0();
     bool simulateVertexAttrib0(GC3Dsizei numVertex);
+    bool validateSimulatedVertexAttrib0(GC3Dsizei numVertex);
     void restoreStatesAfterVertexAttrib0Simulation();