New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 351852 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Mar 2014
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Security

Blocking:
issue 352492



Sign in to add a comment

AsyncPixelTransfersCompletedQuery does not validate shared memory offset

Reported by 70696e6b...@gmail.com, Mar 12 2014

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_2) AppleWebKit/537.74.9 (KHTML, like Gecko) Version/7.0.2 Safari/537.74.9
Platform: 5116.113.0 (Official Build) stable-channel daisy_spring

Steps to reproduce the problem:
1. Do a glBeginQueryEXT/glEndQueryEXT pair with GL_ASYNC_PIXEL_UNPACK_COMPLETED_CHROMIUM and a large sync_shm_offset.

Note that this can be done directly from the web via PNaCl.

What is the expected behavior?
Failure

What went wrong?
GPU process crashes

Did this work before? N/A 

Chrome version: <Copy from: 'about:version'>  Channel: stable
OS Version: 33.0.1750.149
Flash Version: Shockwave Flash 12.0 r0
 
dydi.tar.bz2
5.5 KB Download
Cc: piman@chromium.org
Labels: Cr-Internals-GPU
Cc: siev...@chromium.org reve...@chromium.org
Labels: -Pri-2 Pri-1 Security_Impact-Stable M-33
CC'ing more people.

Comment 3 by jln@chromium.org, Mar 12 2014

Cc: jln@chromium.org

Comment 4 by jln@chromium.org, Mar 12 2014

Cc: mseaborn@chromium.org

Comment 5 by jsc...@chromium.org, Mar 12 2014

Owner: jbau...@chromium.org
Status: Assigned
@jbauman - This is from the Pwnium exploit.

Comment 6 by piman@chromium.org, Mar 12 2014

Cc: epenner@chromium.org
+epenner
I thought you said it was validated somewhere.
It looks like AsyncPixelTransfersCompletedQuery::Process validates this, but not the AsyncPixelTransferCompletionObserverImpl case (in AsyncPixelTransfersCompletedQuery::End). We can probably just do a GetSharedMemoryAs query in AsyncPixelTransfersCompletedQuery::End to ensure it's correct.

Comment 8 by piman@chromium.org, Mar 12 2014

Because of how it's setup, the best is probably to validate explicitly in AsyncPixelTransfersCompletedQuery::End. I have https://codereview.chromium.org/198253002 that does this.
Cc: jbau...@chromium.org
Owner: piman@chromium.org
They should all be validated in the command processing using GetSharedMemoryAs. I did this personally for the AsyncUploadFunctions in the validation.  I didn't implement the query but it should be the same.
Project Member

Comment 11 by bugdroid1@chromium.org, Mar 13 2014

------------------------------------------------------------------------
r256723 | piman@chromium.org | 2014-03-13T01:08:56.014814Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/gpu/command_buffer/service/query_manager.cc?r1=256723&r2=256722&pathrev=256723

Add bounds validation to AsyncPixelTransfersCompletedQuery::End

BUG= 351852 
R=jbauman@chromium.org, jorgelo@chromium.org

Review URL: https://codereview.chromium.org/198253002
------------------------------------------------------------------------
Cc: -piman@chromium.org
Labels: M-34 Merge-Requested
Status: Fixed
Project Member

Comment 13 by bugdroid1@chromium.org, Mar 13 2014

------------------------------------------------------------------
r256918 | piman@chromium.org | 2014-03-13T20:35:22.309179Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc?r1=256918&r2=256917&pathrev=256918
   M http://src.chromium.org/viewvc/chrome/trunk/src/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h?r1=256918&r2=256917&pathrev=256918
   M http://src.chromium.org/viewvc/chrome/trunk/src/gpu/command_buffer/service/query_manager.cc?r1=256918&r2=256917&pathrev=256918
   M http://src.chromium.org/viewvc/chrome/trunk/src/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc?r1=256918&r2=256917&pathrev=256918

Use CheckedNumerics in AsyncPixelTransfersCompletedQuery, add tests

This is a follow-up to https://codereview.chromium.org/198253002/
1- It converts the code to use CheckedNumerics
2- It adds unit tests:
  * converts existing tests to exercise all query types
  * adds an additional test to exercise overflow explicitly
3- Fixes a minor issue with GL_ASYNC_PIXEL_PACK_COMPLETED_CHROMIUM, where
out-of-bounds access would be silently failing instead of losing the context.

BUG= 351852 

Review URL: https://codereview.chromium.org/196633009
-----------------------------------------------------------------
Labels: CVE-2014-1711
Project Member

Comment 15 by bugdroid1@chromium.org, Mar 14 2014

Labels: merge-merged-1750
------------------------------------------------------------------
r256970 | piman@chromium.org | 2014-03-14T01:17:46.257206Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1750/src/gpu/command_buffer/service/query_manager.cc?r1=256970&r2=256969&pathrev=256970

Merge 256723 "Add bounds validation to AsyncPixelTransfersComple..."

> Add bounds validation to AsyncPixelTransfersCompletedQuery::End
> 
> BUG= 351852 
> R=jbauman@chromium.org, jorgelo@chromium.org
> 
> Review URL: https://codereview.chromium.org/198253002

TBR=piman@chromium.org

Review URL: https://codereview.chromium.org/197463012
-----------------------------------------------------------------
Project Member

Comment 16 by bugdroid1@chromium.org, Mar 14 2014

Labels: merge-merged-1750_149
------------------------------------------------------------------
r256971 | piman@chromium.org | 2014-03-14T01:18:31.142977Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1750_149/src/gpu/command_buffer/service/query_manager.cc?r1=256971&r2=256970&pathrev=256971

Merge 256723 "Add bounds validation to AsyncPixelTransfersComple..."

> Add bounds validation to AsyncPixelTransfersCompletedQuery::End
> 
> BUG= 351852 
> R=jbauman@chromium.org, jorgelo@chromium.org
> 
> Review URL: https://codereview.chromium.org/198253002

TBR=piman@chromium.org

Review URL: https://codereview.chromium.org/199643004
-----------------------------------------------------------------
Cc: dxie@chromium.org

Comment 18 by k...@google.com, Mar 14 2014

Cc: kamakshi@chromium.org
Labels: -Merge-Requested Merge-Merged Release-3-M33
merged to m34 in r256989
Project Member

Comment 20 by bugdroid1@chromium.org, Mar 14 2014

Labels: merge-merged-1847
------------------------------------------------------------------
r256989 | inferno@chromium.org | 2014-03-14T04:19:19.878128Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1847/src/gpu/command_buffer/service/query_manager.cc?r1=256989&r2=256988&pathrev=256989

Merge 256970 "Merge 256723 "Add bounds validation to AsyncPixelT..."

> Merge 256723 "Add bounds validation to AsyncPixelTransfersComple..."
> 
> > Add bounds validation to AsyncPixelTransfersCompletedQuery::End
> > 
> > BUG= 351852 
> > R=jbauman@chromium.org, jorgelo@chromium.org
> > 
> > Review URL: https://codereview.chromium.org/198253002
> 
> TBR=piman@chromium.org
> 
> Review URL: https://codereview.chromium.org/197463012

TBR=piman@chromium.org

Review URL: https://codereview.chromium.org/199923002
-----------------------------------------------------------------
Labels: Security_Severity-High
Code exec inside the sandbox.
Labels: Security_Impact-Beta
Blocking: chromium:352492
Labels: -CVE-2014-1711 CVE-2014-1710
Labels: reward-topanel
Labels: -reward-topanel
Labels: reward-ineligible
Reward for this bug is part of the larger Pwnium payment. Marking this particular issue as reward-ineligible and will pay via the master tracking bug.
Reward paid via Issue 352492. If you don't have it in 30 days, please contact me directly.
Project Member

Comment 29 by ClusterFuzz, Jun 19 2014

Labels: -Restrict-View-SecurityTeam
Bulk update: removing view restriction from closed bugs.

Comment 30 by krisr@chromium.org, Jun 24 2014

Labels: VerifyIn-37
Status: Verified
Project Member

Comment 32 by sheriffbot@chromium.org, Mar 22 2016

Labels: -security_impact-beta
Project Member

Comment 33 by sheriffbot@chromium.org, Oct 1 2016

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
Project Member

Comment 34 by sheriffbot@chromium.org, Oct 1 2016

Labels: Restrict-View-SecurityNotify
Project Member

Comment 35 by sheriffbot@chromium.org, Oct 2 2016

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

Sign in to add a comment