Jesse Ruderman Reporter
	Description • 19 years ago

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060103 Firefox/1.6a1

Steps to reproduce:
1. Load a testcase.
2. Load another page in the same tab (e.g. your home page).
3. Press Back.

Result: Crash.


Stack from a nightly build:

0    PresShell::Freeze() + 420
1    PresShell::EnumeratePlugins(nsIDOMDocument*, nsString const&, void (*)(PresShell*, nsIContent*)) + 236
2    PresShell::Thaw() + 132
3    nsDocShell::RestoreFromHistory() + 2744
4    nsDocShell::CaptureState() + 668
5    PL_HandleEvent + 36
6    PL_ProcessPendingEvents + 128

Security-sensitive due to suspicious stack -- how does PresShell::Freeze end up on the top of the stack?  PresShell::EnumeratePlugins only appears to call GetElementsByTagName and (through the callback) StartPluginInstance.

In a debug build, there are two assertion failures, and the stack makes more sense: StartPluginInstance is at the top, and the rest is the same.

###!!! ASSERTION: Object nodes must implement nsIObjectLoadingContent: 'objlc', file /Users/admin/trunk/mozilla/layout/base/nsPresShell.cpp, line 6234
Break: at file /Users/admin/trunk/mozilla/layout/base/nsPresShell.cpp, line 6234
###!!! ASSERTION: You can't dereference a NULL nsCOMPtr with operator->().: 'mRawPtr != 0', file ../../dist/include/xpcom/nsCOMPtr.h, line 849
Break: at file ../../dist/include/xpcom/nsCOMPtr.h, line 849


I think the problem is that EnumeratePlugins assumes that anything with the tagname "object" that QI's to nsIContent is safe to pass to its callback, but StartPluginInstance assumes that its argument can QI's to nsIObjectLoadingContent.  Possible fixes:

(A) Make EnumeratePlugins use GetElementsByTagNameNS, so it only sees <html:object>, not all <object> elements.

(B) Make StartPluginInstance test for nsIObjectLoadingContent instead of asserting and then crashing.

	Christian :Biesinger (don't email me, ping me on IRC) Assignee
	Comment 3 • 19 years ago

>(A) Make EnumeratePlugins use GetElementsByTagNameNS, so it only sees
><html:object>, not all <object> elements.

That'll fail in HTML documents, where everything is in the null namespace.

	Christian :Biesinger (don't email me, ping me on IRC) Assignee
	Comment 4 • 19 years ago

I wish this code didn't use tag names at all and just called this for all content nodes that implement nsIObjectLoadingContent

	Boris Zbarsky [:bzbarsky]
	Comment 5 • 19 years ago

Comment on attachment 207894 [details] [diff] [review]
patch

sr=bzbarsky

	Christian :Biesinger (don't email me, ping me on IRC) Assignee
	Comment 6 • 19 years ago

fixed on trunk, not needed on branch.

Checking in layout/base/nsPresShell.cpp;
/cvsroot/mozilla/layout/base/nsPresShell.cpp,v  <--  nsPresShell.cpp
new revision: 3.890; previous revision: 3.889
done

	Jesse Ruderman Reporter
	Comment 7 • 19 years ago

> Security-sensitive due to suspicious stack -- how does PresShell::Freeze end up
> on the top of the stack?  PresShell::EnumeratePlugins only appears to call
> GetElementsByTagName and (through the callback) StartPluginInstance.

I now think the Mac OS X crash reporter is on crack and this should be public.