[tor-bugs] #16771 [Tor Browser]: TBB crashes on Google Maps when creating markers/clicking

[Tor Bug Tracker & Wiki]

#16771: TBB crashes on Google Maps when creating markers/clicking
-------------------------+-------------------------------------------------
     Reporter:  tom      |      Owner:  arthuredelstein
         Type:  defect   |     Status:  needs_information
     Priority:  major    |  Milestone:
    Component:  Tor      |    Version:
  Browser                |   Keywords:  tbb-crash, tbb-5.0-regression,
   Resolution:           |  TorBrowserTeam201508R
Actual Points:           |  Parent ID:
       Points:           |
-------------------------+-------------------------------------------------

Comment (by arthuredelstein):

 Replying to [comment:16 mikeperry]:
 > It seems that this null check will avoid the crash, but we'll still leak
 the blob URI in this case. This lookup and subsequent remove should be
 succeeding, since the isolation key matches the original host and the blob
 uri domain, right?
 >
 > What I don't understand from my core file is why GetDataInfo() was
 returning null here. It appears to have two entries in the gDataTable
 hashtable still, which I assume are the two blob URIs in mHostObjectURIs
 in the document.
 >
 > I wonder if this also means that blob URIs might not be accessible in
 some cases where they should be, even if the isolation matches? This
 GetDataInfo() lookup is also used during accesses, and if it is returning
 null when it shouldn't, then those objects won't be visible at all, right?

 I tried reverting the blob isolation code, and then I added a patch that
 calls GetDataInfo() inside RemoveDataEntry(), like this:

 {{{
     DataInfo* info = GetDataInfo(aUri);
     MOZ_RELEASE_ASSERT(info);
 }}}

 What I found is that the assert still fails when I run tom's Google Maps
 test. So I think `RemoveDataEntry` is being called multiple times on the
 same blob URL, for some reason, even without our blob isolation patch.
 Perhaps `window.URL.revokeObjectURI(blobURL)` is called first (which calls
 `RemoveDataEntry`), and now we are seeing the results of the `nsDocument`
 destructor calling `RemoveDataEntry` again.

 Here's the full test branch (not for merging):
 https://github.com/arthuredelstein/tor-browser/commits/test_16771

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/16771#comment:18>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online