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 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2013
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security



Sign in to add a comment
link

Issue 174895: IndexedDB: missing check that index_ids and index_keys have equal size

Reported by aedla@chromium.org, Feb 7 2013 Project Member

Issue description

Some code:

void IndexedDBDispatcherHost::DatabaseDispatcherHost::OnPut(
  ...
  database->put(host_transaction_id,
                ...
                params.index_ids,
                params.index_keys);

void WebIDBDatabaseImpl::put(...
    ...
    ASSERT(webIndexIds.size() == webIndexKeys.size());
    Vector<int64_t> indexIds(webIndexIds.size());
    Vector<IDBDatabaseBackendInterface::IndexKeys> indexKeys(webIndexKeys.size());

    for (size_t i = 0; i < webIndexIds.size(); ++i) {
        ...
        indexKeys[i] = indexKeyList;

There's no check that index_ids and index_keys arrays have the same size. There's only an ASSERT, which isn't compiled into release builds. When index_ids is larger, indexKeys array will overflow.

REPRODUCTION CASE
This PoC modifies the renderer to send a large index_ids array to the browser. The result is an OOB read, which triggers a CRASH(). With some exploitation, the CRASH() could be avoided though, to achieve memory corruption.

Apply webkit.patch and open indexeddbpoc.html.

Type of crash: browser
Crash State:
(gdb) bt 
#0  0x0000555558548f96 in WebKit::WebIDBDatabaseImpl::put(long long, long long, WebKit::WebVector<unsigned char>*, WebKit::WebIDBKey const&, WebKit::WebIDBDatabase::PutMode, WebKit::WebIDBCallbacks*, WebKit::WebVector<long long> const&, WebKit::WebVector<WebKit::WebVector<WebKit::WebIDBKey> > const&) ()
(gdb) x/1i $rip
=> 0x555558548f96 <_ZN6WebKit18WebIDBDatabaseImpl3putExxPNS_9WebVectorIhEERKNS_9WebIDBKeyENS_14WebIDBDatabase7PutModeEPNS_15WebIDBCallbacksERKNS1_IxEERKNS1_INS1_IS4_EEEE+1350>:	movl   $0x0,(%rax)
(gdb) info reg
rax            0xbbadbeef	3148725999
 

Comment 1 by aedla@chromium.org, Feb 7 2013

webkit.patch
797 bytes View Download
indexeddbpoc.html
1.1 KB View Download

Comment 2 by aedla@chromium.org, Feb 7 2013

Labels: SecImpacts-Beta

Comment 3 by aedla@chromium.org, Feb 8 2013

DatabaseDispatcherHost::OnSetIndexKeys has the same problem.

Comment 4 by infe...@chromium.org, Feb 11 2013

Cc: -dgro...@chromium.org
Labels: Mstone-25
Owner: dgro...@chromium.org
Status: Assigned

Comment 5 by infe...@chromium.org, Feb 11 2013

Labels: WebKit-Storage-IndexedDB

Comment 6 by dgro...@chromium.org, Feb 11 2013

Cc: tony@chromium.org

Comment 7 by dgro...@chromium.org, Feb 12 2013

Status: Fixed
Committed in 182008

Comment 8 by infe...@chromium.org, Feb 12 2013

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify Merge-Approved

Comment 10 by dgro...@chromium.org, Feb 12 2013

Should I merge this to both 25 and 26 or just 25 for some reason?

Comment 11 by bugdroid1@chromium.org, Feb 12 2013

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

------------------------------------------------------------------------
r182008 | dgrogan@chromium.org | 2013-02-12T21:10:36.310993Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/in_process_webkit/indexed_db_dispatcher_host.cc?r1=182008&r2=182007&pathrev=182008

Improve IndexedDB IPC message sanitization

Defend against a compromised renderer sending junk to the browser.

BUG= 174895 


Review URL: https://chromiumcodereview.appspot.com/12208119
------------------------------------------------------------------------

Comment 12 by dgro...@chromium.org, Feb 13 2013

Merged to 25 in
http://trac.webkit.org/changeset/142671
https://src.chromium.org/viewvc/chrome?view=rev&revision=182090

The chromium code had changed a bunch and the patch was altered some.

inferno, please comment on the 25 v 26 question

Comment 13 by bugdroid1@chromium.org, Feb 13 2013

Project Member
Labels: -Merge-Approved merge-merged-1364
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=182090

------------------------------------------------------------------------
r182090 | dgrogan@chromium.org | 2013-02-13T02:12:28.392391Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1364/src/content/browser/in_process_webkit/indexed_db_dispatcher_host.cc?r1=182090&r2=182089&pathrev=182090

Merge 182008
> Improve IndexedDB IPC message sanitization
> 
> Defend against a compromised renderer sending junk to the browser.
> 
> BUG= 174895 
> 
> 
> Review URL: https://chromiumcodereview.appspot.com/12208119

TBR=dgrogan@chromium.org
Review URL: https://codereview.chromium.org/12223123
------------------------------------------------------------------------

Comment 14 by infe...@chromium.org, Feb 13 2013

it needs to be merged to both m25 and m26.

Comment 15 by scarybea...@gmail.com, Feb 13 2013

Cc: kerz@chromium.org
Maybe this is a bit too fresh to have merged. I'll check wit @kerz.
We're building the final beta tonight which may go to stable...

Comment 16 by scarybea...@gmail.com, Feb 13 2013

Labels: -merge-merged-1364 Merge-Approved
Yeah, it's too fresh. I'm reverting. I'll re-merge it for you for M25 patch 1.

Comment 17 by scarybea...@gmail.com, Feb 13 2013

Reverted Chromium piece at r182104. The WebKit piece is a total nop so it can stay.

Comment 18 by bugdroid1@chromium.org, Feb 13 2013

Project Member
Labels: -Merge-Approved merge-merged-1364
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=182104

------------------------------------------------------------------------
r182104 | cevans@chromium.org | 2013-02-13T03:27:32.107220Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1364/src/content/browser/in_process_webkit/indexed_db_dispatcher_host.cc?r1=182104&r2=182103&pathrev=182104

Revert 182090
> Merge 182008
> > Improve IndexedDB IPC message sanitization
> > 
> > Defend against a compromised renderer sending junk to the browser.
> > 
> > BUG= 174895 
> > 
> > 
> > Review URL: https://chromiumcodereview.appspot.com/12208119
> 
> TBR=dgrogan@chromium.org
> Review URL: https://codereview.chromium.org/12223123

TBR=dgrogan@chromium.org
Review URL: https://codereview.chromium.org/12225161
------------------------------------------------------------------------

Comment 19 by scarybea...@gmail.com, Feb 20 2013

Labels: -merge-merged-1364 Merge-Approved

Comment 20 by scarybea...@gmail.com, Feb 20 2013

Comment 21 by scarybea...@gmail.com, Feb 20 2013

M26 Chromium piece @ r183659

Comment 22 by bugdroid1@chromium.org, Feb 20 2013

Project Member
Labels: -Merge-Approved merge-merged-1410
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=183659

------------------------------------------------------------------------
r183659 | cevans@chromium.org | 2013-02-20T23:43:06.317672Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1410/src/content/browser/in_process_webkit/indexed_db_dispatcher_host.cc?r1=183659&r2=183658&pathrev=183659

Merge 182008
> Improve IndexedDB IPC message sanitization
> 
> Defend against a compromised renderer sending junk to the browser.
> 
> BUG= 174895 
> 
> 
> Review URL: https://chromiumcodereview.appspot.com/12208119

TBR=dgrogan@chromium.org
Review URL: https://codereview.chromium.org/12310027
------------------------------------------------------------------------

Comment 23 by scarybea...@gmail.com, Feb 20 2013

Labels: Merge-Merged Release-1
For M25 Chromium piece I reverted my revert :)
@ r183661

Comment 24 by bugdroid1@chromium.org, Feb 20 2013

Project Member
Labels: merge-merged-1364
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=183661

------------------------------------------------------------------------
r183661 | cevans@chromium.org | 2013-02-20T23:45:06.728414Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1364/src/content/browser/in_process_webkit/indexed_db_dispatcher_host.cc?r1=183661&r2=183660&pathrev=183661

Revert 182104
> Revert 182090
> > Merge 182008
> > > Improve IndexedDB IPC message sanitization
> > > 
> > > Defend against a compromised renderer sending junk to the browser.
> > > 
> > > BUG= 174895 
> > > 
> > > 
> > > Review URL: https://chromiumcodereview.appspot.com/12208119
> > 
> > TBR=dgrogan@chromium.org
> > Review URL: https://codereview.chromium.org/12223123
> 
> TBR=dgrogan@chromium.org
> Review URL: https://codereview.chromium.org/12225161

TBR=cevans@chromium.org
Review URL: https://codereview.chromium.org/12317035
------------------------------------------------------------------------

Comment 25 by scarybea...@gmail.com, Mar 2 2013

Labels: CVE-2013-0906

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

Project Member
Labels: -Type-Security -Area-Internals -SecSeverity-High -SecImpacts-Beta -Mstone-25 -WebKit-Storage-IndexedDB Cr-Content-Storage-IndexedDB Security-Impact-Beta Cr-Internals Security-Severity-High Type-Bug-Security M-25

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

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

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

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

Comment 29 by bugdroid1@chromium.org, Apr 5 2013

Project Member
Labels: Cr-Blink

Comment 30 by bugdroid1@chromium.org, Apr 6 2013

Project Member
Labels: -Cr-Content-Storage-IndexedDB Cr-Blink-Storage-IndexedDB

Comment 31 by jsc...@chromium.org, Nov 18 2013

Labels: -Restrict-View-SecurityNotify
Bulk release of old security bug reports.

Comment 32 by sheriffbot@chromium.org, Jun 14 2016

Project Member
Labels: -release-1
This bug is a regression and does not impact stable. Removing incorrectly added Release- labels.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

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

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

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

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

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

Labels: allpublic

Comment 36 by awhalley@chromium.org, Apr 25 2018

Labels: CVE_description-submitted

Sign in to add a comment