I think we can fix this by ensuring that the cursor removes itself from the transaction when IndexedDBCursor is destructed, rather than when CursorImpl (that we return before constructing) is destructed. See the attached patch.

Deleted: fix.patch 940 bytes

fix.patch 940 bytes Download

jsbell: Would you mind taking a look at this or reassigning?

dmurph@ - please take a look

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d007b8b750851fe1b375c463009ea3b24e5c021d

commit d007b8b750851fe1b375c463009ea3b24e5c021d
Author: Daniel Murphy <dmurph@chromium.org>
Date: Wed Jun 07 01:24:31 2017

[IndexedDB] Fix Cursor UAF

If the connection is closed before we return a cursor, it dies in
IndexedDBCallbacks::IOThreadHelper::SendSuccessCursor. It's deleted on
the correct thread, but we also need to makes sure to remove it from its
transaction.

To make things simpler, we have the cursor remove itself from its
transaction on destruction.

R: pwnall@chromium.org
Bug:  728887 
Change-Id: I8c76e6195c2490137a05213e47c635d12f4d3dd2
Reviewed-on: https://chromium-review.googlesource.com/526284
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#477504}
[modify] https://crrev.com/d007b8b750851fe1b375c463009ea3b24e5c021d/content/browser/indexed_db/cursor_impl.cc
[modify] https://crrev.com/d007b8b750851fe1b375c463009ea3b24e5c021d/content/browser/indexed_db/indexed_db_cursor.cc
[modify] https://crrev.com/d007b8b750851fe1b375c463009ea3b24e5c021d/content/browser/indexed_db/indexed_db_cursor.h

Your change meets the bar and is auto-approved for M60. Please go ahead and merge the CL to branch 3112 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

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

This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

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

Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect.

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

+awhalley@ for security review. 

Thanks for confirming - can you please confirm if both changes are safe merges, and if they are tested/verified in canary?

I strongly believe the change is safe - it should be in canary today. I think it is safe to merge.

Per discussion with awhalley@, since this is not baked in canary, dev, or beta yet - we will not consider this for Monday's M59 respin. Per the usual process, merges should be baked in beta prior to coming to stable.

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/51bbaf611b628c9396f573435bb0a9289434923f

commit 51bbaf611b628c9396f573435bb0a9289434923f
Author: Daniel Murphy <dmurph@chromium.org>
Date: Mon Jun 12 18:40:02 2017

[IndexedDB] Fix Cursor UAF

If the connection is closed before we return a cursor, it dies in
IndexedDBCallbacks::IOThreadHelper::SendSuccessCursor. It's deleted on
the correct thread, but we also need to makes sure to remove it from its
transaction.

To make things simpler, we have the cursor remove itself from its
transaction on destruction.

TBR=dmurph@chromium.org

(cherry picked from commit d007b8b750851fe1b375c463009ea3b24e5c021d)

R: pwnall@chromium.org
Bug:  728887 
Change-Id: I8c76e6195c2490137a05213e47c635d12f4d3dd2
Reviewed-on: https://chromium-review.googlesource.com/526284
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#477504}
Reviewed-on: https://chromium-review.googlesource.com/531774
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/branch-heads/3112@{#308}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}
[modify] https://crrev.com/51bbaf611b628c9396f573435bb0a9289434923f/content/browser/indexed_db/cursor_impl.cc
[modify] https://crrev.com/51bbaf611b628c9396f573435bb0a9289434923f/content/browser/indexed_db/indexed_db_cursor.cc
[modify] https://crrev.com/51bbaf611b628c9396f573435bb0a9289434923f/content/browser/indexed_db/indexed_db_cursor.h

Congratulations nedwilliamson@! The VRP panel decided to award $1,000 for this report :-)

Thanks! Though I thought because this was a use-after-free in the browser process it would be considered a sandbox escape. I was able to reliably crash the browser in my testing but didn't get register control yet (the object is smaller so spraying on the IDB thread isn't as reliable as my other bug, at least using the same spray technique). If there's something I can do to improve the report please let me know!

Hi nedwilliamson@ - I'm terribly sorry, we made a mistake and the reward was too low by a decimal order of magnitude!  Thank you for raising the fact you thought it was a bit low :-D  I've managed to get the value changed with our finance folk so it shouldn't delay payment.

awhalley@: Wow, thanks! I just wanted to make sure I wasn't missing any useful information in the report. Thanks again for the generous reward! :-)

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