New issue
Advanced search Search tips

Issue 788949 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Incorrect-function-pointer-type in gl::Debug::insertMessage

Project Member Reported by ClusterFuzz, Nov 28 2017

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=6394303336415232

Fuzzer: libFuzzer_gpu_angle_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Incorrect-function-pointer-type
Crash Address: 
Crash State:
  gl::Debug::insertMessage
  gl::Debug::insertMessage
  gl::Context::handleError
  
Sanitizer: undefined (UBSAN)

Recommended Security Severity: Medium

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6394303336415232

Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information.

Note: This crash might not be reproducible with the provided testcase. That said, for the past 14 days we've been seeing this crash frequently. If you are unable to reproduce this, please try a speculative fix based on the crash stacktrace in the report. The fix can be verified by looking at the crash statistics in the report, a day after the fix is deployed. We will auto-close the bug if the crash is not seen for 14 days.
 
Project Member

Comment 1 by sheriffbot@chromium.org, Nov 28 2017

Labels: Pri-1

Comment 2 by raymes@chromium.org, Nov 28 2017

Cc: piman@chromium.org
Components: Internals>GPU>Internals
Owner: zmo@chromium.org
Status: Assigned (was: Untriaged)
zmo: could you ptal? Thanks!

Comment 3 by raymes@chromium.org, Nov 28 2017

Labels: Security_Impact-Head M-63

Comment 4 by piman@chromium.org, Nov 28 2017

I suspect this is another false positive like  crbug.com/778918  or crbug.com/781168

Comment 5 by piman@chromium.org, Nov 28 2017

Labels: -Type-Bug-Security -Security_Severity-Medium -Security_Impact-Head Type-Bug
mmh, I think there is a mismatch between ANGLE's header (which is used in the ANGLE DLL) and the third_party/mesa headers that is used by chrome GL bindings:

third_party/angle/include/GLES2/gl2ext.h:
typedef void (GL_APIENTRY  *GLDEBUGPROCKHR)(GLenum source,GLenum type,GLuint id,GLenum severity,GLsizei length,const GLchar *message,const void *userParam);

third_party/mesa/src/include/GL/glext.h:
typedef void (APIENTRY *GLDEBUGPROC)(GLenum source,GLenum type,GLuint id,GLenum severity,GLsizei length,const GLchar *message,GLvoid *userParam);


Note the const difference in the 'userParam' param (GLvoid is typedef'ed to void). Looking at https://www.khronos.org/registry/OpenGL/extensions/KHR/KHR_debug.txt it was changed in revision 13, and ANGLE's header is more up-to-date.

We can simply patch mesa's header there, I think.
Removing security bits though:
1- this is not used by default in production (only with a flag, or in the fuzzer)
2- the users don't actually touch the userParam (it'll be null).

Comment 6 by piman@chromium.org, Mar 2 2018

Cc: -piman@chromium.org zmo@chromium.org
Owner: piman@chromium.org
I'll take this to close.
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 2 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/deps/mesa/+/92521a7a302b58aaa242b90192eaddb67ea3d0c2

commit 92521a7a302b58aaa242b90192eaddb67ea3d0c2
Author: Antoine Labour <piman@chromium.org>
Date: Fri Mar 02 21:19:56 2018

Update GLDEBUGPROCARB to the latest spec version

The latest spec version has the userParam argument to be const GLvoid*,
and this is reflected in the ANGLE headers. Update the header to avoid
type mismatch which UBSAN sees as a security issue.

Bug:  788949 
Change-Id: If16c77501c42412106c47c8427bc799d3d7c3db5
[modify] https://crrev.com/92521a7a302b58aaa242b90192eaddb67ea3d0c2/include/GL/glext.h

Project Member

Comment 9 by bugdroid1@chromium.org, Mar 2 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bc02b84f5db51f036a5ee3f5eb1288f1b0dd573c

commit bc02b84f5db51f036a5ee3f5eb1288f1b0dd573c
Author: Antoine Labour <piman@chromium.org>
Date: Fri Mar 02 23:49:00 2018

Roll third_party/mesa ef811c6..92521a7

Includes the following commit:
92521a7 Update GLDEBUGPROCARB to the latest spec version

Bug:  788949 
Change-Id: I89e981718b42e375db7c2d4eb236643631d80c44
Reviewed-on: https://chromium-review.googlesource.com/946903
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540683}
[modify] https://crrev.com/bc02b84f5db51f036a5ee3f5eb1288f1b0dd573c/DEPS
[modify] https://crrev.com/bc02b84f5db51f036a5ee3f5eb1288f1b0dd573c/third_party/mesa/README.chromium

Status: Fixed (was: Assigned)
This looks fixed, although CF doesn't seem to repro the original issue.
Project Member

Comment 11 by sheriffbot@chromium.org, Mar 10 2018

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Project Member

Comment 12 by sheriffbot@chromium.org, Jun 16 2018

Labels: -Restrict-View-SecurityNotify allpublic
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