Skip to content

Commit

Permalink
Cherry-pick 272448.803@safari-7618-branch (89ee93b). https://bugs.web…
Browse files Browse the repository at this point in the history
…kit.org/show_bug.cgi?id=271634

    GraphicsContextGLANGLE does not validate clearBuffers value length
    https://bugs.webkit.org/show_bug.cgi?id=271634
    rdar://125222153

    Reviewed by Dan Glastonbury.

    Avoid passing too long or small arrays as GL_clearBuffer*v values.

    * Source/WebCore/platform/graphics/angle/GraphicsContextGLANGLE.cpp:
    (WebCore::GraphicsContextGLANGLE::clearBufferiv):
    (WebCore::GraphicsContextGLANGLE::clearBufferuiv):
    (WebCore::GraphicsContextGLANGLE::clearBufferfv):
    (WebCore::GraphicsContextGLANGLE::validateClearBufferv):
    * Source/WebCore/platform/graphics/angle/GraphicsContextGLANGLE.h:
    * Tools/TestWebKitAPI/Tests/WebCore/cocoa/TestGraphicsContextGLCocoa.mm:
    (TestWebKitAPI::TEST_F):

    Canonical link: https://commits.webkit.org/272448.803@safari-7618-branch

Canonical link: https://commits.webkit.org/274313.241@webkitglib/2.44
  • Loading branch information
kkinnunen-apple authored and aperezdc committed May 13, 2024
1 parent b541497 commit 9b1e21b
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 4 deletions.
29 changes: 26 additions & 3 deletions Source/WebCore/platform/graphics/angle/GraphicsContextGLANGLE.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2637,7 +2637,8 @@ void GraphicsContextGLANGLE::clearBufferiv(GCGLenum buffer, GCGLint drawbuffer,
{
if (!makeContextCurrent())
return;

if (!validateClearBufferv(buffer, values.size()))
return;
GL_ClearBufferiv(buffer, drawbuffer, values.data());
checkGPUStatus();
}
Expand All @@ -2646,7 +2647,8 @@ void GraphicsContextGLANGLE::clearBufferuiv(GCGLenum buffer, GCGLint drawbuffer,
{
if (!makeContextCurrent())
return;

if (!validateClearBufferv(buffer, values.size()))
return;
GL_ClearBufferuiv(buffer, drawbuffer, values.data());
checkGPUStatus();
}
Expand All @@ -2655,7 +2657,8 @@ void GraphicsContextGLANGLE::clearBufferfv(GCGLenum buffer, GCGLint drawbuffer,
{
if (!makeContextCurrent())
return;

if (!validateClearBufferv(buffer, values.size()))
return;
GL_ClearBufferfv(buffer, drawbuffer, values.data());
checkGPUStatus();
}
Expand Down Expand Up @@ -3323,6 +3326,26 @@ void GraphicsContextGLANGLE::requestExtension(const String& name)
m_webglColorBufferFloatRGB = true;
}

bool GraphicsContextGLANGLE::validateClearBufferv(GCGLenum buffer, size_t valuesSize)
{
// ClearBuffersfv, iv, uiv are missing ANGLE Robust* entry-point. Make the call act similar way as other
// calls that validate buffer sizes by validating it here.
switch (buffer) {
case GraphicsContextGL::COLOR:
if (valuesSize == 4)
return true;
break;
case GraphicsContextGL::DEPTH:
case GraphicsContextGL::STENCIL:
if (valuesSize == 1)
return true;
break;
default:
break;
}
addError(GCGLErrorCode::InvalidOperation);
return false;
}
}

#endif // ENABLE(WEBGL)
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,7 @@ class WEBCORE_EXPORT GraphicsContextGLANGLE : public GraphicsContextGL {
// Only for non-WebGL 2.0 contexts.
GCGLenum adjustWebGL1TextureInternalFormat(GCGLenum internalformat, GCGLenum format, GCGLenum type);
void setPackParameters(GCGLint alignment, GCGLint rowLength);
bool validateClearBufferv(GCGLenum buffer, size_t valuesSize);

HashSet<String> m_availableExtensions;
HashSet<String> m_requestableExtensions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,93 @@ static bool hasMultipleGPUs()
}
#endif

TEST_F(GraphicsContextGLCocoaTest, ClearBufferIncorrectSizes)
{
using GL = WebCore::GraphicsContextGL;
WebCore::GraphicsContextGLAttributes attributes;
attributes.useMetal = true;
attributes.isWebGL2 = true;
attributes.depth = true;
attributes.stencil = true;
auto gl = TestedGraphicsContextGLCocoa::create(WTFMove(attributes));
gl->reshape(1, 1);

float floats5[5] { 0.1f, 0.2f, 0.3f, 0.4f, 0.5f };
float floats4[4] { 0.1f, 0.2f, 0.3f, 0.4f };
float floats3[3] { 0.1f, 0.2f, 0.3f };
float floats2[2] { 0.1f, 0.2f };
float floats1[1] { 0.1f };
std::span<const float> floats0;

gl->clearBufferfv(GL::COLOR, 0, floats4);
EXPECT_TRUE(gl->getErrors().isEmpty());

gl->clearBufferfv(GL::COLOR, 0, floats5);
EXPECT_TRUE(gl->getErrors().contains(GCGLErrorCode::InvalidOperation));
EXPECT_TRUE(gl->getErrors().isEmpty());

gl->clearBufferfv(GL::COLOR, 0, floats3);
EXPECT_TRUE(gl->getErrors().contains(GCGLErrorCode::InvalidOperation));
EXPECT_TRUE(gl->getErrors().isEmpty());

gl->clearBufferfv(GL::COLOR, 0, floats2);
EXPECT_TRUE(gl->getErrors().contains(GCGLErrorCode::InvalidOperation));
EXPECT_TRUE(gl->getErrors().isEmpty());

gl->clearBufferfv(GL::COLOR, 0, floats1);
EXPECT_TRUE(gl->getErrors().contains(GCGLErrorCode::InvalidOperation));
EXPECT_TRUE(gl->getErrors().isEmpty());

gl->clearBufferfv(GL::COLOR, 0, floats0);
EXPECT_TRUE(gl->getErrors().contains(GCGLErrorCode::InvalidOperation));
EXPECT_TRUE(gl->getErrors().isEmpty());

gl->clearBufferfv(GL::DEPTH, 0, floats1);
EXPECT_TRUE(gl->getErrors().isEmpty());

gl->clearBufferfv(GL::DEPTH, 0, floats4);
EXPECT_TRUE(gl->getErrors().contains(GCGLErrorCode::InvalidOperation));
EXPECT_TRUE(gl->getErrors().isEmpty());

int ints2[2] { 1, 2 };
int ints1[1] { 1 };

gl->clearBufferiv(GL::STENCIL, 0, ints1);
EXPECT_TRUE(gl->getErrors().isEmpty());

gl->clearBufferiv(GL::STENCIL, 0, ints2);
EXPECT_TRUE(gl->getErrors().contains(GCGLErrorCode::InvalidOperation));
EXPECT_TRUE(gl->getErrors().isEmpty());

auto texture = gl->createTexture();
gl->bindTexture(GL::TEXTURE_2D, texture);
gl->texParameteri(GL::TEXTURE_2D, GL::TEXTURE_MIN_FILTER, GL::NEAREST);
gl->texImage2D(GL::TEXTURE_2D, 0, GL::R8UI, 1, 1, 0, GL::RED_INTEGER, GL::UNSIGNED_BYTE, 0);
ASSERT_TRUE(gl->getErrors().isEmpty());

auto fbo = gl->createFramebuffer();
gl->bindFramebuffer(GL::FRAMEBUFFER, fbo);
gl->framebufferTexture2D(GL::FRAMEBUFFER, GL::COLOR_ATTACHMENT0, GL::TEXTURE_2D, texture, 0);
ASSERT_EQ(gl->checkFramebufferStatus(GL::FRAMEBUFFER), GL::FRAMEBUFFER_COMPLETE);

unsigned uints4[4] { 1, 2, 3, 4 };
unsigned uints2[2] { 1, 2 };
unsigned uints1[1] { 1 };

gl->clearBufferuiv(GL::COLOR, 0, uints4);
EXPECT_TRUE(gl->getErrors().isEmpty());

gl->clearBufferuiv(GL::COLOR, 0, uints2);
EXPECT_TRUE(gl->getErrors().contains(GCGLErrorCode::InvalidOperation));
EXPECT_TRUE(gl->getErrors().isEmpty());

gl->clearBufferuiv(GL::COLOR, 0, uints1);
EXPECT_TRUE(gl->getErrors().contains(GCGLErrorCode::InvalidOperation));
EXPECT_TRUE(gl->getErrors().isEmpty());

gl = nullptr;
}

TEST_F(GraphicsContextGLCocoaTest, TwoLinks)
{
WebCore::GraphicsContextGLAttributes attributes;
Expand Down Expand Up @@ -453,7 +540,7 @@ static bool hasMultipleGPUs()
if (context->contextAttributes().useMetal)
EXPECT_NE(Thread::current().uid(), signalThreadUID);
else
EXPECT_EQ(Thread::current().uid(), signalThreadUID);
EXPECT_EQ(Thread::current().uid(), signalThreadUID);
}

#if PLATFORM(IOS_FAMILY) || PLATFORM(IOS_FAMILY_SIMULATOR)
Expand Down

0 comments on commit 9b1e21b

Please sign in to comment.