Issue metadata
Sign in to add a comment
|
Unused attributes may be read out-of-bounds by drivers |
||||||||||||||||||||||
Issue descriptionForked from item 3 of https://bugs.chromium.org/p/chromium/issues/detail?id=738228#c7 If a vertex shader attribute is not used after linking the program, we expect the driver to never read the data (whether in host memory or buffer objects). In particular, we don't check for out-of-bounds. However the spec doesn't prohibit the drivers from reading the data even if unused. We don't currently have direct evidence that drivers do out-of-bounds reads in this case, but it hasn't been tested extensively. Forcing a GL_INVALID_OPERATION if there is out-of-bounds risk for unused attributes (like we do for used ones) is problematic, as it breaks the WebGL spec (https://www.khronos.org/registry/webgl/specs/latest/1.0/#6.6). Another solution would be to disable arrays that are bound to unused locations. This would eliminate the problem, but the cost could be non-trivial (possibly many glEnable/DisableVertexAttribArray per draw-call) if not done carefully. Marking as Security_Severity-Low because even though this would be OOB reads, the data never surfaces back to the attacker (since the attribute is unused), eliminating the potential for information leak.
,
Jul 7 2017
,
Jul 10 2017
,
Aug 23 2017
Apparently this problem is real. Kai is looking at it
,
Aug 23 2017
Kai, please take a look at GLES2DecoderImpl::AttribsTypeMatch() function. It has the masks you will need (from programs which attrbs are used, from VertexAttribManager which ones are enabled), so you can do something similar per draw call: For most draw calls, the attribs that are not used should already be disabled, so that should be the optimized code path.
,
Aug 23 2017
In particular, on macOS, attribute 0 has this issue if the bound buffer is unallocated or of size zero. It causes a null dereference in the GL driver. I don't see any obvious issues (at least, I don't see a crash) if the bound buffer is too small.
,
Aug 23 2017
Adding Stability-Crash for issue 756293 which I just duped into this issue.
,
Aug 23 2017
,
Aug 24 2017
,
Aug 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1f98b98ade43dcf76219832e6cf929c3bb0d95a6 commit 1f98b98ade43dcf76219832e6cf929c3bb0d95a6 Author: Kai Ninomiya <kainino@chromium.org> Date: Tue Aug 29 16:49:49 2017 Disable attributes which are enabled, but unconsumed by the program This is take 3 of the patch at http://crrev.com/c/627481 Bug: 756293 , 740278 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: Ib6bdd7c3b04d91d41dd41dea63a07e782cc7e5cb Reviewed-on: https://chromium-review.googlesource.com/636513 Commit-Queue: Kai Ninomiya <kainino@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/heads/master@{#498141} [modify] https://crrev.com/1f98b98ade43dcf76219832e6cf929c3bb0d95a6/gpu/command_buffer/build_gles2_cmd_buffer.py [modify] https://crrev.com/1f98b98ade43dcf76219832e6cf929c3bb0d95a6/gpu/command_buffer/service/context_state.cc [modify] https://crrev.com/1f98b98ade43dcf76219832e6cf929c3bb0d95a6/gpu/command_buffer/service/gles2_cmd_decoder.cc [modify] https://crrev.com/1f98b98ade43dcf76219832e6cf929c3bb0d95a6/gpu/command_buffer/service/gles2_cmd_decoder_unittest_1_autogen.h [modify] https://crrev.com/1f98b98ade43dcf76219832e6cf929c3bb0d95a6/gpu/command_buffer/service/gles2_cmd_decoder_unittest_attribs.cc [modify] https://crrev.com/1f98b98ade43dcf76219832e6cf929c3bb0d95a6/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc [modify] https://crrev.com/1f98b98ade43dcf76219832e6cf929c3bb0d95a6/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h [modify] https://crrev.com/1f98b98ade43dcf76219832e6cf929c3bb0d95a6/gpu/command_buffer/service/vertex_attrib_manager.cc [modify] https://crrev.com/1f98b98ade43dcf76219832e6cf929c3bb0d95a6/gpu/command_buffer/service/vertex_attrib_manager.h
,
Aug 30 2017
ClusterFuzz testcase 5471760211509248 is verified as fixed, so closing issue as verified. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
,
Aug 30 2017
,
Sep 26 2017
,
Dec 6 2017
This bug has been closed for more than 14 weeks. Removing security view restrictions. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by piman@chromium.org
, Jul 7 2017