272381 - Crash when printing a iframe [PoC included][@ GlobalWindowImpl::Print] GlobalWindowImpl::GetInterface is broken

Status: RESOLVED FIXED

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

Description

[Niek v/d Maas]

User-Agent:       Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0
Build Identifier: Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0

Just discovered this crashing vulnerability when working on one of my projects,
please take a look at the attached URL for the PoC. Mozilla (tried only Firefox)
will crash when opening this page, tested on Firefox 1.0 final on the platforms
Windows, Linux and SunOS.
CC-ing this bug to the Mozilla Security Group, AFAIK it meets all the criteria.

Reproducible: Always
Steps to Reproduce:
Go the the given URL (http://niekvandermaas.nl/zooi/firefox-crash.html)

Actual Results:  
Firefox will crash

Expected Results:  
Firefox will print the iframe

Comment 1

[Niek v/d Maas]

Additional information:

Replacing the javascript with the following code makes the problem disappears:
<a href="#" onclick="window.frames.pdfframe.print();">Click here to print</a>

Also moving the javascript to onload (in the body tag) 'solves' the problem. So
this crash only appears when directly executing the javascript in the document.

Comment 2

[Niek v/d Maas]

(In reply to comment #1)
> -snip-
> <a href="#" onclick="window.frames.pdfframe.print();">Click here to print</a>
> -snip

Of course that should be pocframe...

Comment 3

[Daniel Veditz [:dveditz]]

Can't reach your site, please add your testcase as an attachment to the bug.

Comment 4

[Niek v/d Maas]

Attachment: Firefox crash testcase/PoC

(In reply to comment #3)
> Can't reach your site, please add your testcase as an attachment to the bug.

My ISP has some problems, here's the testcase as an attachment.

Comment 5

[Niek v/d Maas]

Has anyone of the security team actually /looked/ at this bug? Alex's story
comes to mind (http://weblogs.mozillazine.org/weirdal/archives/human259708.html)...
If there are no objections, I will post this vulnerability tommorow on
full-disclosure.

Comment 6

[timeless]

go away, this is a null pointer deref, however thanks to the security flag our
normal qa was not able to see the bug and could not get a stack trace for us.

it doesn't help that you filed this bug in the wrong product.

Comment 7

[timeless]

Incident ID: 2389668
Stack Signature	GlobalWindowImpl::Print() 368029c0
Product ID	CaminoTrunk
Build ID	2004120108
Trigger Time	2004-12-06 07:57:40.0
Platform	MacOSX
Operating System	Darwin 7.6.0
Module	Camino + (003a5284)
URL visited	https://bugzilla.mozilla.org/attachment.cgi?id=167454&action=view
User Comments	bug 272381
Since Last Crash	385065 sec
Total Uptime	385065 sec
Trigger Reason	SIGBUS: Bus Error: (signal 10)
Source File, Line No.
/builds/camino-release/camino/trunk/mozilla/dom/src/base/nsGlobalWindow.cpp,
line 848
Stack Trace 	
GlobalWindowImpl::Print() 
[/builds/camino-release/camino/trunk/mozilla/dom/src/base/nsGlobalWindow.cpp,
line 848]

analysis: GetInterface returns NS_OK and outs null. this is absolutely broken.
it can happen for all sorts of reasons because the code is really broken. it
just needs to be fixed.

Comment 8

[timeless]

Attachment: make GetInterface honor the interface

Comment 9

[timeless]

Comment on attachment 168028 [details] [diff] [review]
make GetInterface honor the interface

mozilla/dom/src/base/nsGlobalWindow.cpp 	1.715

Comment 10

[timeless]

Comment on attachment 168028 [details] [diff] [review]
make GetInterface honor the interface

mozilla/dom/src/base/nsGlobalWindow.cpp 	1.716

oh well, the test was reversed, but it was the right problem and fix.

Comment 11

[[e-mail]]

Comment on attachment 168028 [details] [diff] [review]
make GetInterface honor the interface

>   if (aIID.Equals(NS_GET_IID(nsIDocCharset))) {
>     if (mDocShell) {
>       nsCOMPtr<nsIDocCharset> docCharset(do_QueryInterface(mDocShell));
>       if (docCharset) {
>         *aSink = docCharset;
>         NS_ADDREF(((nsISupports *) *aSink));
>       }
>     }
>   }
A common technique here would be:
   if (aIID.Equals(NS_GET_IID(nsIDocCharset))) {
     if (mDocShell) {
       return mDocShell->QueryInterface(aIID, aSink);
     }
   }

Comment 12

[Marek Stępień [:marcoos, inactive]]

Can this be checked into the 1.0.x branch for 1.0.4? This seems to be only fixed
on trunk.

This crasher problem has given Firefox some really bad publicity in Poland,
recently - http://di.com.pl/n/?lp=9796&r=1 (this is one of the major IT related
vortals).

Comment 13

[Zibi Braniecki [:zbraniecki][:gandalf]]

I'd say no. We should not panic just because some press decided to write a news
about such "amazing" bug.

Comment 14

[Marek Stępień [:marcoos, inactive]]

If attachment 168028 [details] [diff] [review] is what was checked in on trunk, it looks like a simple
thing - changing just one line of the code. I don't think it would do any harm
to the branch, while being good for marketing reasons. ;)

Comment 15

[Josh Birnbaum]

*** Bug 293919 has been marked as a duplicate of this bug. ***

Comment 16

[Natalia Kowalska]

I wouldn't say that. We in fact shouldn't be panicking just because someone decided to write a news
about something about this "amazing" bug. However there is also something that you should check out https://www.zdrowy-styl-zycia.pl/. Some people should really find this helpful.