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

Issue metadata

Status: Fixed
Owner:
User never visited
Closed: Jun 2010
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug-Security

Restricted
  • Only users with Commit permission may comment.



Sign in to add a comment
link

Issue 43322: [MD audit] Problems with video messages and sizes

Reported by scarybea...@gmail.com, May 5 2010

Issue description

I think I see a bunch of nearby problems, so I'll assign this to myself for 
now...

---
4. UpdateVideo Out of Bounds Memory Access

File: chrome/gpu/gpu_video_layer_glx.cc
Function: GpuVideoLayerGLX::OnPaintToVideoLayer()
Problem Type: Illegal Memory Access (Read)
Compromise Type: Sandbox escalation
Severity: Medium
Message: ViewHostMsg_UpdateVideo

A shared memory buffer specified by the user is mapped in to the GPU 
process and then used as a source buffer to read pixel data from. However, 
the size of 
the shared memory buffer is not verified against the size of the video size 
set up previously with the ViewHostMsg_CreateVideo message. Therefore, it 
is 
possible to read out of bounds memory data in to the video window, leading 
to potential information leaks.
---
 

Comment 1 by cpu@chromium.org, May 5 2010

Also in the nearby line:
// TODO(scherkus): support actual video ids!

Not sure what that means but sounds like any renderer could affect any other renderer

Comment 2 by jsc...@chromium.org, May 25 2010

Labels: -Pri-0 Pri-1
Cris expressed interest in taking a look at this.

Comment 3 by cdn@google.com, May 26 2010

Was there a PoC associated with this? I think I have a patch but trying to figure out 
how to trigger it.

Comment 4 by jsc...@chromium.org, May 26 2010

Comment 5 by jsc...@chromium.org, May 26 2010

Comment 6 by chromium...@gmail.com, May 26 2010

scherkus: Any idea how we could trigger this? The function of interest is 
GpuVideoLayerGLX::OnPaintToVideoLayer(). I am thinking we can just add a check within 
the for loop after the per plane width/heights are set like this

    if ((dib->size() - (planes[kYPlane] - planes[i])) < (plane_width * plane_height)) 
{
      glActiveTexture(GL_TEXTURE0);
      glPixelStorei(GL_UNPACK_ROW_LENGTH, 0);
      glFlush();
      return;
    }

This check can also be done up above but this is nicer if formats are added/changed 
in the future.

Comment 7 by scherkus@chromium.org, May 27 2010

Woah this is my experimental-and-not-really-finished GPU video layering stuff.

Only possible to instantiate + hit this code if:
  a) Running on Linux
  b) --enable-video-layering
  c) --enable-gpu-rendering

Of course I'm not a security guru so maybe it's possible somehow to jump to that 
instruction?

Hope that helps!

Comment 8 by jsc...@chromium.org, May 27 2010

Labels: -Pri-1 Pri-3 SecSeverity-Medium
Dropping the priority and severity ratings since this is currently behind flags.

Comment 9 by chromium...@gmail.com, Jun 1 2010

Status: Started

Comment 10 by bugdro...@gmail.com, Jun 8 2010

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=49132 

------------------------------------------------------------------------
r49132 | scherkus@chromium.org | 2010-06-07 18:01:58 -0700 (Mon, 07 Jun 2010) | 7 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/gpu/gpu_video_layer_glx.cc?r1=49132&r2=49131

Added check for negative height/width and ensure that buffer size has not changed before reading buffers.

Patch by cdn@chromium.org:
http://codereview.chromium.org/2479002/show

BUG= 43322 
TEST=Run with --enable-video-layering and --enable-gpu-rendering and use <video> tag
------------------------------------------------------------------------

Comment 11 by infe...@chromium.org, Jun 8 2010

Status: WillMerge

Comment 12 by infe...@chromium.org, Jun 9 2010

Labels: -SecSeverity-Medium SecSeverity-High

Comment 13 by bugdro...@gmail.com, Jun 11 2010

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=49558 

------------------------------------------------------------------------
r49558 | inferno@chromium.org | 2010-06-11 11:34:08 -0700 (Fri, 11 Jun 2010) | 10 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/375/src/chrome/gpu/gpu_video_layer_glx.cc?r1=49558&r2=49557

Merge 49132 - Added check for negative height/width and ensure that buffer size has not changed before reading buffers.

Patch by cdn@chromium.org:
http://codereview.chromium.org/2479002/show

BUG= 43322 
TEST=Run with --enable-video-layering and --enable-gpu-rendering and use <video> tag

TBR=scherkus@chromium.org
Review URL: http://codereview.chromium.org/2755008
------------------------------------------------------------------------

Comment 14 by infe...@chromium.org, Jun 11 2010

Status: FixUnreleased

Comment 15 by jsc...@chromium.org, Jun 17 2010

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Bulk edit for SecurityNotify Migration.

Comment 16 by jsc...@chromium.org, Mar 21 2011

Labels: Type-Security

Comment 17 by jsc...@chromium.org, Oct 5 2011

Labels: SecImpacts-Stable
Batch update.

Comment 18 by jsc...@chromium.org, Apr 18 2012

Labels: -Restrict-View-SecurityNotify
Lifting view restrictions.

Comment 19 by jsc...@chromium.org, Apr 18 2012

Status: Fixed

Comment 20 by bugdroid1@chromium.org, Oct 13 2012

Project Member
Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.

Comment 21 by bugdroid1@chromium.org, Mar 10 2013

Project Member
Labels: -SecSeverity-High -Type-Security -SecImpacts-Stable Security-Impact-Stable Security-Severity-High Type-Bug-Security

Comment 22 by bugdroid1@chromium.org, Mar 11 2013

Project Member
Labels: -Area-Undefined

Comment 23 by bugdroid1@chromium.org, Mar 21 2013

Project Member
Labels: -Security-Severity-High Security_Severity-High

Comment 24 by bugdroid1@chromium.org, Mar 21 2013

Project Member
Labels: -Security-Impact-Stable Security_Impact-Stable

Comment 25 by mbarbe...@chromium.org, Oct 2 2016

Labels: allpublic

Sign in to add a comment