Crash in [@ mozilla::dom::indexedDB::FileManager::GetDirectory]
Categories
(Core :: Storage: IndexedDB, defect, P1)
Tracking
()
People
(Reporter: sg, Assigned: sg)
References
Details
(4 keywords, Whiteboard: [sec-survey][post-critsmash-triage][adv-main79+r][adv-ESR78.1+r] )
Crash Data
Attachments
(2 files)
|
47 bytes,
text/x-phabricator-request
|
tjr
:
approval-mozilla-beta+
tjr
:
approval-mozilla-esr78+
tjr
:
sec-approval+
|
Details | Review |
|
47 bytes,
text/x-phabricator-request
|
tjr
:
approval-mozilla-beta+
tjr
:
approval-mozilla-esr78+
tjr
:
sec-approval+
|
Details | Review |
This bug is for crash report bp-8447b298-7e9a-4da8-abec-fbb160200616.
Top 10 frames of crashing thread:
0 xul.dll mozilla::dom::indexedDB::FileManager::GetDirectory dom/indexedDB/ActorsParent.cpp:16612
1 xul.dll mozilla::dom::indexedDB::FileInfoT<mozilla::dom::indexedDB::FileManager>::GetFileForFileInfo const dom/indexedDB/FileInfoTImpl.h:139
2 xul.dll static mozilla::dom::indexedDB::`anonymous namespace'::DatabaseOperationBase::GetStructuredCloneReadInfoFromExternalBlob dom/indexedDB/ActorsParent.cpp:19567
3 xul.dll static mozilla::dom::indexedDB::`anonymous namespace'::DatabaseOperationBase::GetStructuredCloneReadInfoFromStatement dom/indexedDB/ActorsParent.cpp:5487
4 xul.dll mozilla::dom::indexedDB::`anonymous namespace'::ObjectStoreGetRequestOp::DoDatabaseWork dom/indexedDB/ActorsParent.cpp:25796
5 xul.dll mozilla::dom::indexedDB::`anonymous namespace'::TransactionDatabaseOperationBase::RunOnConnectionThread dom/indexedDB/ActorsParent.cpp:23047
6 xul.dll mozilla::dom::indexedDB::`anonymous namespace'::TransactionDatabaseOperationBase::Run dom/indexedDB/ActorsParent.cpp:23213
7 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1200
8 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:481
9 xul.dll mozilla::dom::indexedDB::`anonymous namespace'::ConnectionPool::ThreadRunnable::Run dom/indexedDB/ActorsParent.cpp:13130
| Assignee | ||
Updated•5 years ago
|
| Assignee | ||
Comment 1•5 years ago
|
||
Updated•5 years ago
|
| Assignee | ||
Comment 2•5 years ago
|
||
Many of the crash reports come from Facebook Apps, most frequently https://apps.facebook.com/coin-master/
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
| Assignee | ||
Comment 3•5 years ago
|
||
Comment on attachment 9156989 [details]
Bug 1646006 - Check if CreateFileInfo returns nullptr. r=#dom-workers-and-storage,janv
Security Approval Request
- How easily could an exploit be constructed based on the patch?: Not that easy, since it is rather unclear how to provoke a situation where the creation of the FileInfo fails. We don't have any STR this yet.
- Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
- Which older supported branches are affected by this flaw?: 77
- If not all supported branches, which bug introduced the flaw?: None
- Do you have backports for the affected branches?: Yes
- If not, how different, hard to create, and risky will they be?:
- How likely is this patch to cause regressions; how much testing does it need?: Unlikely, it just adds nullptr checks and returns errors in those cases. Even if those errors are not properly handled, this only affects cases that would have crashed anyway at some point.
Comment 4•5 years ago
|
||
If it affects 77, I'm assuming it affects 78 and 79, so I'm going to approve uplift to esr and beta (79), but I'm not explicitly marking it as 'affected' because it's an assumption. I'm wontfixing 77. I'm leaving 78 tracking as '?' although I'm guessing this won't go in a dot release. From a security POV I don't think it needs to.
Comment 5•5 years ago
|
||
Comment on attachment 9156989 [details]
Bug 1646006 - Check if CreateFileInfo returns nullptr. r=#dom-workers-and-storage,janv
sec-approved to land and uplift to 78 and ESR78
Updated•5 years ago
|
Comment 6•5 years ago
|
||
Comment 7•5 years ago
|
||
| uplift | ||
Comment 8•5 years ago
|
||
Comment 9•5 years ago
|
||
As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.
Please visit this google form to reply.
| Assignee | ||
Comment 10•5 years ago
|
||
(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #9)
As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.
Please visit this google form to reply.
Looking at this again to fill out the survey, I no longer think that the patch actually fixes the problem, or at least not all problems with this signature. I guess some cases may have been accesses to a nullptr SafeRefPtr<FileInfo> but the most recent one I looked at, which is https://crash-stats.mozilla.org/report/index/bb717d19-a13c-4378-b158-1e2990200709 looks like a UAF on a SafeRefPtr<FileInfo> (actually this is more consistent with the csectype-uaf classification here, not sure why I did not notice this discrepancy earlier). When looking at unsafe acquistions, this one is striking: https://searchfox.org/mozilla-central/rev/85ae3b911d5fcabd38ef315725df32e25edef83b/dom/indexedDB/FileManagerBase.h#39-45 There already is comment saying that this looks quirky, and indeed it is unclear what would guarantee that the object pointed to by fileInfo is not released between releasing the lock and the subsequent acquisition of a strong reference in the return statement.
I guess a proper solution would be to add a FileInfo::AddRefWithLockHeld method that is called from within the lock-holding block in FileManager::GetFileInfo and from FileInfo::AddRef, instead of AddRefing outside the lock-holding block.
A similar problem seems to exist in CreateFileInfo, but since this creates a new FileInfo object, and no one else has access to it or its id, this is probably safe, but might nonetheless be changed along to make it look less suspicous.
Jan, WDYT?
| Assignee | ||
Comment 12•5 years ago
|
||
| Assignee | ||
Comment 13•5 years ago
|
||
Comment on attachment 9162436 [details]
Bug 1646006 - Merge GetFileInfo and CreateFileInfo and make locking less quirky. r=janv,#dom-workers-and-storage
Security Approval Request
- How easily could an exploit be constructed based on the patch?: Unclear. We don't have STR the concurrent destruction and retrieval of a FileInfo, but it is well possible to enforce that. On the other hand, the patch tries to obscure that it is a fix for a sec-issue by combining it with a refactoring that merges GetFileInfo (which is exposed to the secissue) and CreateFileInfo (which is not exposed to the secissue).
- Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
- Which older supported branches are affected by this flaw?: esr78, 79
- If not all supported branches, which bug introduced the flaw?: None
- Do you have backports for the affected branches?: Yes
- If not, how different, hard to create, and risky will they be?: (I didn't try yet, but the patches should apply cleanly)
- How likely is this patch to cause regressions; how much testing does it need?: Unlikely, I verified it works under normal circumstances, and it essentially just moves the acquisition of the strong reference to happen under the FileManager lock rather than outside.
Updated•5 years ago
|
Comment 14•5 years ago
|
||
Comment on attachment 9162436 [details]
Bug 1646006 - Merge GetFileInfo and CreateFileInfo and make locking less quirky. r=janv,#dom-workers-and-storage
Approved to land and uplift (after conferring with relman)
Comment 15•5 years ago
|
||
Comment 16•5 years ago
|
||
This got backed out:
https://hg.mozilla.org/mozilla-central/rev/f13654aaef85
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=309658258&repo=autoland
[task 2020-07-13T22:41:57.578Z] 22:41:57 INFO - TEST-PASS | dom/indexedDB/test/test_file_delete.html | Correct db ref count
[task 2020-07-13T22:41:57.579Z] 22:41:57 INFO - Buffered messages finished
[task 2020-07-13T22:41:57.579Z] 22:41:57 ERROR - TEST-UNEXPECTED-FAIL | dom/indexedDB/test/test_file_delete.html | application terminated with exit code 1
[task 2020-07-13T22:41:57.579Z] 22:41:57 INFO - runtests.py | Application ran for: 0:00:12.508948
[task 2020-07-13T22:41:57.579Z] 22:41:57 INFO - zombiecheck | Reading PID log: /var/folders/13/549yrsld39sgm33m_440hh6r000017/T/tmpceLhD6pidlog
[task 2020-07-13T22:41:57.579Z] 22:41:57 INFO - ==> process 1304 launched child process 1305
[task 2020-07-13T22:41:57.579Z] 22:41:57 INFO - ==> process 1304 launched child process 1306
[task 2020-07-13T22:41:57.579Z] 22:41:57 INFO - ==> process 1304 launched child process 1307
[task 2020-07-13T22:41:57.579Z] 22:41:57 INFO - ==> process 1304 launched child process 1308
[task 2020-07-13T22:41:57.579Z] 22:41:57 INFO - zombiecheck | Checking for orphan process with PID: 1305
[task 2020-07-13T22:41:57.579Z] 22:41:57 INFO - zombiecheck | Checking for orphan process with PID: 1306
[task 2020-07-13T22:41:57.579Z] 22:41:57 INFO - zombiecheck | Checking for orphan process with PID: 1307
[task 2020-07-13T22:41:57.579Z] 22:41:57 INFO - zombiecheck | Checking for orphan process with PID: 1308
[task 2020-07-13T22:41:57.580Z] 22:41:57 INFO - mozcrash Downloading symbols from: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/DSxKr0G0ThuU4Lcw4kG15g/artifacts/public/build/target.crashreporter-symbols.zip
[task 2020-07-13T22:42:01.619Z] 22:42:01 INFO - mozcrash Copy/paste: /Users/cltbld/tasks/task_1594679569/fetches/minidump_stackwalk/minidump_stackwalk /var/folders/13/549yrsld39sgm33m_440hh6r000017/T/tmpA0zPnS.mozrunner/minidumps/F7D8E436-E4BE-4D9F-AD40-31B8A7C6B072.dmp /var/folders/13/549yrsld39sgm33m_440hh6r000017/T/tmpOQdADK
[task 2020-07-13T22:42:08.261Z] 22:42:08 INFO - mozcrash Saved minidump as /Users/cltbld/tasks/task_1594679569/build/blobber_upload_dir/F7D8E436-E4BE-4D9F-AD40-31B8A7C6B072.dmp
[task 2020-07-13T22:42:08.261Z] 22:42:08 INFO - mozcrash Saved app info as /Users/cltbld/tasks/task_1594679569/build/blobber_upload_dir/F7D8E436-E4BE-4D9F-AD40-31B8A7C6B072.extra
[task 2020-07-13T22:42:08.405Z] 22:42:08 INFO - PROCESS-CRASH | dom/indexedDB/test/test_file_delete.html | application crashed [@ mozilla::dom::indexedDB::FileInfoT<mozilla::dom::indexedDB::FileManager>::LockedAddRef()]
[task 2020-07-13T22:42:08.405Z] 22:42:08 INFO - Crash dump filename: /var/folders/13/549yrsld39sgm33m_440hh6r000017/T/tmpA0zPnS.mozrunner/minidumps/F7D8E436-E4BE-4D9F-AD40-31B8A7C6B072.dmp
[task 2020-07-13T22:42:08.405Z] 22:42:08 INFO - Operating system: Mac OS X
[task 2020-07-13T22:42:08.405Z] 22:42:08 INFO - 10.14.5 18F132
[task 2020-07-13T22:42:08.405Z] 22:42:08 INFO - CPU: amd64
[task 2020-07-13T22:42:08.405Z] 22:42:08 INFO - family 6 model 69 stepping 1
[task 2020-07-13T22:42:08.405Z] 22:42:08 INFO - 4 CPUs
[task 2020-07-13T22:42:08.405Z] 22:42:08 INFO -
[task 2020-07-13T22:42:08.405Z] 22:42:08 INFO - GPU: UNKNOWN
[task 2020-07-13T22:42:08.405Z] 22:42:08 INFO -
[task 2020-07-13T22:42:08.405Z] 22:42:08 INFO - Crash reason: EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - Crash address: 0x8
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - Process uptime: 12 seconds
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO -
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - Thread 27 (crashed)
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - 0 XUL!mozilla::dom::indexedDB::FileInfoT<mozilla::dom::indexedDB::FileManager>::LockedAddRef() [FileInfoTImpl.h:cddd6ccb366e2953a6f7c0ff05f63d30a9a0321a : 113 + 0x0]
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - rax = 0x0000000000000000 rdx = 0x0000000000000002
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - rcx = 0x0000000000000000 rbx = 0x0000000000000000
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - rsi = 0x000000012c6370c0 rdi = 0x0000000000000000
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - rbp = 0x00007000073da6c0 rsp = 0x00007000073da6c0
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - r8 = 0xffffffff00000000 r9 = 0x00007000073da6d0
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - r10 = 0x000000009e3779b8 r11 = 0x0000000110c26680
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - r12 = 0x000000010d581430 r13 = 0x00000000ffffffff
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - r14 = 0x000000012038e450 r15 = 0x0000000000000000
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - rip = 0x0000000110bf81d4
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - Found by: given as instruction pointer in context
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - 1 XUL!mozilla::SafeRefPtr<mozilla::dom::indexedDB::FileInfoT<mozilla::dom::indexedDB::FileManager> > mozilla::dom::indexedDB::FileManagerBase<mozilla::dom::indexedDB::FileManager>::AcquireFileInfo<mozilla::dom::indexedDB::FileManagerBase<mozilla::dom::indexedDB::FileManager>::GetFileInfo(long long) const::'lambda'()>(mozilla::dom::indexedDB::FileManagerBase<mozilla::dom::indexedDB::FileManager>::GetFileInfo(long long) const::'lambda'() const&) const [FileManagerBase.h:cddd6ccb366e2953a6f7c0ff05f63d30a9a0321a : 91 + 0x45]
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - rbp = 0x00007000073da6f0 rsp = 0x00007000073da6d0
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - rip = 0x0000000110ba2224
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - Found by: previous frame's frame pointer
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - 2 XUL!mozilla::detail::RunnableFunction<mozilla::dom::indexedDB::(anonymous namespace)::DispatchAndReturnFileReferences(mozilla::dom::quota::PersistenceType, nsTSubstring<char> const&, nsTSubstring<char16_t> const&, long long, int*, int*, bool*)::$_8>::Run() [nsThreadUtils.h:cddd6ccb366e2953a6f7c0ff05f63d30a9a0321a : 577 + 0x21]
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - rbp = 0x00007000073da740 rsp = 0x00007000073da700
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - rip = 0x0000000110bdeb9c
[task 2020-07-13T22:42:08.406Z] 22:42:08 INFO - Found by: previous frame's frame pointer
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - 3 XUL!nsThread::ProcessNextEvent(bool, bool*) [nsThread.cpp:cddd6ccb366e2953a6f7c0ff05f63d30a9a0321a : 1234 + 0x6]
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - rbp = 0x00007000073dac70 rsp = 0x00007000073da750
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - rip = 0x000000010e4496a4
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - Found by: previous frame's frame pointer
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - 4 XUL!mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) [MessagePump.cpp:cddd6ccb366e2953a6f7c0ff05f63d30a9a0321a : 332 + 0x2b]
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - rbp = 0x00007000073dacd0 rsp = 0x00007000073dac80
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - rip = 0x000000010ea542b2
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - Found by: previous frame's frame pointer
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - 5 XUL!MessageLoop::Run() [message_loop.cc:cddd6ccb366e2953a6f7c0ff05f63d30a9a0321a : 309 + 0xc]
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - rbp = 0x00007000073dad00 rsp = 0x00007000073dace0
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - rip = 0x000000010ea04ea0
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - Found by: previous frame's frame pointer
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - 6 XUL!nsThread::ThreadFunc(void*) [nsThread.cpp:cddd6ccb366e2953a6f7c0ff05f63d30a9a0321a : 447 + 0x8]
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - rbp = 0x00007000073daec0 rsp = 0x00007000073dad10
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - rip = 0x000000010e444bf7
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - Found by: previous frame's frame pointer
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - 7 libnss3.dylib!_pt_root [ptthread.c:cddd6ccb366e2953a6f7c0ff05f63d30a9a0321a : 201 + 0x8]
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - rbp = 0x00007000073daf10 rsp = 0x00007000073daed0
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - rip = 0x000000010bd1fd94
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - Found by: previous frame's frame pointer
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - 8 libsystem_pthread.dylib!_pthread_body + 0x7e
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - rbp = 0x00007000073daf30 rsp = 0x00007000073daf20
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - rip = 0x00007fff6437e2eb
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - Found by: previous frame's frame pointer
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - 9 libsystem_pthread.dylib!_pthread_start + 0x42
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - rbp = 0x00007000073daf50 rsp = 0x00007000073daf40
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - rip = 0x00007fff64381249
[task 2020-07-13T22:42:08.407Z] 22:42:08 INFO - Found by: previous frame's frame pointer
[task 2020-07-13T22:42:08.408Z] 22:42:08 INFO - 10 libsystem_pthread.dylib!thread_start + 0xd
[task 2020-07-13T22:42:08.408Z] 22:42:08 INFO - rbp = 0x00007000073daf78 rsp = 0x00007000073daf60
[task 2020-07-13T22:42:08.408Z] 22:42:08 INFO - rip = 0x00007fff6437d40d
[task 2020-07-13T22:42:08.408Z] 22:42:08 INFO - Found by: previous frame's frame pointer
[task 2020-07-13T22:42:08.408Z] 22:42:08 INFO - 11 libnss3.dylib + 0x11fc60
[task 2020-07-13T22:42:08.408Z] 22:42:08 INFO - rsp = 0x00007000073db090 rip = 0x000000010bd1fc60
[task 2020-07-13T22:42:08.408Z] 22:42:08 INFO - Found by: stack scanning
| Assignee | ||
Comment 17•5 years ago
|
||
(In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #16)
This got backed out:
https://hg.mozilla.org/mozilla-central/rev/f13654aaef85
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=309658258&repo=autoland
I already fixed this issue, and updated the patch.
Comment 18•5 years ago
|
||
Comment 19•5 years ago
|
||
| uplift | ||
Comment 20•5 years ago
|
||
| uplift | ||
Comment 21•5 years ago
|
||
| Assignee | ||
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Description
•