[tor-bugs] #6539 [Firefox Patch Issues]: Image cache isolation causes assert crash in debug builds (and other cases?)

[Tor Bug Tracker & Wiki]

#6539: Image cache isolation causes assert crash in debug builds (and other
cases?)
----------------------------------------------+-----------------------------
 Reporter:  mikeperry                         |          Owner:  mikeperry
     Type:  defect                            |         Status:  new      
 Priority:  major                             |      Milestone:           
Component:  Firefox Patch Issues              |        Version:           
 Keywords:  MikePerry201211d tbb-linkability  |         Parent:  #6528    
   Points:                                    |   Actualpoints:  3        
----------------------------------------------+-----------------------------

Comment(by mikeperry):

 Replying to [comment:14 mcs]:
 > We fixed several issues but some remain.  Here is what we fixed:
 >
 > (1) Added NS_ADDREF() in imgLoader::FindEntryProperties() (accidentally
 removed?)

 Possibly. This addref confused me. It appeared to me to be a reference
 leak because all callers of FindEntryProperties also perform an addref. In
 fact, I still don't understand how it's not a leak. Can you explain?

 Though I suppose even if it is a leak, we should leave it in and simply
 report it to Mozilla.

 > (2) Fixed imgLoader::SetHasProxies() to use the new key format.
 > (3) Modified imgLoader::GetCacheKey() to not use the new partitioned key
 format for chrome:// images.
 > (4) Added some assertions and NULL checks for safety.
 > (5) Renamed some parameters for clarity (e.g., key -> imgURI).
 >
 > I will attach a patch that shows the above changes.  Stability is much
 improved.
 >
 > Change (3) is necessary because the firstPartyURI is NULL when loading
 images referenced by some internal pages such as the bookmarks or history
 sidebars.  Unfortunately, we still see NULL firstPartyURI values for some
 internal image loads such as:
 >    moz-anno:favicon:http://www.mozilla.org/media/img/firefox/favicon.png
 > (when referenced by the history sidebar).
 >
 > So -- is the intent that firstPartyURI will always be non-NULL inside
 the image cache?  If so, we need to determine why
 ThirdPartyUtil::GetFirstPartyURI() fails in some cases.

 Yes. Can we log such cases to the Error Console? I think normal
 PR_LOG/stdout logs are unlikely to be seen.

 I think I am not concerned about the favicon case, as it does not seem
 like it should allow third party tracking, but if there are other weird
 cases that could be problematic.

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