CVE-2026-14384
Overview
Files Changed
src/compiler/translator/IntermNode.cppsrc/compiler/translator/IntermNode.hsrc/compiler/translator/spirv/OutputSPIRV.cpp
Patch
From 3b5b2ff3d0bec7e42e89adb9c0af7bbca92be859 Mon Sep 17 00:00:00 2001 From: Zhenyao Mo <[email protected]> Date: Tue, 05 May 2026 14:04:36 -0700 Subject: [PATCH] [angle] Use full logic for HLSL short-circuit unfolding. The full logic is implemented in OutputSPIRV.cpp, so moving that into TIntermNode and TIntermOperator, so both sides can use it. Bug: b/497543485 Change-Id: I4e9e2ab84d28193f3cd459356748727f48e7a450 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/7816349 Auto-Submit: Zhenyao Mo <[email protected]> Reviewed-by: Geoff Lang <[email protected]> Commit-Queue: Geoff Lang <[email protected]> Reviewed-by: Shahbaz Youssefi <[email protected]> --- diff --git a/src/compiler/translator/IntermNode.cpp b/src/compiler/translator/IntermNode.cpp index afc4922..4459401 100644 --- a/src/compiler/translator/IntermNode.cpp +++ b/src/compiler/translator/IntermNode.cpp @@ -1018,6 +1018,22 @@ return false; } +bool TIntermAggregate::isSafeToExecuteInShortCircuit() const +{ + if (mOp == EOpConstruct) + { + for (TIntermNode *component : mArguments) + { + if (!component->isSafeToExecuteInShortCircuit()) + { + return false; + } + } + return true; + } + return false; +} + void TIntermBlock::appendStatement(TIntermNode *statement) { // Declaration nodes with no children can appear if it was an empty declaration or if all the @@ -1298,6 +1314,17 @@ } } +bool TIntermOperator::isShortCircuitNeeded() const +{ + if (mOp != EOpLogicalAnd && mOp != EOpLogicalOr) + { + return false; + } + + ASSERT(getChildCount() == 2); + return !getChildNode(1)->isSafeToExecuteInShortCircuit(); +} + TOperator TIntermBinary::GetMulOpBasedOnOperands(const TType &left, const TType &right) { if (left.isMatrix()) @@ -1776,6 +1803,11 @@ return mSwizzleOffsets.size() == 1 && mSwizzleOffsets[0] == offset; } +bool TIntermSwizzle::isSafeToExecuteInShortCircuit() const +{ + return mOperand->isSafeToExecuteInShortCircuit(); +} + ImmutableString TIntermSwizzle::getOffsetsAsXYZW() const { ImmutableStringBuilder offsets(mSwizzleOffsets.size()); @@ -2321,6 +2353,12 @@ return structure->fields()[index]->name(); } +bool TIntermBinary::isSafeToExecuteInShortCircuit() const +{ + return (mOp == EOpIndexDirectInterfaceBlock || mOp == EOpIndexDirectStruct) && + mLeft->isSafeToExecuteInShortCircuit(); +} + TIntermTyped *TIntermUnary::fold(TDiagnostics *diagnostics) { TConstantUnion *constArray = nullptr; diff --git a/src/compiler/translator/IntermNode.h b/src/compiler/translator/IntermNode.h index 1d85837..002c634 100644 --- a/src/compiler/translator/IntermNode.h +++ b/src/compiler/translator/IntermNode.h @@ -119,6 +119,10 @@ // node and it is replaced; otherwise, return false. virtual bool replaceChildNode(TIntermNode *original, TIntermNode *replacement) = 0; + // True if executing the expression represented by this node is safe to execute even if it's + // inside a short-circuited expression's branch that's not taken. + virtual bool isSafeToExecuteInShortCircuit() const { return false; } + TIntermNode *getAsNode() { return this; } protected: @@ -299,6 +303,8 @@ bool hasSideEffects() const override { return false; } + bool isSafeToExecuteInShortCircuit() const override { return true; } + const TType &getType() const override; const TSymbolUniqueId &uniqueId() const; @@ -360,6 +366,8 @@ bool hasSideEffects() const override { return false; } + bool isSafeToExecuteInShortCircuit() const override { return true; } + int getIConst(size_t index) const { return mUnionArrayPointer ? mUnionArrayPointer[index].getIConst() : 0; @@ -439,6 +447,8 @@ bool hasSideEffects() const override { return isAssignment(); } + bool isShortCircuitNeeded() const; + protected: TIntermOperator(TOperator op) : TIntermExpression(TType(EbtFloat, EbpUndefined)), mOp(op) {} TIntermOperator(TOperator op, const TType &type) : TIntermExpression(type), mOp(op) {} @@ -466,6 +476,8 @@ bool hasSideEffects() const override { return mOperand->hasSideEffects(); } + bool isSafeToExecuteInShortCircuit() const override; + TIntermTyped *getOperand() { return mOperand; } ImmutableString getOffsetsAsXYZW() const; void writeOffsetsAsXYZW(TInfoSinkBase *out) const; @@ -523,6 +535,8 @@ return isAssignment() || mLeft->hasSideEffects() || mRight->hasSideEffects(); } + bool isSafeToExecuteInShortCircuit() const override; + TIntermTyped *getLeft() const { return mLeft; } TIntermTyped *getRight() const { return mRight; } TIntermTyped *fold(TDiagnostics *diagnostics) override; @@ -651,6 +665,8 @@ bool hasSideEffects() const override; + bool isSafeToExecuteInShortCircuit() const override; + TIntermTyped *fold(TDiagnostics *diagnostics) override; TIntermSequence *getSequence() override { return &mArguments; } diff --git a/src/compiler/translator/spirv/OutputSPIRV.cpp b/src/compiler/translator/spirv/OutputSPIRV.cpp index 6f1139d..75fe239 100644 --- a/src/compiler/translator/spirv/OutputSPIRV.cpp +++ b/src/compiler/translator/spirv/OutputSPIRV.cpp @@ -2406,77 +2406,6 @@ nodeDataInitRValue(&mNodeData.back(), castResultId, intTypeId); } -// If an expression is short-circuited, it must not be executed. However, in some cases there is -// nothing to execute, such as constants, variables etc. Notably, hasSideEffects() is not -// a sufficient check, because it could include read-only operations that are out of bounds, despite -// not having any side effects. -bool IsSafeToExecuteInShortCircuit(TIntermTyped *node) -{ - // Constants and symbols are safe to execute. - if (node->getAsConstantUnion() || node->getAsSymbolNode()) - { - return true; - } - - // Swizzle is safe if the operand is safe. - { - TIntermSwizzle *asSwizzle = node->getAsSwizzleNode(); - if (asSwizzle) - { - return IsSafeToExecuteInShortCircuit(asSwizzle->getOperand()); - } - } - - // Indexing a struct or interface block is safe to execute, as long as no array index is in the - // access chain. - { - TIntermBinary *asBinary = node->getAsBinaryNode(); - if (asBinary != nullptr) - { - return (asBinary->getOp() == EOpIndexDirectInterfaceBlock || - asBinary->getOp() == EOpIndexDirectStruct) && - IsSafeToExecuteInShortCircuit(asBinary->getLeft()); - } - } - - // Constructors are safe as long as every member is safe. - { - TIntermAggregate *asAggregate = node->getAsAggregate();
Regression Test / PoC
diff --git a/src/tests/gl_tests/GLSLTest.cpp b/src/tests/gl_tests/GLSLTest.cpp
index eed6360..f0d3e8f 100644
--- a/src/tests/gl_tests/GLSLTest.cpp
+++ b/src/tests/gl_tests/GLSLTest.cpp
@@ -3913,6 +3913,51 @@
}
}
+// Test that short circuiting works correctly even if the right hand side has no side effects but
+// is otherwise unsafe to execute (e.g. contains an out-of-bounds array access).
+// This is a regression test for a bug where the HLSL backend would not unfold short-circuit
+// expressions if the right hand side had no side effects.
+TEST_P(GLSLTest_ES3, ShortCircuitUnsafeExpressionNoSideEffects)
+{
+ // Fragment shader based on a reproduction case for a bug where short-circuit unfolding
+ // was gated by hasSideEffects().
+ constexpr char kFS[] = R"(#version 300 es
+precision highp float;
+uniform int idx;
+uniform bool safe;
+out vec4 color;
+void main() {
+ float data[4] = float[4](0.25, 0.5, 0.75, 1.0);
+ // hasSideEffects() == false on the RHS -> should be unfolded because it's unsafe.
+ bool hit = safe && (data[idx] > 0.0);
+ // If short-circuiting works, 'hit' must be false when 'safe' is false,
+ // regardless of 'idx'.
+ color = vec4(hit ? 1.0 : 0.0, 0.0, 0.0, 1.0);
+})";
+
+ ANGLE_GL_PROGRAM(program, essl3_shaders::vs::Simple(), kFS);
+ glUseProgram(program);
+
+ GLint idxLocation = glGetUniformLocation(program, "idx");
+ GLint safeLocation = glGetUniformLocation(program, "safe");
+
+ // Baseline: safe=true, idx=0. hit should be true.
+ glUniform1i(safeLocation, 1);
+ glUniform1i(idxLocation, 0);
+ drawQuad(program, essl3_shaders::PositionAttrib(), 0.5f);
+ EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
+
+ // Test: safe=false, idx=100 (OOB). hit should be false.
+ // On some drivers/backends, if it's NOT short-circuited, this might crash or
+ // return an unexpected value (e.g. if OOB read returns something > 0.0).
+ glUniform1i(safeLocation, 0);
+ glUniform1i(idxLocation, 100);
+ drawQuad(program, essl3_shaders::PositionAttrib(), 0.5f);
+ EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::black);
+
+ ASSERT_GL_NO_ERROR();
+}
+
// Test that nesting ternary and short-circuitting operators work.
TEST_P(GLSLTest, NestedTernaryAndShortCircuit)
{
Original Bug Report
Potential info leak in ANGLE HLSL backend via unsafe short-circuit evaluation
Project Fortify, an experimental security project, has identified the following potential security issue.
Overview: The ANGLE HLSL backend fails to unfold logical short-circuit expressions containing unsafe but side-effect-free operations, such as indirect array accesses. On the Windows D3D backend, where local array index clamping is disabled by the passthrough decoder, this results in unconditional out-of-bounds reads. This can potentially leak uninitialized GPU temporary registers containing cross-origin data.
Affected files:
third_party/angle/src/compiler/translator/tree_util/IntermNodePatternMatcher.cppthird_party/angle/src/compiler/translator/hlsl/OutputHLSL.cpp
Estimated timestamp from git blame: 2016-12-12
Description
There is a potential out-of-bounds read and information leak vulnerability in ANGLE’s HLSL backend due to the intersection of two logic flaws: improper short-circuit expression unfolding and missing indirect array bounds clamping.
When compiling WebGL shaders to HLSL, ANGLE transforms the AST. Because HLSL does not guarantee short-circuiting for && and || operators (the HLSL compiler often flattens the execution path, evaluating both sides), ANGLE uses the UnfoldShortCircuitToIf pass to convert these expressions into explicit if statements.
However, the pattern matcher for this pass (IntermNodePatternMatcher::matchInternal in third_party/angle/src/compiler/translator/tree_util/IntermNodePatternMatcher.cpp, lines 52-59) only unfolds the expression if the right-hand side has side effects:
if ((mMask & kUnfoldedShortCircuitExpression) != 0)
{
if (node->getRight()->hasSideEffects() &&
(node->getOp() == EOpLogicalOr || node->getOp() == EOpLogicalAnd))
{
return true;
}
}
Operations like indirect array indexing (EOpIndexIndirect) do not have side effects. Consequently, an expression like safe && (data[idx] > 0.0) is not unfolded. The HLSL code generator (OutputHLSL.cpp) then emits a raw && operator, incorrectly assuming that any expression requiring short-circuiting has already been handled.
Compounding this issue, the Chrome WebGL passthrough command decoder on Windows (which uses the D3D11 backend) bypasses the manual ShaderTranslator initialization that normally enables clampIndirectArrayBounds. Furthermore, the D3D backend itself (ShaderD3D.cpp) does not explicitly enable options->clampIndirectArrayBounds = true (unlike ShaderVk.cpp and ShaderGL.cpp). Therefore, indirect indices on local HLSL arrays are not clamped.
When the shader executes on the GPU, the flattened execution path unconditionally evaluates the out-of-bounds array access. D3D11’s robustness guarantees prevent a full crash, but the read will fetch values from adjacent indexable temporary registers (x registers). If the GPU driver has not zero-initialized these registers between executions, this allows an attacker to leak cross-origin WebGL data or data from other GPU processes.
Potential Reproduction Steps
Note: These are theoretical steps derived from code analysis; our tooling agent does not currently have the ability to run code to verify the exploit end-to-end.
- On a Windows Chrome instance (using the D3D11 ANGLE backend and passthrough decoder), create a WebGL 2.0 context.
- Compile a fragment shader that utilizes a logical AND with an out-of-bounds array read on the right-hand side:
#version 300 es precision highp float; uniform int idx; uniform bool safe; out vec4 color; void main() { float data[4] = float[4](1., 2., 3., 4.); bool b = safe && (data[idx] > 0.0); color = vec4(b ? 1.0 : 0.0); } - Bind the shader and set the uniform
safe = falseandidxto a large out-of-bounds value. - Execute a draw call and read back the resulting pixels. Observe that the result depends on the evaluation of
data[idx](leaking GPU temporary register state) despitesafebeing false.
Suggested Fix
- Fix Short-Circuit Unfolding: In
IntermNodePatternMatcher::matchInternal, replace the reliance onhasSideEffects()with a more comprehensive safety check. The SPIR-V backend already implements anIsSafeToExecuteInShortCircuit()function (third_party/angle/src/compiler/translator/spirv/OutputSPIRV.cpp), which correctly identifies that array indexing is unsafe to execute speculatively. This logic should be abstracted and reused for the HLSL backend. - Enable Index Clamping: Ensure that
clampIndirectArrayBoundsis enabled for the D3D backend. Inthird_party/angle/src/libANGLE/renderer/d3d/ShaderD3D.cpp, explicitly setoptions->clampIndirectArrayBounds = true;duringShaderD3D::compileto provide defense-in-depth against OOB local array reads.
Evaluated with Chrome root at commit: a9cbf6e8b275fe4147435aa905f3b7f5a656f5f0
Results from 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.