Closed Bug 1242255 Opened 4 years ago Closed 4 years ago

crash in PLDHashTable::Remove | mozilla::dom::quota::QuotaObject::Release

Categories

(Core :: DOM: IndexedDB, defect, critical)

44 Branch
Unspecified
Windows NT
defect
Not set
critical

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox44 --- wontfix
firefox45 --- unaffected
firefox46 --- unaffected
firefox47 --- unaffected

People

(Reporter: philipp, Unassigned)

References

Details

(Keywords: crash, csectype-uaf, sec-high)

Crash Data

This bug was filed from the Socorro interface and is 
report bp-7012c0a2-79e7-4908-a86e-18bdb2160122.
=============================================================
Crashing Thread (61)
Frame 	Module 	Signature 	Source
0 	xul.dll 	PLDHashTable::Remove(void const*) 	xpcom/glue/PLDHashTable.cpp
1 	xul.dll 	mozilla::dom::quota::QuotaObject::Release() 	dom/quota/QuotaManager.cpp
2 	xul.dll 	`anonymous namespace'::xClose(sqlite3_file*) 	storage/TelemetryVFS.cpp
3 	nss3.dll 	sqlite3OsClose 	db/sqlite3/src/sqlite3.c
4 	nss3.dll 	sqlite3WalClose 	db/sqlite3/src/sqlite3.c
5 	nss3.dll 	sqlite3PagerClose 	db/sqlite3/src/sqlite3.c
6 	nss3.dll 	sqlite3BtreeClose 	db/sqlite3/src/sqlite3.c
7 	nss3.dll 	sqlite3LeaveMutexAndCloseZombie 	db/sqlite3/src/sqlite3.c
8 	nss3.dll 	sqlite3Close 	db/sqlite3/src/sqlite3.c
9 	xul.dll 	mozilla::storage::Connection::internalClose(sqlite3*) 	storage/mozStorageConnection.cpp
10 	xul.dll 	mozilla::storage::Connection::Close() 	storage/mozStorageConnection.cpp
11 	xul.dll 	`mozilla::dom::indexedDB::`anonymous namespace'::QuotaClient::PerformIdleMaintenanceOnDatabaseInternal'::`16'::AutoClose::~AutoClose 	dom/indexeddb/ActorsParent.cpp
12 	xul.dll 	mozilla::dom::indexedDB::`anonymous namespace'::QuotaClient::PerformIdleMaintenanceOnDatabaseInternal(unsigned int, mozilla::dom::indexedDB::A0xc34feeaa::QuotaClient::SingleMaintenanceInfo const&) 	dom/indexeddb/ActorsParent.cpp
13 	xul.dll 	mozilla::dom::indexedDB::`anonymous namespace'::QuotaClient::PerformIdleMaintenanceOnDatabase(unsigned int, nsACString_internal const&, mozilla::dom::indexedDB::A0xc34feeaa::QuotaClient::SingleMaintenanceInfo&&) 	dom/indexeddb/ActorsParent.cpp
14 	xul.dll 	nsRunnableMethodArguments<unsigned int, nsCString, mozilla::dom::indexedDB::`anonymous namespace'::QuotaClient::SingleMaintenanceInfo&&>::apply<mozilla::dom::indexedDB::`anonymous namespace'::QuotaClient, void ( mozilla::dom::indexedDB::A0xc34feeaa::QuotaClient::*)(unsigned int, nsACString_internal const&, mozilla::dom::indexedDB::A0xc34feeaa::QuotaClient::SingleMaintenanceInfo&&)>(mozilla::dom::indexedDB::`anonymous namespace'::QuotaClient*, void ( mozilla::dom::indexedDB::A0xc34feeaa::QuotaClient::*)(unsigned int, nsACString_internal const&, mozilla::dom::indexedDB::A0xc34feeaa::QuotaClient::SingleMaintenanceInfo&&)) 	xpcom/glue/nsThreadUtils.h
15 	xul.dll 	nsRunnableMethodImpl<void ( mozilla::dom::indexedDB::`anonymous namespace'::QuotaClient::*)(unsigned int, nsACString_internal const&, mozilla::dom::indexedDB::A0xc34feeaa::QuotaClient::SingleMaintenanceInfo&&), 1, unsigned int, nsCString, mozilla::dom::indexedDB::A0xc34feeaa::QuotaClient::SingleMaintenanceInfo&&>::Run() 	xpcom/glue/nsThreadUtils.h
16 	xul.dll 	nsThreadPool::Run() 	xpcom/threads/nsThreadPool.cpp
17 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp
18 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/glue/nsThreadUtils.cpp
19 	xul.dll 	mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp
20 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc
21 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc
22 	xul.dll 	nsThread::ThreadFunc(void*) 	xpcom/threads/nsThread.cpp
23 	nss3.dll 	PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c
24 	nss3.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c
25 	msvcr120.dll 	_callthreadstartex 	f:\dd\vctools\crt\crtw32\startup\threadex.c:376
26 	msvcr120.dll 	_threadstartex 	f:\dd\vctools\crt\crtw32\startup\threadex.c:354
27 	kernel32.dll 	BaseThreadInitThunk 	
28 	ntdll.dll 	RtlUserThreadStart 	
29 	kernel32.dll 	BasepReportFault 	
30 	kernel32.dll 	BasepReportFault

this signature started first occurring on 2016-01-12 and is nearly exclusively affecting the 44 beta channel so far. looking at 44rc crash stats, this crash is the #26 with 0.33% of all crashes there at the moment.
since it popped up so recently, maybe there is a third-party website change at the root of this. do submitted urls contain something interesting in this regard?
Flags: needinfo?(kairo)
Flags: needinfo?(jvarga)
(In reply to philipp from comment #1)
> since it popped up so recently, maybe there is a third-party website change
> at the root of this. do submitted urls contain something interesting in this
> regard?

Not really:

19 	https://www.facebook.com/
10 	https://mail.google.com/mail/u/0/#inbox
10 	about:newtab
8 	about:home
5 	about:blank
5 	https://www.facebook.com/?_rdr=p
4 	https://mail.google.com/mail/u/0/?tab=wm#inbox
4 	https://www.google.co.in/?gws_rd=ssl
3 	https://www.youtube.com/
Flags: needinfo?(kairo)
What is concerning here is that many crashes show a "0xffffffffe5e5e5e5" address on 32bit or a rax register of "0xe5e5e5e5e5e5e5e5" on 64bit, which is our poison on free and therefore indicates a UAF issue. I am marking this bug security for now.
Group: core-security
Component: General → DOM: IndexedDB
Group: core-security → dom-core-security
A big change landed in 45 cycle, it was quota manager on PBackground which may have fixed this issue somehow.

Anyway, the crash in QuotaObject::Release() means that we're closing a sqlite connection for a file which was already deleted on disk.
Flags: needinfo?(jvarga)
Jan thinks this may be related to the db idle maintenance and some of the comments in socorro correlate well with that:

" I was not interacting with the browser when it crashed. "

" Was AFK. Came back to this. "
Is there any way to contact the users reporting via socorro?

Since Jan thinks this is fixed in 45, given that there are only 5 weeks left until release it would be good to know if the affected users can reproduce using beta (45).
Flags: needinfo?(kairo)
I'm going to mark this sec-high. If this happens with idle maintenance it seems like it would be hard for a page to exploit.
(In reply to Andrew Overholt [:overholt] from comment #6)
> Is there any way to contact the users reporting via socorro?

We can, but it's not very easy, and it would need someone dedicated to actually handle all the replies we get (which has proven to be the harder task in previous outreach to crashed users), we usually handle that through User Advocacy.

> Since Jan thinks this is fixed in 45, given that there are only 5 weeks left
> until release it would be good to know if the affected users can reproduce
> using beta (45).

That's a good observation, we indeed see this signature in 44 beta versions but not in 45 beta 1 so far. I think that given that we saw this in beta previously, watching the data should be sufficient to confirm that it's fixed in 45.
Flags: needinfo?(kairo)
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #8)
> (In reply to Andrew Overholt [:overholt] from comment #6)
> > Is there any way to contact the users reporting via socorro?
> 
> We can, but it's not very easy, and it would need someone dedicated to
> actually handle all the replies we get (which has proven to be the harder
> task in previous outreach to crashed users), we usually handle that through
> User Advocacy.

OK, sounds like it's not a good course of action. Thanks.

> > Since Jan thinks this is fixed in 45, given that there are only 5 weeks left
> > until release it would be good to know if the affected users can reproduce
> > using beta (45).
> 
> That's a good observation, we indeed see this signature in 44 beta versions
> but not in 45 beta 1 so far. I think that given that we saw this in beta
> previously, watching the data should be sufficient to confirm that it's
> fixed in 45.

Are we ok waiting that long? I don't to see a known crash affecting users sitting for 5 weeks if we can avoid it. Of course, it would be some work to determine what exactly fixed this in 45 and then we'd have to do a point release ...
Flags: needinfo?(kairo)
(In reply to Andrew Overholt [:overholt] from comment #9)
> Are we ok waiting that long? I don't to see a known crash affecting users
> sitting for 5 weeks if we can avoid it. Of course, it would be some work to
> determine what exactly fixed this in 45 and then we'd have to do a point
> release ...

Well, we already know that the signature is not present in 45.0b1 - if we do find what fixed this, then this might contribute to a potential dot-release for 44 if we do one.
It's also possible that this just changed signatures, though.
Flags: needinfo?(kairo)
FWIW still looks like no reports after 44.0.2. 45 releases in 8 days.

Kairo, how long should we wait to close this after 45's release?
Flags: needinfo?(kairo)
Well, the bug status should ideally track nightly and it seems to have been fixed there and up to 45 anyhow, so let's close it (and turn 44 to wontfix).
Status: NEW → RESOLVED
Closed: 4 years ago
Flags: needinfo?(kairo)
Resolution: --- → WORKSFORME
Blocks: 1389561
Group: dom-core-security
You need to log in before you can comment on or make changes to this bug.