535641 - Assertion failure: (mContext)->tempValueRooters == (&mTvr)

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Carsten Book [:Tomcat]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.7pre) Gecko/20091217 Shiretoko/3.5.7pre (debug)

Steps to reproduce:
-> Load http://n.yam.com/view/mkvideopage.php/20091202455780
--> Crashes after a few seconds on load

also crashes on 1.9.2 but different stack :/

Assertion failure: (mContext)->tempValueRooters == (&mTvr), at c:\work\mozilla\b
uilds\1.9.1\mozilla\firefox-debug\dist\include\js\jscntxt.h:1071

(3c4.840): Break instruction exception - code 80000003 (!!! second chance !!!)
eax=00000091 ebx=0012edf0 ecx=2c0ba550 edx=10313d38 esi=08aba000 edi=00000000
eip=7c90120e esp=0012ebbc ebp=0012ebc0 iopl=0         nv up ei pl nz ac pe nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00000216

7c90120e cc              int     3
0:000> cdb: Reading initial command '!load winext\msec.dll;.logappend;!exploitab
le;k;q'

Exploitability Classification: UNKNOWN
Recommended Bug Title: Breakpoint starting at ntdll!DbgBreakPoint+0x000000000000
0000 called from gkplugin!JSAutoTempValueRooter::~JSAutoTempValueRooter+0x000000
0000000031 (Hash=0x687e1e27.0x2e7e0429)

While a breakpoint itself is probably not exploitable, it may also be an indicat
ion that an attacker is testing a target. In either case breakpoints should not
exist in production code.
ChildEBP RetAddr
WARNING: Stack unwind information not available. Following frames may be wrong.
0012ebc0 0750b501 ntdll!DbgBreakPoint
0012ebd8 0750aef3 gkplugin!JSAutoTempValueRooter::~JSAutoTempValueRooter+0x31
0012ed4c 08606102 gkplugin!_evaluate+0x4e3
0012ed88 085058e1 NPSWF32!NP_Shutdown+0xcde
0012ede4 085b9774 NPSWF32+0x358e1
0012ee98 08570238 NPSWF32+0xe9774
0012f0c4 085a47d8 NPSWF32+0xa0238
0012f168 0860959a NPSWF32+0xd47d8
0012f23c 1021cb0a NPSWF32!NP_Shutdown+0x4176
0012f24c 1021bb21 MSVCR80D!CrtIsValidHeapPointer+0x15a
0012f268 0045936a MSVCR80D!free_dbg+0x161
0012f27c 004593ba nspr4!_MD_CURRENT_THREAD+0x1a
00d153f8 00000000 nspr4!_MD_CURRENT_THREAD+0x6a
quit:

Comment 1

[Carsten Book [:Tomcat]]

Attachment: zipped page source

zipped page source, but do not crash on local testing :(

Comment 2

[Carsten Book [:Tomcat]]

(In reply to comment #0)
> also crashes on 1.9.2 but different stack :/
> 

more testing show, the same fatal assertion can be triggered also on 1.9.2 debug build  Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b6pre) Gecko/20091217 Namoroka/3.6b6pre - but there is also another crash on the same site for 1.9.1/1.9.2 - will file a seperate bug for this.

Report for 1.9.2:

Assertion failure: (mContext)->tempValueRooters == (&mTvr), at c:\work\mozilla\b
uilds\1.9.2\mozilla\firefox-debug\dist\include\jscntxt.h:1338


(228.47c): Break instruction exception - code 80000003 (!!! second chance !!!)
eax=0000008e ebx=0012eeb0 ecx=9df06db6 edx=10313d38 esi=094e5000 edi=00000000
eip=7c90120e esp=0012ec70 ebp=0012ec74 iopl=0         nv up ei pl nz na pe nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00000206

Exploitability Classification: UNKNOWN
Recommended Bug Title: Breakpoint starting at ntdll!DbgBreakPoint+0x000000000000
0000 called from gkplugin!JSAutoTempValueRooter::~JSAutoTempValueRooter+0x000000
0000000031 (Hash=0x687e1e27.0x06660534)

While a breakpoint itself is probably not exploitable, it may also be an indicat
ion that an attacker is testing a target. In either case breakpoints should not
exist in production code.
ChildEBP RetAddr
WARNING: Stack unwind information not available. Following frames may be wrong.
0012ec74 0695ac41 ntdll!DbgBreakPoint
0012ec8c 0695a5aa gkplugin!JSAutoTempValueRooter::~JSAutoTempValueRooter+0x31
0012ee0c 09026102 gkplugin!_evaluate+0x4fa
0012ee48 08f258e1 NPSWF32!NP_Shutdown+0xcde
0012eea4 08fd9774 NPSWF32+0x358e1
0012ef58 08f90238 NPSWF32+0xe9774
0012f184 08fc47d8 NPSWF32+0xa0238
0012f228 0902959a NPSWF32+0xd47d8
0012f2e8 004321e2 NPSWF32!NP_Shutdown+0x4176
00d0be58 00000000 nspr4!PR_GetThreadPrivate+0x62
quit:

Comment 3

[Jeff Walden [:Waldo]]

(gdb) p *script
$3 = {UTF8Characters = 0x7fffd84252c8 "try { __flash__toXML(onADVideoStop()) ; } catch (e) { \"<undefined/>\"; }", UTF8Length = 71}
(gdb) p rv
$4 = 0
(gdb) p result
$5 = (NPVariant *) 0x7fffffffb6a0
(gdb) p *rval
$6 = 140737020061732
(gdb) p/x *rval
$7 = 0x7fffe4166824
(gdb) p (JSString*)(rval&~7)
Argument to arithmetic operation not a number or boolean.
(gdb) p (JSString*)(*rval&~7)
$8 = (JSString *) 0x7fffe4166820
(gdb) p $8->chars()
$9 = (jschar *) 0x7fffde7d8620
(gdb) x/5ch $9
0x7fffde7d8620:	60 '<'	117 'u'	110 'n'	100 'd'	101 'e'

Something in one or the other of those function calls threw and didn't unwind tvrs correctly, more debugging anon.

Comment 4

[Jeff Walden [:Waldo]]

  JSContext *cx = GetJSContextFromDoc(doc);
...
  JSAutoTempValueRooter tvr(cx, NS_ARRAY_LENGTH(vec), vec);
...
  nsCOMPtr<nsIScriptContext> scx = GetScriptContextFromJSContext(cx);
...
}

tvr pushes onto cx->tempValueRooters.  scx wraps cx.  The evaluate call happens.  The function body finishes.  We start calling destructors before returning.  scx releases its context via XPConnect, which is supposed to handle release by destroying at the appropriate time, but: !XPCPerThreadData::GetData(aJSContext)->GetCallContext(), so destruction happens immediately because the context's "not in use".  So now you're using freed memory, and that's not so good.

This is a plugins bug, throwing over there for lack of certainty about the precise look of whatever should be added to _evaluate to Push/Pop cx correctly (or maybe the tvr just needs to be moved down, or maybe more is needed, or or or)...

The code dates back quite a ways (although it used to be in a differently-named file), so realistically this is probably in all releases we care to support.

Comment 5

[Carsten Book [:Tomcat]]

cc: jst

Also note: depending on some timing (it seems) i crashed with the stack of Bug 535651 at the same site with 1.9.1/1.9.2

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix

Yeah, we just need to make sure we hold onto the scx longer than the tvr.

Comment 7

[Daniel Veditz [:dveditz]]

I assume from comment 4 we need this on 1.9.0 as well?

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

I marked this blocking- because it was new and didn't seem common, but wondering if it should actually block after all.

Blake? Jonas? Any thoughts? See related bug 535651

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Decided on a triage call with bsmedberg and Waldo that we should fix in a security release, but not block final on it. Would take the patch as a ridealong if we need a RC respin and it's baked on trunk.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Landed on mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/f322da97935a

Please remember to nominate this for an appropriate security release.

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 418304 [details] [diff] [review]
Fix

I'm not sure how to specify "ready for ridealong".

Comment 12

[Daniel Veditz [:dveditz]]

Comment on attachment 418304 [details] [diff] [review]
Fix

Approved for 1.9.1.8, a=dveditz for release-drivers

Do we need a different patch for 1.9.0.x or will this apply?

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 418304 [details] [diff] [review]
Fix

This applies (with a file rename to 1.9.0).

Comment 14

[Daniel Veditz [:dveditz]]

Comment on attachment 418304 [details] [diff] [review]
Fix

Approved for 1.9.0.18, a=dveditz for release-drivers

Comment 15

[Blake Kaplan (:mrbkap) (inactive)]

Checking in modules/plugin/base/src/ns4xPlugin.cpp;
/cvsroot/mozilla/modules/plugin/base/src/ns4xPlugin.cpp,v  <--  ns4xPlugin.cpp
new revision: 1.168; previous revision: 1.167
done

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/2ce89e85b342

Comment 16

[Blake Kaplan (:mrbkap) (inactive)]

Oops, wrong 1.9.1 changeset. I wanted http://hg.mozilla.org/releases/mozilla-1.9.1/rev/16764ab59892

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

a192=beltzner

Comment 18

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/654d7d4a64bd

Comment 19

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

Verified this as fixed for 1.9.1 with my debug build, Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.8pre) Gecko/20100201 Shiretoko/3.5.8pre (.NET CLR 3.5.30729). The pre-fix debug build that I have crashes on the live page. Post-fix, there is no crash.

Comment 20

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

Strangely, my pre-fix debug build for 1.9.0 does not crash on XP (whereas 1.9.1 build on the same machine does). I don't see the assertions listed above either.

Did we ever see this crash on 1.9.0?

There is no difference in the debug output between 1.9.0.18 and the pre-fix build and neither crashes.

Comment 21

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to comment #20)
> Strangely, my pre-fix debug build for 1.9.0 does not crash on XP (whereas 1.9.1
> build on the same machine does). I don't see the assertions listed above
> either.

It's possible that this wouldn't crash based on GC/CC things or other random references to the nsIScriptContext.