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

Issue 45267 link

Starred by 1 user

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

ViewHostMsg_UpdateVideo memory corruption

Reported by chromium...@gmail.com, May 27 2010

Issue description

I am guessing this is similar to 
http://code.google.com/p/chromium/issues/detail?id=43322 and this is just 
not completed code and behind flags but I noticed this while looking into 
that issue and figured I would start a bug for it just in case. 

RenderWidgetHost::OnMsgCreateVideo() called to create video layer of given 
size
RenderWidgetHostViewGtk::AllocVideoLayer() when enable_gpu_rendering_ is 
false new VideoLayerX is created which defines 

scoped_array<uint8> rgb_frame_;

when VideoLayerX::CopyTransportDIB() is called it does 
// Lazy allocate |rgb_frame_|.
  if (!rgb_frame_.get()) {
    // TODO(scherkus): handle changing dimensions and re-allocating.
    CHECK(size() == rgb_rect_.size());

    rgb_frame_.reset(new uint8[rgb_rect_.width() * rgb_rect_.height() * 
4]);
  }

so if this can be called twice with different rgb_rect_ sizes we can get 
this allocated small on the first pass then on the second call we can 
clobber it with a very large buffer

ViewHostMsg_UpdateVideo messages from renderer are the vector... This is 
probably possible from javascript but definately possible from renderer. 
 

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

+scherkus

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

This was found via manual audit so I don't have a file to trigger it.

scherkus: any chance this would be an easy thing to get / walk me through?
Prereqs:
  a) Running on Linux
  b) --enable-video-layering
  c) --enable-gpu-rendering

Then open a page with <video> on it.  The code is super experimental/crashy.
Labels: -Pri-0 Pri-3


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

------------------------------------------------------------------------
r49131 | scherkus@chromium.org | 2010-06-07 17:56:37 -0700 (Mon, 07 Jun 2010) | 8 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/backing_store_x.cc?r1=49131&r2=49130
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/video_layer_x.cc?r1=49131&r2=49130
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/video_layer_x.h?r1=49131&r2=49130

Add rgb_frame size tracking and resizing to fix security issue with changing sizes.
Added negative checks on signed heights and widths, added negative check for signed heights and widths in backing store for video layering.

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

BUG= 45267 
TEST=Run on linux with --enable-video-layering and use <video> tag
------------------------------------------------------------------------

Status: WillMerge
Labels: SecSeverity-High
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=49559 

------------------------------------------------------------------------
r49559 | inferno@chromium.org | 2010-06-11 11:36:07 -0700 (Fri, 11 Jun 2010) | 11 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/375/src/chrome/browser/renderer_host/backing_store_x.cc?r1=49559&r2=49558
   M http://src.chromium.org/viewvc/chrome/branches/375/src/chrome/browser/renderer_host/video_layer_x.cc?r1=49559&r2=49558
   M http://src.chromium.org/viewvc/chrome/branches/375/src/chrome/browser/renderer_host/video_layer_x.h?r1=49559&r2=49558

Merge 49131 - Add rgb_frame size tracking and resizing to fix security issue with changing sizes.
Added negative checks on signed heights and widths, added negative check for signed heights and widths in backing store for video layering.

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

BUG= 45267 
TEST=Run on linux with --enable-video-layering and use <video> tag

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

Status: FixUnreleased
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Bulk edit for SecurityNotify Migration.
Labels: Type-Security
Labels: SecImpacts-Stable
Batch update.
Labels: -Restrict-View-SecurityNotify
Lifting view restrictions.
Status: Fixed
Project Member

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

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

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

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

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

Labels: -Area-Undefined
Project Member

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

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

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

Labels: -Security-Impact-Stable Security_Impact-Stable
Labels: reward-topanel
Labels: allpublic

Sign in to add a comment