Closed Bug 1166742 Opened 5 years ago Closed 5 years ago

Nightly crashes in RefPtr coming from SimpleEnumerator::GetNext via dom::HTMLInputElement::nsFilePickerShownCallback::Done, often with 0x5a5a... addresses

Categories

(Core :: DOM: Core & HTML, defect, critical)

Unspecified
Windows NT
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla41
Tracking Status
firefox38 --- unaffected
firefox38.0.5 --- unaffected
firefox39 --- unaffected
firefox40 --- unaffected
firefox41 + verified
firefox-esr31 --- unaffected
firefox-esr38 --- unaffected
b2g-v2.0 --- unaffected
b2g-v2.0M --- unaffected
b2g-v2.1 --- unaffected
b2g-v2.1S --- unaffected
b2g-v2.2 --- unaffected
b2g-master --- fixed

People

(Reporter: kairo, Assigned: baku)

References

Details

(5 keywords)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-33e6d02a-81f5-465d-8c97-a68d12150519.
=============================================================

The two entries in the Crash Signature field (click those links for more stats and reports) are new with the 2015-05-19 Nightly build, both have similar stacks, one is 32bit and one 64bit Windows (in total, all Windows versions are affected).

The crash ID mentioned above is the 64bit version and has the following top stack frames:
0 	xul.dll 	nsHtml5RefPtr<nsHtml5StreamParser>::nsHtml5RefPtr<nsHtml5StreamParser>(nsHtml5StreamParser*) 	xpcom/glue/nsCOMPtr.h
1 	xul.dll 	`anonymous namespace'::SimpleEnumerator::GetNext(nsISupports**) 	widget/nsFilePickerProxy.cpp
2 	xul.dll 	mozilla::dom::HTMLInputElement::nsFilePickerShownCallback::Done(short) 	dom/html/HTMLInputElement.cpp
3 	xul.dll 	nsFilePickerProxy::Recv__delete__(mozilla::dom::MaybeInputFiles const&, short const&) 	widget/nsFilePickerProxy.cpp
4 	xul.dll 	mozilla::dom::PFilePickerChild::OnMessageReceived(IPC::Message const&) 	obj-firefox/ipc/ipdl/PFilePickerChild.cpp
5 	xul.dll 	mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) 	obj-firefox/ipc/ipdl/PContentChild.cpp
6 	xul.dll 	mozilla::ipc::MessageChannel::OnMaybeDequeueOne() 	ipc/glue/MessageChannel.cpp
7 	xul.dll 	RunnableMethod<mozilla::ipc::MessageChannel, bool ( mozilla::ipc::MessageChannel::*)(void), Tuple0>::Run() 	ipc/chromium/src/base/task.h
8 	xul.dll 	MessageLoop::DoWork() 	ipc/chromium/src/base/message_loop.cc
[...]

The address is probably bogus as it's 0xffffffffffffffff which usually just means it's a >32bit address, the edx register has "0x5a5a5a5a5a5a5a5a", which is the poison-on-free value.

A 32bit example is bp-71094f46-ba92-4607-8572-d5eac2150519 which has the following top frames:
0 	xul.dll 	nsRefPtr<nsTextNode>::nsRefPtr<nsTextNode>(nsTextNode*) 	xpcom/base/nsRefPtr.h
1 	xul.dll 	`anonymous namespace'::SimpleEnumerator::GetNext(nsISupports**) 	widget/nsFilePickerProxy.cpp
2 	xul.dll 	mozilla::dom::HTMLInputElement::nsFilePickerShownCallback::Done(short) 	dom/html/HTMLInputElement.cpp
3 	xul.dll 	nsFilePickerProxy::Recv__delete__(mozilla::dom::MaybeInputFiles const&, short const&) 	widget/nsFilePickerProxy.cpp
4 	xul.dll 	mozilla::dom::PFilePickerChild::OnMessageReceived(IPC::Message const&) 	obj-firefox/ipc/ipdl/PFilePickerChild.cpp
5 	xul.dll 	mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) 	obj-firefox/ipc/ipdl/PContentChild.cpp
6 	xul.dll 	mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) 	ipc/glue/MessageChannel.cpp
7 	xul.dll 	mozilla::ipc::MessageChannel::OnMaybeDequeueOne() 	ipc/glue/MessageChannel.cpp
8 	xul.dll 	MessageLoop::DoWork() 	ipc/chromium/src/base/message_loop.cc
[...]

The address there is the poison value 0x5a5a5a5a itself. The stacks are basically the same, just that the RefPtr in frame 0 is a different one
So far haven't found what could lead to the crash.
But most probably a regression from bug 1163388
http://hg.mozilla.org/mozilla-central/annotate/4fb7ff694bf5/widget/nsFilePickerProxy.cpp#l210
sure is wrong. What if we have no files, then mFiles.Length() >= mIndex will evaluate true, and
we try to access out of bounds in GetNext
Blocks: 1163388
Please file use-after-free crashes as security bugs.
Group: core-security
Assignee: nobody → amarchesini
Attached patch crash.patchSplinter Review
Attachment #8608140 - Flags: review?(bugs)
(In reply to Andrew McCreight [:mccr8] from comment #4)
> Please file use-after-free crashes as security bugs.

I wondered about that but thought we may not need to hide as it's Nightly-only and only yesterday's and probably today's builds affected.
Comment on attachment 8608140 [details] [diff] [review]
crash.patch

Does this fix the crash?
(I couldn't figure out how to reproduce the crash)

I would use the same comparison in both places. Maybe
*aRetvalue = mIndex < mFiles.Length();

and then
NS_ENSURE_TRUE(mIndex < mFiles.Length(), NS_ERROR_FAILURE);
Attachment #8608140 - Flags: review?(bugs) → review+
Duplicate of this bug: 1166395
https://hg.mozilla.org/mozilla-central/rev/9ce09518b752
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Duplicate of this bug: 1166941
Looks like this bug has fixed the crashes I reported in bug # 1166395. I couldn't reproduce the original issue with today's m-c:
* http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-05-21-03-02-04-mozilla-central/

- attached & uploaded several documents/images to aws s3 without any crashes
- attached & uploaded several documents/images into an email using Gmail without any issues
Crash Signature: [@ nsHtml5RefPtr<nsHtml5StreamParser>::nsHtml5RefPtr<nsHtml5StreamParser>(nsHtml5StreamParser*)] [@ nsRefPtr<nsTextNode>::nsRefPtr<nsTextNode>(nsTextNode*) | `anonymous namespace''::SimpleEnumerator::GetNext(nsISupports**) ] → [@ nsHtml5RefPtr<nsHtml5StreamParser>::nsHtml5RefPtr<nsHtml5StreamParser>(nsHtml5StreamParser*)] [@ nsRefPtr<nsTextNode>::nsRefPtr<nsTextNode>(nsTextNode*) | `anonymous namespace''::SimpleEnumerator::GetNext(nsISupports**) ] [@ nsRefPtr<mozilla::dom::Au…
Duplicate of this bug: 1166727
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #6)
> (In reply to Andrew McCreight [:mccr8] from comment #4)
> > Please file use-after-free crashes as security bugs.
> 
> I wondered about that but thought we may not need to hide as it's
> Nightly-only and only yesterday's and probably today's builds affected.

Yes, but if it's not fixed it will turn into a publicly-known vulnerability in Release. Flagging things as security bugs brings extra oversight to make sure it stays Nightly-only.
Marking this as verified due to the issue being resolved. I actually ran into this issue via bug # 1166395 which I haven't seen happening since this has been resolved. This was affecting the nightly channel and would happen anytime you initiated any type of upload via Gmail, Google Drive, Facebook etc.. If this was still a problem, we would definitely hear about it.

I searched for similar crash ID's for the top 100 crashes/28 days for all the channels and didn't see similar crashes.
Status: RESOLVED → VERIFIED
Group: core-security → core-security-release
Group: core-security-release
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.