My favorites | Sign in
Logo
Project Home
  
Downloads
  
Wiki
  
Issues
    
New issue | Search
for
| Advanced search | Search tips
Issue 10869: Buffer overflow in browser process while de-serializing SkBitmap (heap overwrite)
1 person starred this issue and may be notified of changes. Back to list
 
Reported by cpu@chromium.org, Apr 22, 2009 
The function 
bool ParamTraits<SkBitmap>::Read(const Message* m, void** iter, SkBitmap* has a buffer overflow when it calls 

bmp_data->InitSkBitmapFromData(r, variable_data, variable_data_size);

Because |bmp_data| and |variable_data| need to be consistent with each 
other but nobody checks that is so.

In other words, we have: 
   memcpy(bitmap->getPixels(), pixels, total_pixels);

and |pixels| and |total_pixels| are attacker controlled.
Comment 1 by abarth@chromium.org, Apr 22, 2009 
This is a render->browser escalation, right?  In other words, ParamTraits<SkBitmap>::Read isn't exposed directly to web content.
Cc: aba...@chromium.org
Comment 2 by abarth@chromium.org, Apr 22, 2009 
Assigned for investigation.
Status: Assigned
Owner: aba...@chromium.org
Labels: -Area-Misc Area-BrowserBackend
Comment 3 by abarth@chromium.org, Apr 22, 2009 
(No comment was entered for this change.)
Cc: c...@chromium.org secur...@chromium.org
Comment 4 by abarth@chromium.org, Apr 22, 2009 
http://codereview.chromium.org/92064
Comment 5 by cpu@chromium.org, Apr 23, 2009 
@1 : Yes, I think you are correct.
Comment 6 by bugdroid1@chromium.org, Apr 23, 2009 
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=14398 

------------------------------------------------------------------------
r14398 | abarth@chromium.org | 2009-04-23 17:31:12 -0700 (Thu, 23 Apr 2009) | 7 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/ipc_message_unittest.cc?r1=14398&r2=14397
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/ipc_message_utils.cc?r1=14398&r2=14397

Check the size of bitmaps coming over IPC.

R=cpu
BUG=10869
TEST=IPCMessageTest.Bitmap

Review URL: http://codereview.chromium.org/92064
------------------------------------------------------------------------
Comment 7 by abarth@chromium.org, Apr 23, 2009 
I reverted the above because the mac build was messed up.  Real fix landed in 14408.
Comment 8 by abarth@chromium.org, Apr 23, 2009 
We probably want this fix on all branches.
Status: FixUnreleased
Owner: m...@chromium.org
Comment 9 by bugdroid1@chromium.org, Apr 23, 2009 
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=14416 

------------------------------------------------------------------------
r14416 | mal@chromium.org | 2009-04-23 22:56:26 -0700 (Thu, 23 Apr 2009) | 7 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/release_154.next/src/chrome/common/ipc_message_unittest.cc?r1=14416&r2=14415
   M http://src.chromium.org/viewvc/chrome/branches/release_154.next/src/chrome/common/ipc_message_utils.cc?r1=14416&r2=14415

Merge r14408.

Fixes an IPC problem.

BUG=10869
TBR= abarth, cpu
Review URL: http://codereview.chromium.org/94006
------------------------------------------------------------------------
Comment 10 by abarth@chromium.org, Apr 23, 2009 
Depending how fastidious you want to be about the release branch, you could consider 
moving http://src.chromium.org/viewvc/chrome?view=rev&revision=14413 as well.  That 
patch affects only the unit test.
Comment 11 by bugdroid1@chromium.org, Apr 23, 2009 
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=14418 

------------------------------------------------------------------------
r14418 | mal@chromium.org | 2009-04-23 23:46:37 -0700 (Thu, 23 Apr 2009) | 7 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/release_154.next/src/chrome/common/ipc_message_unittest.cc?r1=14418&r2=14417

Merge r14413.

Another unit test change for the IPC fix.

BUG= 10869
TBR= abarth
Review URL: http://codereview.chromium.org/92118
------------------------------------------------------------------------
Comment 12 by bugdroid1@chromium.org, Apr 25, 2009 
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=14554 

------------------------------------------------------------------------
r14554 | mal@chromium.org | 2009-04-25 18:02:08 -0700 (Sat, 25 Apr 2009) | 7 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/172/src/chrome/common/ipc_message_unittest.cc?r1=14554&r2=14553
   M http://src.chromium.org/viewvc/chrome/branches/172/src/chrome/common/ipc_message_utils.cc?r1=14554&r2=14553

Merge r14408 and r14413.

Fixes an IPC problem.

BUG= 10869
TBR= abarth,cpu
Review URL: http://codereview.chromium.org/99033
------------------------------------------------------------------------
Comment 13 by mal.chromium, Apr 27, 2009 
Any thoughts on how QA can verify this fix on a release branch?
Comment 14 by abarth@chromium.org, Apr 27, 2009 
Hum...  That's a good question.  I don't think there is any input you can feed to 
Chrome to trigger this condition (you'd basically have to find another exploit to get 
here).  Theoretically, we could write a QA tool to talk directly to the IPC channel, 
but that would take some amount of time.  Carlos, do you have any ideas?  Maybe 
something based on our IPC fuzzing tests?
Comment 15 by mal.chromium, Apr 27, 2009 
I think we'll just depend on the unit test. I forgot that we included one for this bug 
when I was writing the test notes for QA.
Comment 16 by mal.chromium, May 05, 2009 
154.64 has been released.
Status: Fixed
Labels: -private
Sign in to add a comment