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

Issue 31307 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Email to this user bounced
Closed: Dec 2009
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Security
M-4

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment

[MD audit] [RPC] More errors deserializing SkBitmaps!!

Reported by scarybea...@gmail.com, Dec 30 2009

Issue description

From Mark:

---
1. Bitmap Row size Desynchronization

File: chrome/common/common_param_traits.cc
Function: ParamTraits<SkBitmap>::Read()
Problem Type: Logic
Compromise Type: Local Escalation
Severity: Medium

ParamTraits<SkBitmap>::Read() allows a largely unchecked SkBitmap_Data 
structure to be used to create an SkBitmap. While the allocPixels() 
function protects 
from integer overflows, it does so by checking that fHeight * fRowBytes do 
not overflow. However, fWidth can be desynchronized from fRowBytes, which 
seems to 
create problems in the privileged process. For example, when the SkBitmap 
is stored in a Thumbnail store (via the ViewHostMsg_Thumbnail message), the 
JPEGCodec encodes the data from the SkBitmap, which does calculations based 
on the images fWidth value, not fRowBytes.

2. Bitmap Information Leak

File: chrome/common/common_param_traits.cc
Function: ParamTraits<SkBitmap>::Read()
Problem Type: Information Leak
Compromise Type: Information Leak
Severity: Medium

ParamTraits<SkBitmap>::Read() seems to allow you to allocate an SkBitmap 
without supplying enough data to fill it. The data buffer it is copied it 
into is 
not zero'd out and so if a small amount of variable data is supplied, the 
remainder of the buffer will contain random data from the process. I am not 
certain 
if this data can be read back from the child host, but I suspect it can 
from a thumbnail database (or possibly by placing it on a HTML canvas).
---
 
Status: FixUnreleased
Fixed in r35371. I need to merge this, but I'm going to wait until it's cycled through 
the bots a bit.
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=35371 

------------------------------------------------------------------------
r35371 | cevans@chromium.org | 2009-12-29 21:27:59 -0800 (Tue, 29 Dec 2009) | 7 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/common_param_traits.cc?r1=35371&r2=35370

Fix up rowbytes vs. width desynchronization, and fix failure to initialize entire bitmap memory. To fix up the rowbytes value properly, we simply don't send it via IPC any more, and recalulate it from width and depth in the trusted code. It's a cheap calculation.
Also one bonus fix: don't use an unintialized IconInfo if deserialization fails.

BUG= 31307 
TEST=Manual; ran with breakpoints on the failure paths.

Review URL: http://codereview.chromium.org/517023
------------------------------------------------------------------------

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

------------------------------------------------------------------------
r35476 | cevans@chromium.org | 2010-01-04 12:28:39 -0800 (Mon, 04 Jan 2010) | 10 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/249/src/chrome/common/common_param_traits.cc?r1=35476&r2=35475

Merge 35371 - Fix up rowbytes vs. width desynchronization, and fix failure to initialize entire bitmap memory. To fix up the rowbytes value properly, we simply don't send it via IPC any more, and recalulate it from width and depth in the trusted code. It's a cheap calculation.
Also one bonus fix: don't use an unintialized IconInfo if deserialization fails.

BUG= 31307 
TEST=Manual; ran with breakpoints on the failure paths.

Review URL: http://codereview.chromium.org/517023

TBR=jschuh@chromium.org
Review URL: http://codereview.chromium.org/518030
------------------------------------------------------------------------

Success on the bots. Merged to Beta with r35476.
Labels: -Restrict-View-SecurityTeam
Status: Fixed
Fixed in 4.0.249.78... releasing.

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

Labels: Type-Security
Labels: SecImpacts-Stable
Batch update.
Project Member

Comment 8 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 9 by bugdroid1@chromium.org, Mar 10 2013

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

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

Labels: -Area-Undefined
Project Member

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

Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Project Member

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

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

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

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

Comment 14 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 15 by sheriffbot@chromium.org, Oct 1 2016

Labels: Restrict-View-SecurityNotify
Project Member

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

Comment 18 by sheriffbot@chromium.org, Jul 29

Labels: -Pri-0 Pri-1

Sign in to add a comment