486269 - (CVE-2009-1837) Race condition [@ NPObjWrapper_NewResolve] while accessing the private data of an NPObject JS wrapper class object

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect, P2)

Description

[Reed Loden [:reed]]

From Secunia:
-----------------
Hi guys,

A long time ago, I noticed a vulnerability in Firefox 2.x when loading
Java applets. Before getting around to reporting it, it was fixed (and
still looks fixed). However, a similar problem seems to have been
introduced in Firefox 3.x.

The vulnerability is caused due to a race condition in xul.dll while
accessing the private data of an NPObject JS wrapper class object when
navigating away from a web page during loading of a Java applet. This
can be exploited via a specially crafted web page to use already freed
memory.

Successful exploitation may allow execution of arbitrary code.

The vulnerability is confirmed in Firefox 3.0.7 for Windows with Java
JRE 6 Update 13 including npjp2.dll version 6.0.130.3. Other versions
may also be affected.



Exploitation:
-------------

Secunia Research has created a PoC, which is available upon request.

The vulnerability can also be reproduced quite simply by accessing a
web page loading a java applet and navigate away from it (e.g. using
the "Back" button) before the applet is loaded.

As an example, do the following:
1) Navigate to:
http://secunia.com/vulnerability_scanning/online/

2) Click the "Start Scanner" button.
3) Immediately click the back button before the applet is loaded.
4) After hanging for a couple of seconds, the browser should crash.

NOTE: In most cases zeroed memory is used, thus only causing a crash.
However, it has been proven that it is possible to write to the memory
before it is re-used.


Closing comments:
-----------------

We have assigned this vulnerability Secunia advisory SA34241, but have
not assigned a CVE identifier from our CNA pool. Please do so from your
pool.

A preliminary disclosure date of 2009-04-08 10am CET has been set where
the details will be publicly disclosed. However, we are naturally
prepared to push the disclosure date if you need more time to address
the vulnerability.

Please acknowledge receiving this e-mail and let us know when you
expect to fix the vulnerability.

Credits should go to:
Jakob Balle and Carsten Eiram, Secunia Research.

If this qualifies for your bug bounty, then please keep the money and
spend it on beers for the developers instead.

Also, if you have any questions, then please don't hesitate to contact
me.

-----------------

This bug needs a better summary...

Comment 1

[Martijn Wargers (dead)]

CC-ing Kenneth, perhaps this is a Java issue of some sorts?
After a lot of tries, I could make it crash, using Firefox3.0.7 and after I updated to the latest Java version. However the crash reporter didn't come up for me.

Comment 2

[Kenneth Russell]

I was able to catch this in the debugger, but it looks like a bug in Firefox to me and not a bug in the Java Plug-In. If investigation on the Mozilla side indicates that there is a bug in the Java Plug-In please let me know. I would regardless recommend to ask Secunia to push out their planned announcement date by at least a few weeks -- one week to address an issue like this is unreasonable.

Comment 3

[Reed Loden [:reed]]

Attachment: PoC

From Carsten responding to my request for the PoC:
----------------
I've attached a crude test page - it basically just loads a java applet
from our website. Since timing may vary, we haven't set up a timer to
trigger the crash. Instead, open the page, give it ½ second or so and
then navigate away from the page (before the applet is fully loaded),
which should crash Firefox.

Just a bit more on what seems to be the problem:

Initially, I was evaluating whether this was a Java plug-in problem and
not a Firefox problem, but eventually determined that the problem does
seem to be Firefox.

Firefox 2.x does not allow you to navigate away from a page loading a
Java applet before it is fully loaded. It is therefore not possible to
trigger this issue in the 2.x version.

However, Firefox 3.x does allow navigating away from a page while still
attempting to load a java applet. This may cause objects to be
destructed while loading the applet and when returning to the affected
function in xul.dll, the referenced object will no longer valid. This
triggers the seen crash when calling a virtual function from inside
xul.dll.

I haven't tested this on Linux, but I've included some IDA output from
the Windows tests with a few comments.

Please let me know if you need additional information - I will also be
monitoring the bug report.
----------------

Comment 4

[Reed Loden [:reed]]

Attachment: IDA output

From Carsten -- "IDA output from the Windows tests with a few comments"

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to comment #2)
> I was able to catch this in the debugger, but it looks like a bug in Firefox to
> me and not a bug in the Java Plug-In.

Kenneth, could you attach the crash stack you saw?

Comment 6

[Kenneth Russell]

It was useless -- no symbols. A Mozilla developer with a debug build will need to catch it to get a useful stack trace. All I can say is that the crash was provoked from the JavaScript interpreter (js3250.dll) calling into xul.dll.

Comment 7

[Martijn Wargers (dead)]

Attachment: stack from debug build

Comment 8

[Martijn Wargers (dead)]

 	259a2115()	
>	gkplugin.dll!NPObjWrapper_NewResolve(JSContext * cx=0x095d7b50, JSObject * obj=0x0a003480, int id=176624452, unsigned int flags=1, JSObject * * objp=0x0012e160)  Line 1537 + 0x12 bytes	C++
 	js3250.dll!js_LookupPropertyWithFlags(JSContext * cx=0x095d7b50, JSObject * obj=0x0a003480, int id=176624452, unsigned int flags=1, JSObject * * objp=0x0012e1c0, JSProperty * * propp=0x0012e1b0)  Line 3889 + 0x17 bytes	C++
 	js3250.dll!js_GetPropertyHelper(JSContext * cx=0x095d7b50, JSObject * obj=0x0a003420, int id=176624452, int * vp=0x0012e84c, JSPropCacheEntry * * entryp=0x0012e664)  Line 4240 + 0x23 bytes	C++
 	js3250.dll!js_GetMethod(JSContext * cx=0x095d7b50, JSObject * obj=0x0a003420, int id=176624452, int * vp=0x0012e84c, JSPropCacheEntry * * entryp=0x0012e664)  Line 4335 + 0x19 bytes	C++
 	js3250.dll!js_Interpret(JSContext * cx=0x095d7b50)  Line 4368 + 0x2b bytes	C++
 	js3250.dll!js_Invoke(JSContext * cx=0x095d7b50, unsigned int argc=1, int * vp=0x0931701c, unsigned int flags=0)  Line 1315 + 0x9 bytes	C++
 	js3250.dll!js_InternalInvoke(JSContext * cx=0x095d7b50, JSObject * obj=0x0a8830a0, int fval=176241504, unsigned int flags=0, unsigned int argc=1, int * argv=0x09317018, int * rval=0x0012ea44)  Line 1373 + 0x15 bytes	C++
 	js3250.dll!JS_CallFunctionValue(JSContext * cx=0x095d7b50, JSObject * obj=0x0a8830a0, int fval=176241504, unsigned int argc=1, int * argv=0x09317018, int * rval=0x0012ea44)  Line 5189 + 0x1f bytes	C++
 	gklayout.dll!nsJSContext::CallEventHandler(nsISupports * aTarget=0x08e1fac0, void * aScope=0x0a8830a0, void * aHandler=0x0a813b60, nsIArray * aargv=0x08dd4870, nsIVariant * * arv=0x0012ec10)  Line 2007 + 0x21 bytes	C++
 	gklayout.dll!nsJSEventListener::HandleEvent(nsIDOMEvent * aEvent=0x093d1b70)  Line 247 + 0x67 bytes	C++
 	gklayout.dll!nsEventListenerManager::HandleEventSubType(nsListenerStruct * aListenerStruct=0x08ca43f8, nsIDOMEventListener * aListener=0x09bc3ce8, nsIDOMEvent * aDOMEvent=0x093d1b70, nsPIDOMEventTarget * aCurrentTarget=0x095d7948, unsigned int aPhaseFlags=6)  Line 1090 + 0x12 bytes	C++
 	gklayout.dll!nsEventListenerManager::HandleEvent(nsPresContext * aPresContext=0x098111c0, nsEvent * aEvent=0x0012efe4, nsIDOMEvent * * aDOMEvent=0x0012ef20, nsPIDOMEventTarget * aCurrentTarget=0x095d7948, unsigned int aFlags=6, nsEventStatus * aEventStatus=0x0012ef24)  Line 1189	C++
 	gklayout.dll!nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor & aVisitor={...}, unsigned int aFlags=6, int aMayHaveNewListenerManagers=1)  Line 228	C++
 	gklayout.dll!nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor & aVisitor={...}, unsigned int aFlags=6, nsDispatchingCallback * aCallback=0x00000000, int aMayHaveNewListenerManagers=1)  Line 293	C++
 	gklayout.dll!nsEventDispatcher::Dispatch(nsISupports * aTarget=0x095d7908, nsPresContext * aPresContext=0x098111c0, nsEvent * aEvent=0x0012efe4, nsIDOMEvent * aDOMEvent=0x00000000, nsEventStatus * aEventStatus=0x0012efe0, nsDispatchingCallback * aCallback=0x00000000)  Line 508 + 0x14 bytes	C++
 	gklayout.dll!DocumentViewerImpl::LoadComplete(unsigned int aStatus=0)  Line 1001 + 0x21 bytes	C++
etc...

Seems related to bug 430593.

Comment 9

[Martijn Wargers (dead)]

I guess Boris might have ideas, since it seems related to bug 430593.

Comment 10

[Johnny Stenback (:jst)]

Attachment: Possible fix.

This is likely to fix this crash. The problem is, AFAICT, that we're calling into a Java NPObject (npobj->hasProperty()) which ends up tearing down the plugin, and then we try to call into the same object again (npobj->hasMethod()) and we crash. This patch adds plugin destruction guards to this code, which should prevent the plugin from being destroyed while we're doing our work here.

I pushed this to the try server (look for java-crashfix) for others to test with, but it'll be a few hours before builds will be available. I've only been able to reproduce this crash once myself, so I can't say for sure that this does indeed fix the problem. But it feels right.

Comment 11

[Johnny Stenback (:jst)]

Tryserver builds available here:

https://build.mozilla.org/tryserver-builds/2009-04-01_17:[e-mail]-java-crashfix/

Martijn. can you see if you can reproduce with one of those builds?

Comment 12

[Martijn Wargers (dead)]

Yeah, it seems like the tryserver build fixes this crash. Note however, I'm not completely sure, since the crash isn't too easily reproducable.

Comment 13

[Carsten Eiram]

(In reply to comment #12)
> Yeah, it seems like the tryserver build fixes this crash. Note however, I'm not
> completely sure, since the crash isn't too easily reproducable.

We've been able to reproduce the crash very reliably in both Firefox 3.0.8 and 3.0.7. However, I've just spent some time testing the tryserver build and haven't been able to crash it so it seems to fix the vulnerability nicely.

Comment 14

[Josh Aas]

Comment on attachment 370549 [details] [diff] [review]
Possible fix.

Looks good to me, makes sense that we should be guarding against destruction when calling into the object twice like that. However, it is odd that hasProperty or hasMethod would cause destruction.

Comment 15

[Johnny Stenback (:jst)]

Yeah, it is, but in the case of Java it makes sense, since those calls are remoted to the applet process which means the in-process Java plugin processes events while waiting for the applet, which can cause the Java plugin to be destroyed.

Comment 16

[Johnny Stenback (:jst)]

Blocking to make sure we get this landed.

Comment 17

[Johnny Stenback (:jst)]

Fixed on trunk and branch.

http://hg.mozilla.org/mozilla-central/rev/64fda97c4965
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/677ab64c4e9d

Comment 18

[Reed Loden [:reed]]

Comment on attachment 370549 [details] [diff] [review]
Possible fix.

Not sure if this applies fine on 1.9.0.10, but requesting approval.

Comment 19

[Samuel Sidler (old account; do not CC)]

Johnny, does this patch apply cleanly to the 1.9.0 branch?

Comment 20

[Daniel Veditz [:dveditz]]

Comment on attachment 370549 [details] [diff] [review]
Possible fix.

Not sure this is the patch jst wants approval on failing answer to Sam's question.

Comment 21

[Samuel Sidler (old account; do not CC)]

Johnny: Does this patch apply to 1.9.0? If not, can you please attach a patch suitable for that branch? Code freeze for 1.9.0.10 is April 22.

Comment 22

[Johnny Stenback (:jst)]

Attachment: Same thing for 1.9.0.

This is the same as above, but applies cleanly for 1.9.0.

Comment 23

[Daniel Veditz [:dveditz]]

Comment on attachment 373405 [details] [diff] [review]
Same thing for 1.9.0.

Approved for 1.9.0.10, a=dveditz for release-drivers

Comment 24

[Johnny Stenback (:jst)]

Fixed in CVS.

Comment 25

[Samuel Sidler (old account; do not CC)]

(In reply to comment #23)
> Approved for 1.9.0.10, a=dveditz for release-drivers

Because of an urgent issue, this will now be fixed in Firefox 3.0.11, which has a very similar timeline for 3.0.10 (an inserted release).

Comment 26

[Al Billings [:abillings - ex-MoCo]]

I have seen the crash in 1.9.0.10 on XP but it doesn't reliably crash with the POC. I haven't seen the crash when testing on 1.9.0.11 but because of the unreliability, it is hard to say it is fixed by watching the behavior.

Comment 27

[joris huizer]

This may or may not be valid, but I'm getting the crash on the POC (regularly), running firefox 3.0.11 (which I just downloaded), running under Wine