Medium chrome Type Confusion 📄 Reporter bug report 🔧 Commit mapped

Overview

Medium
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactType Confusion in ANGLE
DescriptionType Confusion in ANGLE
ComponentANGLE
Bug ClassType Confusion
Tracker500030250
Fix commit4be13e30b547 (angle/angle) +145/-21
CISA KEVNot listed
CreditedGoogle
Disclosed2026-06-30

Changed Functions

FunctionChangeNotes
PruneNoOpsTraverser
src/compiler/translator/tree_ops/PruneNoOps.cpp
modified

Files Changed

  • src/compiler/translator/tree_ops/PruneNoOps.cpp
From 4be13e30b547e9e77113824aa36557370a45c2cc Mon Sep 17 00:00:00 2001
From: Shahbaz Youssefi <syoussefi@chromium.org>
Date: Fri, 22 May 2026 11:45:18 -0400
Subject: [PATCH] Translator: Prune comma expressions more aggressively

With this change, `(a[side_effect].b, c)` is transformed to
`(side_effect, c)`.  Also expressions such as `(side_effect, b, c)` are
transformed to `(side_effect, c)`.

Bug: chromium:500030250
Bug: chromium:518243653
Change-Id: I7c2f853f0f39b246f44bf4ad383115dfcab31309
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/7872078
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Reviewed-by: Yuxin Hu <yuxinhu@google.com>
---

diff --git a/src/compiler/translator/tree_ops/PruneNoOps.cpp b/src/compiler/translator/tree_ops/PruneNoOps.cpp
index 6e832c1..0895324 100644
--- a/src/compiler/translator/tree_ops/PruneNoOps.cpp
+++ b/src/compiler/translator/tree_ops/PruneNoOps.cpp
@@ -99,6 +99,15 @@
     return !node->getAsTyped()->hasSideEffects();
 }
 
+enum class CommaExpression
+{
+    // The LHS of the comma expression will get thrown away, so it can be decomposed and its side
+    // effects extracted
+    ThrowAway,
+    // The RHS of the comma expression needs to be retained as its result is used.
+    FinalResult,
+};
+
 class PruneNoOpsTraverser : private TIntermTraverser
 {
   public:
@@ -111,9 +120,12 @@
     bool visitDeclaration(Visit, TIntermDeclaration *node) override;
     bool visitSwitch(Visit visit, TIntermSwitch *node) override;
     bool visitBlock(Visit visit, TIntermBlock *node) override;
+    bool visitBinary(Visit visit, TIntermBinary *node) override;
     bool visitLoop(Visit visit, TIntermLoop *loop) override;
     bool visitBranch(Visit visit, TIntermBranch *node) override;
-    TIntermTyped *pruneNoOpCommaExpressions(TIntermTyped *statement);
+    TIntermTyped *pruneCommaThrowAwayExpression(TIntermTyped *statement);
+    TIntermTyped *pruneNoOpCommaExpressions(TIntermTyped *statement, CommaExpression commaExpr);
+    TIntermTyped *mergePrunedNoOpCommaExpressions(TIntermTyped *lhs, TIntermTyped *rhs);
 
     bool mIsBranchVisited = false;
 
@@ -129,15 +141,12 @@
 }
 
 PruneNoOpsTraverser::PruneNoOpsTraverser(TSymbolTable *symbolTable)
-    : TIntermTraverser(true, true, true, symbolTable)
+    : TIntermTraverser(true, false, false, symbolTable)
 {}
 
 bool PruneNoOpsTraverser::visitDeclaration(Visit visit, TIntermDeclaration *node)
 {
-    if (visit != PreVisit)
-    {
-        return true;
-    }
+    ASSERT(visit == PreVisit);
 
     TIntermSequence *sequence = node->getSequence();
     if (sequence->size() >= 1)
@@ -195,9 +204,10 @@
                 queueReplacementWithParent(node, declaratorSymbol, new TIntermSymbol(variable),
                                            OriginalNode::IS_DROPPED);
             }
+            return false;
         }
     }
-    return false;
+    return true;
 }
 
 bool PruneNoOpsTraverser::visitSwitch(Visit visit, TIntermSwitch *node)
@@ -295,7 +305,8 @@
         // the ones with side effect back together with comma.
         if (statement->getAsBinaryNode() != nullptr)
         {
-            statement = pruneNoOpCommaExpressions(statement->getAsBinaryNode());
+            statement = pruneNoOpCommaExpressions(statement->getAsBinaryNode(),
+                                                  CommaExpression::FinalResult);
             if (statement == nullptr)
             {
                 continue;
@@ -319,19 +330,93 @@
     return false;
 }
 
-TIntermTyped *PruneNoOpsTraverser::pruneNoOpCommaExpressions(TIntermTyped *statement)
+bool PruneNoOpsTraverser::visitBinary(Visit visit, TIntermBinary *node)
+{
+    if (node->getOp() == EOpComma && getParentNode()->getAsBlock() == nullptr)
+    {
+        // Prune LHS of the comma.  This is not done if the parent is a block node because
+        // visitBlock() already does it.
+        TIntermTyped *prunedLeft = pruneCommaThrowAwayExpression(node->getLeft());
+        if (prunedLeft != node->getLeft())
+        {
+            // If completely pruned, replace with RHS, otherwise replace the LHS with its side
+            // effects.
+            queueReplacement(prunedLeft != nullptr
+                                 ? new TIntermBinary(EOpComma, prunedLeft, node->getRight())
+                                 : node->getRight(),
+                             OriginalNode::IS_DROPPED);
+
+            node->getRight()->traverse(this);
+            if (prunedLeft)
+            {
+                prunedLeft->traverse(this);
+            }
+            return false;
+        }
+    }
+
+    return true;
+}
+
+TIntermTyped *PruneNoOpsTraverser::pruneCommaThrowAwayExpression(TIntermTyped *statement)
+{
+    if (IsNoOp(statement))
+    {
+        return nullptr;
+    }
+
+    TIntermBinary *asBinary = statement->getAsBinaryNode();
+    if (asBinary == nullptr)
+    {
+        return statement;
+    }
+
+    switch (asBinary->getOp())
+    {
+        case EOpIndexDirect:
+        case EOpIndexDirectStruct:
+        case EOpIndexDirectInterfaceBlock:
+            return pruneNoOpCommaExpressions(asBinary->getLeft(), CommaExpression::ThrowAway);
+        case EOpIndexIndirect:
+        case EOpComma:
+        {
+            // Prune both the indexed and the indexee.  If both have side effects, join them with a
+            // comma.
+            //
+            // Same with a comma operation's left and right hand side expressions.  Since
+            // |statement| is itself the LHS of a comma operation, both its LHS and RHS can be
+            // pruned.
+            TIntermTyped *prunedLeft =
+                pruneNoOpCommaExpressions(asBinary->getLeft(), CommaExpression::ThrowAway);
+            TIntermTyped *prunedRight =
+                pruneNoOpCommaExpressions(asBinary->getRight(), CommaExpression::ThrowAway);
+            return mergePrunedNoOpCommaExpressions(prunedLeft, prunedRight);
+        }
+        default:
+            return statement;
+    }
+}
+
+TIntermTyped *PruneNoOpsTraverser::pruneNoOpCommaExpressions(TIntermTyped *statement,
+                                                             CommaExpression commaExpr)
 {
     TIntermBinary *commaSeparatedExpressions = statement->getAsBinaryNode();
     if (commaSeparatedExpressions == nullptr || commaSeparatedExpressions->getOp() != EOpComma)
     {
-        return statement;
+        // If this is not the final result of comma, try to extract side effect out of the
+        // expression and throw the rest away.  In an expression like
+        // |struct_with_sampler[side_effect]|, this allows it to be replaced by |side_effect| alone.
+        return commaExpr == CommaExpression::ThrowAway ? pruneCommaThrowAwayExpression(statement)
+                                                       : statement;
     }
 
     TIntermTyped *left  = commaSeparatedExpressions->getLeft();
     TIntermTyped *right = commaSeparatedExpressions->getRight();
 
-    TIntermTyped *prunedLeft  = IsNoOp(left) ? nullptr : pruneNoOpCommaExpressions(left);
-    TIntermTyped *prunedRight = IsNoOp(right) ? nullptr : pruneNoOpCommaExpressions(right);
+    TIntermTyped *prunedLeft =
+        IsNoOp(left) ? nullptr : pruneNoOpCommaExpressions(left, CommaExpression::ThrowAway);
+    TIntermTyped *prunedRight =
+        IsNoOp(right) ? nullptr : pruneNoOpCommaExpressions(right, commaExpr);
 
     if (left == prunedLeft && right == prunedRight)
     {
@@ -339,26 +424,29 @@
         return statement;
     }
 
+    return mergePrunedNoOpCommaExpressions(prunedLeft, prunedRight);
+}
+
+TIntermTyped *PruneNoOpsTraverser::mergePrunedNoOpCommaExpressions(TIntermTyped *lhs,
+                                                                   TIntermTyped *rhs)
+{
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/src/tests/angle_end2end_tests_expectations.txt b/src/tests/angle_end2end_tests_expectations.txt
index 9f95376..1090127 100644
--- a/src/tests/angle_end2end_tests_expectations.txt
+++ b/src/tests/angle_end2end_tests_expectations.txt
@@ -428,6 +428,7 @@
 497533569 MAC INTEL OPENGL : TransformFeedbackTest.ProgramSwitchDuringPauseAndResume/* = SKIP
 497533569 MAC INTEL OPENGL : TransformFeedbackTest.ProgramSwitchDuringPauseAndResumeWithBufferChange/* = SKIP
 513172707 MAC INTEL OPENGL : DrawBaseVertexBaseInstanceTest_ES3.InstanceIDDoesNotIncludeBaseInstance/* = SKIP
+519471062 MAC OPENGL : GLSLTest_ES3.SamplerInStructRHSOfCommaWithSideEffect/* = SKIP
 
 // BlitFramebufferTest.ScissoredMultisampleStencil failures
 42262159 MAC INTEL OPENGL : BlitFramebufferTest.ScissoredMultisampleStencil/* = SKIP
diff --git a/src/tests/gl_tests/GLSLTest.cpp b/src/tests/gl_tests/GLSLTest.cpp
index 79ed94c..cd8223f 100644
--- a/src/tests/gl_tests/GLSLTest.cpp
+++ b/src/tests/gl_tests/GLSLTest.cpp
@@ -6997,6 +6997,41 @@
 }
 
 // Test that samplers in structs can be used on the right-hand side of a comma, where the expression
+// has side effect.
+TEST_P(GLSLTest_ES3, SamplerInStructRHSOfCommaWithSideEffect)
+{
+    constexpr char kFS[] = R"(#version 300 es
+precision mediump float;
+uniform struct {
+    sampler2D n;
+    vec2 c;
+} s[4];
+ivec4 global = ivec4(0);
+out vec4 color;
+void main()
+{
+    int i = 0;
+    (s, s[i += 1].n), (global.x = 10);
+    s[i += 2], s[i].c, (s[i - 1].n, (global.y = 20));
+    s[global.w = 80, i += 4].c, (global.z = 40);
+
+    int c11 = ((s, s[i += 8].n), (global.x += 1));
+    int c22 = (s[i += 16], s[i].c, (s[i - 1].n, (global.y += 2)));
+    int c43 = (s[global.w += 4, i += 32].c, (global.z += 3));
+    vec4 allOnes = vec4(((i += 64, s), 1.0));
+
+    color = vec4(i == 127,
+                 global.x == 11 && global.y == 22,
+                 global.z == 43 && global.w == 84,
+                 c11 == 11 && c22 == 22 && c43 == 43) * allOnes;
+})";
+
+    ANGLE_GL_PROGRAM(program, essl3_shaders::vs::Simple(), kFS);
+    drawQuad(program, essl3_shaders::PositionAttrib(), 0.5f);
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::white);
+}
+
+// Test that samplers in structs can be used on the right-hand side of a comma, where the expression
 // has side effect, and that the struct field can be selected on the comma expression.
 TEST_P(GLSLTest_ES3, SamplerInStructRHSOfCommaWithSideEffectWithSelectField)
 {
Loading diff…

Original Bug Report

reported by vm...@google.com

Sandbox Escape in GPU Process via Type Confusion in ANGLE RewriteStructSamplers

Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports without the security team.

Overview: A logic error in ANGLE’s RewriteStructSamplers AST pass causes incorrect traversal of EOpComma operators during struct field resolution. This results in type-confused AST nodes, which subsequently causes OutputSPIRV to generate structurally invalid SPIR-V instructions. Passing this malformed SPIR-V to native Vulkan drivers can cause memory corruption in the driver’s shader compiler, leading to a potential sandbox escape from WebGL.

Affected files:

  • third_party/angle/src/compiler/translator/tree_ops/RewriteStructSamplers.cpp
  • third_party/angle/src/compiler/translator/ParseContext.cpp
  • third_party/angle/src/compiler/translator/IntermNode.cpp
  • third_party/angle/src/compiler/translator/spirv/OutputSPIRV.cpp

Estimated timestamp from git blame: 2024-07-30

Summary

A vulnerability exists in ANGLE’s RewriteStructSamplers AST transformation pass. The function RewriteModifiedStructFieldSelectionExpression incorrectly assumes it can trace the base uniform of a struct field selection by repeatedly traversing the left child of binary nodes. When a comma operator (EOpComma) is present, it mistakenly traverses into the unevaluated left operand instead of the right operand. In release builds, an unhandled node type within a switch statement causes the comma operator to be silently dropped, leading to a type-confused AST and the generation of invalid SPIR-V bytecode.

Technical Details

  1. Incorrect Traversal: During the RewriteStructSamplers pass, ANGLE needs to trace field selections (e.g., (a[i++], b).s) back to their base uniform to extract samplers into global scope. RewriteModifiedStructFieldSelectionExpression uses a while loop that iterates using iter = iter->getLeft()->getAsBinaryNode(). For an EOpComma node, getLeft() returns the left operand (a[i++]), failing to traverse the semantically correct right operand (b) which holds the actual struct being accessed. This misidentifies the base uniform.
  2. Silently Dropping Nodes: During the rebuild phase, the code iterates over the collected nodes bottom-up. The switch (indexNode->getOp()) block lacks a case EOpComma:, falling through to default:, which calls UNREACHABLE(). In Chromium release builds, UNREACHABLE() expands to a no-op macro (ASSERT(false) becomes true ? static_cast<void>(0) : ...). As a result, the comma node and its correct right-hand side are silently dropped, and the incorrectly identified left-hand uniform is substituted in its place.
  3. AST Type Confusion: The parent TIntermAggregate node for an associated function call (e.g., texture()) is not updated. Its constant TFunction pointer still expects the original arguments (e.g., sampler2D), but its child node now evaluates to the incorrectly substituted type (e.g., samplerCube).
  4. Malformed SPIR-V Generation: During SPIR-V generation, OutputSPIRVTraverser::createImageTextureBuiltIn uses the original function signature to determine coordinate requirements (e.g., expecting vec2 coordinates for a 2D sampler) but loads the actual sampler ID from the rewritten, type-confused child node. This emits structurally invalid SPIR-V instructions, such as OpImageSampleImplicitLod attempting to sample a Cube image using 2-component coordinates.
  5. Validation Bypass: At the end of OutputSPIRV, ANGLE calls ASSERT(spirv::Validate(result)). Because ASSERT is compiled out in release builds, ANGLE fails to catch the malformed SPIR-V and passes it directly to vkCreateShaderModule.

Potential Exploitation Scenario

An attacker can construct a malicious WebGL 2.0 fragment shader to trigger this vulnerability:

#version 300 es
precision highp float;
struct S1 { samplerCube s; };
struct S2 { sampler2D s; float pad; };
uniform S1 a[2];
uniform S2 b;
out vec4 fragColor;
int i = 0;
void main() {
    // The comma operator survives folding due to the side effect in a[i++].
    // The rewriter incorrectly extracts the sampler from 'a' instead of 'b'.
    fragColor = texture((a[i++], b).s, vec2(0.5));
}

By serving this shader via WebGL, the attacker forces the GPU process to send malformed SPIR-V to the underlying proprietary Vulkan driver (e.g., AMD, NVIDIA, Intel, Adreno, Mali). These drivers’ JIT compilers are highly complex and often assume the incoming SPIR-V is strictly valid. Encountering instructions with mismatched image and coordinate dimensions can break compiler invariants, leading to out-of-bounds reads/writes during register allocation or code emission. This provides a direct vector for Remote Code Execution and Sandbox Escape within the highly privileged GPU process.

Note: These steps describe a potential exploitation path; our automated tooling does not currently run the exploit code to verify the final outcome in the Vulkan driver.

Suggested Fix

  1. Modify RewriteModifiedStructFieldSelectionExpression in RewriteStructSamplers.cpp to properly handle EOpComma. When tracing the base uniform, the traversal must correctly follow getRight() for EOpComma nodes, as the right operand determines the evaluated struct.
  2. Ensure that the rebuilding phase of RewriteModifiedStructFieldSelectionExpression properly reconstructs the EOpComma node, preserving both the left-hand side effects and the right-hand struct access.
  3. Consider enforcing a strict spirv::Validate check in release builds before passing shaders to Vulkan drivers, or emitting a safe default shader if validation fails, to harden the GPU process against driver-level compiler vulnerabilities.

Evaluated with Chrome root at commit: f200f57a19490707ff8bc7aa5de3cbc443a3afad


Results so far have been promising, but there can be wrong deductions. If this proves to be a false positive, please close as WAI; data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.

View on issue tracker