Robert Kaiser Reporter
	Description • 9 years ago

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

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 1 • 9 years ago

So far haven't found what could lead to the crash.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 2 • 9 years ago

But most probably a regression from bug 1163388

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 3 • 9 years ago

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

	Andrew McCreight [:mccr8]
	Comment 4 • 9 years ago

Please file use-after-free crashes as security bugs.

	Robert Kaiser Reporter
	Comment 6 • 9 years ago

(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.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 7 • 9 years ago

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);

	Andrea Marchesini [:baku] Assignee
	Comment 8 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/9ce09518b752

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 10 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/9ce09518b752

	Kamil Jozwiak [:kjozwiak]
	Comment 12 • 9 years ago

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

	Daniel Veditz [:dveditz]
	Comment 14 • 9 years ago

(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.

	Kamil Jozwiak [:kjozwiak]
	Comment 16 • 9 years ago

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.