From 6a749cdacdeeaea866ee1497368a468fb0a59fb7 Mon Sep 17 00:00:00 2001
From: Amirali Abdolrashidi <abdolrashidi@google.com>
Date: Fri, 29 Oct 2021 12:45:53 -0700
Subject: [PATCH] Complete validation of glGetAttribLocation()

-Added more checks to glGetAttribLocation() similar to
glBindAttribLocation().
-Added the corresponding unit tests. Using a reserved
prefix in glGetAttribLocation() should return -1.

Bug: angleproject:2419
Change-Id: I3f691f344c7003f855e53d35cd5f9578069acdae
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3252643
Commit-Queue: Amirali Abdolrashidi <abdolrashidi@google.com>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
---
 src/libANGLE/validationES2.cpp                | 24 +++++++++--
 src/tests/gl_tests/WebGLCompatibilityTest.cpp | 40 +++++++++++++++++++
 2 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/src/libANGLE/validationES2.cpp b/src/libANGLE/validationES2.cpp
index 7d5aeb9a6..e706a2c39 100644
--- a/src/libANGLE/validationES2.cpp
+++ b/src/libANGLE/validationES2.cpp
@@ -4335,14 +4335,30 @@ bool ValidateGetAttribLocation(const Context *context,
                                ShaderProgramID program,
                                const GLchar *name)
 {
-    // The WebGL spec (section 6.20) disallows strings containing invalid ESSL characters for
-    // shader-related entry points
-    if (context->isWebGL() && !IsValidESSLString(name, strlen(name)))
+    if (strncmp(name, "gl_", 3) == 0)
     {
-        context->validationError(entryPoint, GL_INVALID_VALUE, kInvalidNameCharacters);
         return false;
     }
 
+    if (context->isWebGL())
+    {
+        const size_t length = strlen(name);
+
+        if (!IsValidESSLString(name, length))
+        {
+            // The WebGL spec (section 6.20) disallows strings containing invalid ESSL characters
+            // for shader-related entry points
+            context->validationError(entryPoint, GL_INVALID_VALUE, kInvalidNameCharacters);
+            return false;
+        }
+
+        if (!ValidateWebGLNameLength(context, entryPoint, length) ||
+            strncmp(name, "webgl_", 6) == 0 || strncmp(name, "_webgl_", 7) == 0)
+        {
+            return false;
+        }
+    }
+
     Program *programObject = GetValidProgram(context, entryPoint, program);
 
     if (!programObject)
diff --git a/src/tests/gl_tests/WebGLCompatibilityTest.cpp b/src/tests/gl_tests/WebGLCompatibilityTest.cpp
index 656cf5d52..cb961eec4 100644
--- a/src/tests/gl_tests/WebGLCompatibilityTest.cpp
+++ b/src/tests/gl_tests/WebGLCompatibilityTest.cpp
@@ -2249,6 +2249,35 @@ TEST_P(WebGLCompatibilityTest, BindAttribLocationLimitation)
     EXPECT_GL_ERROR(GL_INVALID_VALUE);
 }
 
+// Tests getAttribLocation for reserved prefixes
+TEST_P(WebGLCompatibilityTest, GetAttribLocationNameLimitation)
+{
+    GLint attrLocation;
+
+    attrLocation = glGetAttribLocation(0, "gl_attr");
+    EXPECT_GL_NO_ERROR();
+    EXPECT_EQ(-1, attrLocation);
+
+    attrLocation = glGetAttribLocation(0, "webgl_attr");
+    EXPECT_GL_NO_ERROR();
+    EXPECT_EQ(-1, attrLocation);
+
+    attrLocation = glGetAttribLocation(0, "_webgl_attr");
+    EXPECT_GL_NO_ERROR();
+    EXPECT_EQ(-1, attrLocation);
+}
+
+// Tests getAttribLocation for length limits
+TEST_P(WebGLCompatibilityTest, GetAttribLocationLengthLimitation)
+{
+    constexpr int maxLocStringLength = 256;
+    const std::string tooLongString(maxLocStringLength + 1, '_');
+
+    glGetAttribLocation(0, static_cast<const GLchar *>(tooLongString.c_str()));
+
+    EXPECT_GL_ERROR(GL_INVALID_VALUE);
+}
+
 // Test that having no attributes with a zero divisor is valid in WebGL2
 TEST_P(WebGL2CompatibilityTest, InstancedDrawZeroDivisor)
 {
@@ -5404,6 +5433,17 @@ TEST_P(WebGL2CompatibilityTest, BindAttribLocationLimitation)
     EXPECT_GL_ERROR(GL_INVALID_VALUE);
 }
 
+// Tests getAttribLocation for length limit
+TEST_P(WebGL2CompatibilityTest, GetAttribLocationLengthLimitation)
+{
+    constexpr int maxLocStringLength = 1024;
+    const std::string tooLongString(maxLocStringLength + 1, '_');
+
+    glGetAttribLocation(0, static_cast<const GLchar *>(tooLongString.c_str()));
+
+    EXPECT_GL_ERROR(GL_INVALID_VALUE);
+}
+
 // Covers a bug in transform feedback loop detection.
 TEST_P(WebGL2CompatibilityTest, TransformFeedbackCheckNullDeref)
 {