668366 - Investigate whether WebGL shaders can make out-of-bounds accesses into uniform arrays

Status: RESOLVED WORKSFORME

(Core :: Graphics: CanvasWebGL, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

A common concern about WebGL is whether shaders can make illegal accesses into video memory by addressing array-like structures out-of-bounds.

For textures that's impossible since there is no such thing as out-of-bounds accesses: out-of-bounds texture coordinates have completely well-defined semantics defined by the glTexParameter functions, and real code relies on that all the time.

For vertex attrib arrays that's impossible by the very nature of the shading language: shaders can't even specify an index when addressing them, the index is prescribed by the calling context. Code says 'array', not 'array[i]'.

That leaves uniform arrays. This has been discussed on ANGLE bug 49:
http://code.google.com/p/angleproject/issues/detail?id=49

It would be nice to run a testcase on some platforms to see if one ever succeeds making an out-of-bounds access into a uniform array.

It seems that on Windows with D3D via ANGLE, there is no possibility of out-of-bounds access, but that would be nice to check for real.

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

Notice that if a testcase shows this to be a real issue, ANGLE could be extended to perform static analysis to check against such out-of-bounds accesses. Indeed, the WebGL shading language is AFAIK restricted enough wrt conditional branching that this can be validated at compile time.

Comment 2

[Christoph Diehl [:posidron]]

Attachment: Uniform OOB Read Test

If you define a Uniform array of 4 and you want to access the 5th element the compiler responds with: '[' :  array index out of range '5'

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

Thanks, that's a great start, but it's an easy case for the shader compiler as it passes '5' as a literal value. Can you please try what ANGLE bug 49 suggests: use a vertex attrib as the index:

  attribute int index;
  uniform vec4 positions[4];

  void main() {
    gl_Position = positions[index];
  }

In this case, the compiler won't be able to know at compile time the value of 'index', since it could be set at runtime by a webgl.vertexAttrib1i call. In this case, the only way that the compiler could catch that is by generating code that clamps 'index' at runtime, or perhaps just skips the array access, leaving gl_Position unchanged.

How to test: in a fragment shader I would set up a uniform array with some colors, then I would do

   vec4 result = SomeLightGrayColor; 
   result *= colors[index];
   gl_FragColor = result;

and call that with various values of index at runtime. If the out-of-bounds accesses are omitted, then the resulting color will be SomeLightGrayColor. If the out-of-bounds accesses are clamped then the resulting color will be the last entry in the colors[] array multiplied by SomeLightGrayColor. The point of doing this *= business is to prevent the shader compiler from optimizing away the initial result = SomeLightGrayColor;

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

Also, I was a bit too optimistic in comment 1: since a vertex attrib can be used as the index, in some cases as discussed in comment 3, the best one can do is detect that and clamp the indices.

Comment 5

[Kelsey Gilbert [:jgilbert]]

We're safe here these days.