Fix integer overflow in getArraySizeProduct() / ArraySizeProduct() / InnerArraySizeProduct()#107
Open
0xASTRA wants to merge 1 commit into
Open
Fix integer overflow in getArraySizeProduct() / ArraySizeProduct() / InnerArraySizeProduct()#1070xASTRA wants to merge 1 commit into
0xASTRA wants to merge 1 commit into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
786221b to
062e7f3
Compare
…ass) TType::getArraySizeProduct() and gl::ArraySizeProduct()/InnerArraySizeProduct() multiplied attacker-controlled per-dimension array sizes as plain unsigned int with no overflow handling. A WebGL shader declaring an array whose dimension product exceeds UINT_MAX (e.g. float x[65536][65536], product = 2^32 ≡ 0) caused these functions to return a wrapped, near-zero value. That wrapped value is consumed by security-relevant arithmetic, notably CalculateVariableSize() which feeds TParseContext::checkVariableSize() — the WebGL oversize-variable guard (rejectWebglShadersWithLargeVariables). The CheckedNumeric there cannot detect that the unsigned int operand already wrapped, so the oversize variable passes the guard. VariablePacker's packing limit check is bypassed the same way. Saturate to UINT_MAX on overflow, mirroring the existing saturating behavior of TType::getObjectSize() and getLocationCount(), and the CheckedNumeric hardening recently added to TIntermAggregate::getConstantValue() (crbug.com/498400132).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TType::getArraySizeProduct()(src/compiler/translator/Types.cpp:560),gl::ArraySizeProduct()andgl::InnerArraySizeProduct()(
src/common/utilities.cpp:977/987) multiply per-dimension sizes as plainunsigned intwith no overflow handling. A shader declaringfloat x[65536][65536]produces a product of2^32 ≡ 0, which collapsesCalculateVariableSize()to zero and silently passesTParseContext::checkVariableSize()— the guard (ParseContext.cpp:1767)that exists specifically to prevent driver-side integer-size overflows in
WebGL contexts (
rejectWebglShadersWithLargeVariables = true).The same wrapped value bypasses the register-packing limit in
VariablePacker.cpp:220and feeds buffer/loop-count calculations in theD3D, Metal and HLSL back-ends (~30 call sites, e.g.
ProgramExecutableD3D.cpp:1476).This is a variant of the class hardened in
TIntermAggregate::getConstantValue()(
IntermNode.cpp:879,CheckedNumeric … ValueOrDie(),crbug.com/498400132) — that fix covered the constant-folding allocationpath but missed these type-level product functions that feed the oversize guard.
Fix
Saturate the product to
UINT_MAXon overflow in all three functions,matching the existing behaviour of
getObjectSize()andgetLocationCount()on the same type.
Minimal reproduction
Verified:
getArraySizeProduct({65536u, 65536u})returns0; after fix returnsUINT_MAX.