Coverity found this, CID 373 at http://scan.coverity.com:7454/. This is mostly a theoretical problem: cookperm.txt is only used if hostperm.1 does not exist, which only normally happens with very old profiles (pre 2004 or so). The file also needs to be invalid, and there is no way that I know to make it invalid through a user action, so I don't think this could possibly be exploited. But I had fun figuring this one out anyways. I'll attach an invalid cookperm.txt file that shows the problem. Basically, what ends up happening is the following: AddInternal gets an invalid aTypeIndex (9, with my testcase), so |entry->SetPermission(aTypeIndex, aPermission);| fails (by writing past the end of mPermissions' buffer), which means entry->PermissionsAreEmpty() is then true, which means the engine thinks it's set a permission back to default, so mHostCount is decremented, but the entry is removed with RawRemoveEntry so the hashtable isn't shrunk. Later, in nsPermissionManager::GetEnumerator, the hashtable (mHostTable) is enumerated, so the stale entry is left in hostList, which is then passed to the nsPermissionEnumerator constructor which calls Prefetch which calls GetEntry which crashes when passed a null key. Since mPermissions is only as large as NUMBER_OF_TYPES, I believe the solution is to treat the entry as invalid if type >= NUMBER_OF_TYPES. Steps to reproduce: 1) Save the attached cookperm.txt file to a test profile 2) Delete any existing hostperm.1 files (won't exist for clean profiles) 3) Start Firefox, Tools->Options->Privacy->Cookies->Exceptions 4) Crash!

Comment on attachment 218662 [details] [diff] [review] patch I'm not sure i understand the problem, but the solution sure looks good :) r=mvl

mozilla/extensions/cookie/nsPermissionManager.cpp 1.58