Last Comment Bug 575836 - (CVE-2010-2755) free() of garbage values when an <object> has a |data=""| attribute and no "src" attribute --> crash [@ free | nsPluginInstanceOwner::~nsPluginInstanceOwner() ]
(CVE-2010-2755)
: free() of garbage values when an <object> has a |data=""| attribute and no "s...
Status: VERIFIED FIXED
[sg:critical?]
: fixed1.9.0.20, regression, topcrash, verified1.9.2
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
:
Mentors:
http://itunes.apple.com/us/podcast/th...
: 574533 574762 580600 580874 581520 (view as bug list)
Depends on:
Blocks: CVE-2010-1214
  Show dependency treegraph
 
Reported: 2010-06-29 16:50 PDT by Daniel Holbert [:dholbert] (largely AFK until June 28)
Modified: 2011-06-13 10:01 PDT (History)
21 users (show)
benjamin: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta2+
.8+
.8-fixed
-
.12-fixed


Attachments
Don't try to copy |data=""| to |src=""| (1.47 KB, patch)
2010-06-29 22:58 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
jst: review+
Details | Diff | Review
(WARNING: CRASHER) Small testcase (35 bytes, text/html)
2010-06-30 10:55 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details
1.9.2 fix v1.0 (2.16 KB, patch)
2010-07-20 19:29 PDT, Josh Aas
cjones.bugs: review+
dveditz: approval1.9.2.8+
Details | Diff | Review
1.9.1 fix v1.0 (2.27 KB, patch)
2010-07-22 12:49 PDT, Josh Aas
cjones.bugs: review+
dveditz: approval1.9.1.12+
dveditz: approval1.9.0.next+
Details | Diff | Review
Test (based on m-c tip) (872 bytes, patch)
2010-07-22 13:01 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Review

Description Daniel Holbert [:dholbert] (largely AFK until June 28) 2010-06-29 16:50:18 PDT
OS: Ubuntu Maverick (10.10) pre-release

STR:
 1. Load URL
   http://itunes.apple.com/us/podcast/the-bugle-audio-newspaper/id265799883

ACTUAL RESULTS: Immediate crash in opt build; shutdown crash in debug build.

NOTE: This doesn't affect my other machine running an earlier Ubuntu version (10.04).

On the broken machine, if I disable the "QuickTime Plug-in 7.6.6" in about:addons, it fixes the problem. (Note: this plugin's subtitle shows that it's actually handled by "The Totem 2.30.2 plugin", since this is Linux and there's no _actual_ Quicktime support)  So, this is probably due to a totem bug, but I'm filing here because it crashes Firefox and I'd hope that OOPP would prevent that.

Firefox version:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:2.0b2pre) Gecko/20100629 Minefield/4.0b2pre

TOTEM PLUGIN VERSIONS:
  Working Ubuntu 10.04 box: 2.30.2-0ubuntu1
  Broken  Ubuntu 10.10 box: 2.30.2-2ubuntu2
Comment 1 Daniel Holbert [:dholbert] (largely AFK until June 28) 2010-06-29 16:54:01 PDT
(FWIW, I installed Ubuntu 10.10 today using the daily CD image.  Alpha 2 comes out this Thursday, but I had a glitch in my 10.04 installation and I couldn't wait for that. :))
Comment 3 Daniel Holbert [:dholbert] (largely AFK until June 28) 2010-06-29 17:53:57 PDT
Here's the terminal output from a debug build, starting from just after I load the itunes page, up until the shutdown crash:
{
For video/quicktime found plugin libtotem-narrowspace-plugin.so
** (firefox-bin:13625): DEBUG: NP_Initialize
** (firefox-bin:13625): DEBUG: NP_Initialize succeeded
###!!! ASSERTION: attribute/parameter array not setup correctly for NPAPI plugins: '!values[count]', file ../../../../../mozilla/modules/plugin/base/src/nsNPAPIPluginInstance.cpp, line 1131
** (firefox-bin:13625): DEBUG: totemPlugin [0xa51c0540]
** (firefox-bin:13625): DEBUG: 0xa51c0540: "Init mimetype 'video/quicktime' mode 1"
** (firefox-bin:13625): DEBUG: 0xa51c0540: "GetScriptableNPObject [0xa51c0540]"
** (firefox-bin:13625): DEBUG: totemNarrowSpacePlayer [0xa52feb20]
** (firefox-bin:13625): DEBUG: 0xa51c0540: "Document URI is 'http://itunes.apple.com/us/podcast/the-bugle-audio-newspaper/id265799883'"
** (firefox-bin:13625): DEBUG: 0xa51c0540: "Base URI is 'http://itunes.apple.com/us/podcast/the-bugle-audio-newspaper/id265799883'"
** (firefox-bin:13625): DEBUG: 0xa51c0540: "Real mimetype for 'video/quicktime' is 'video/quicktime'"
argv[0] type video/quicktime
argv[1] data 
argv[2] id media-player
argv[3] style 
argv[4] height 1
argv[5] width 1
argv[6] PARAM 
argv[7] autoplay false
argv[8] controller false
argv[9] hide true
argv[10] EnableJavaScript true
argv[11] postdomevents true
argv[12]  
** (firefox-bin:13625): DEBUG: 0xa51c0540: "mSrcURI: "
** (firefox-bin:13625): DEBUG: 0xa51c0540: "mBaseURI: http://itunes.apple.com/us/podcast/the-bugle-audio-newspaper/id265799883"
** (firefox-bin:13625): DEBUG: 0xa51c0540: "mCache: 0"
** (firefox-bin:13625): DEBUG: 0xa51c0540: "mControllerHidden: 1"
** (firefox-bin:13625): DEBUG: 0xa51c0540: "mShowStatusbar: 0"
** (firefox-bin:13625): DEBUG: 0xa51c0540: "mHidden: 0"
** (firefox-bin:13625): DEBUG: 0xa51c0540: "mAudioOnly: 0"
** (firefox-bin:13625): DEBUG: 0xa51c0540: "mAutoPlay: 0, mRepeat: 0"
** (firefox-bin:13625): DEBUG: 0xa51c0540: "mHref: "
** (firefox-bin:13625): DEBUG: 0xa51c0540: "mTarget: "
** (firefox-bin:13625): DEBUG: 0xa51c0540: "Viewer spawned, PID 13832"
nsPluginNativeWindowGtk2: NPPVpluginNeedsXEmbed=1
nsPluginNativeWindowGtk2: call SetWindow with xid=0x5800f77
** (firefox-bin:13625): DEBUG: 0xa51c0540: "Initial window set, XID 5800f77 size 1x1"
** (firefox-bin:13625): DEBUG: 0xa51c0540: "No viewer proxy yet, deferring SetWindow"
nsPluginNativeWindowGtk2: call SetWindow with xid=0x5800f77
** (firefox-bin:13625): DEBUG: 0xa51c0540: "NewStream mimetype 'video/quicktime' URL 'http://itunes.apple.com/us/podcast/the-bugle-audio-newspaper/id265799883'"
** (firefox-bin:13625): DEBUG: 0xa51c0540: "Not expecting a new stream; aborting stream"
** (firefox-bin:13625): DEBUG: 0xa51c0540: "GetScriptableNPObject [0xa51c0540]"
WARNING: NS_ENSURE_TRUE(mBoundFrame) failed: file ../../../../../mozilla/content/html/content/src/nsTextEditorState.cpp, line 1326
WARNING: NS_ENSURE_TRUE(mBoundFrame) failed: file ../../../../../mozilla/content/html/content/src/nsTextEditorState.cpp, line 1326
WARNING: NS_ENSURE_TRUE(mBoundFrame) failed: file ../../../../../mozilla/content/html/content/src/nsTextEditorState.cpp, line 1326
WARNING: NS_ENSURE_TRUE(mBoundFrame) failed: file ../../../../../mozilla/content/html/content/src/nsTextEditorState.cpp, line 1326
** (firefox-bin:13625): DEBUG: 0xa51c0540: "Viewer DBus interface name is 'org.gnome.totem.PluginViewer_13832'"
** (firefox-bin:13625): DEBUG: 0xa51c0540: "NameOwnerChanged old-owner '' new-owner ':1.433'"
** (firefox-bin:13625): DEBUG: 0xa51c0540: "Viewer now connected to the bus"
** (firefox-bin:13625): DEBUG: 0xa51c0540: "ViewerSetup"
** (firefox-bin:13625): DEBUG: 0xa51c0540: "Calling SetWindow"
Viewer: SetWindow XID 92278647 size 1:1
Referrer URI: http://itunes.apple.com/us/podcast/the-bugle-audio-newspaper/id265799883
WARNING: NS_ENSURE_TRUE(mBoundFrame) failed: file ../../../../../mozilla/content/html/content/src/nsTextEditorState.cpp, line 1326
TotemEmbedded-Message: Viewer state: STOPPED
** (firefox-bin:13625): DEBUG: SetWindow reply
** (firefox-bin:13625): DEBUG: 0xa51c0540: "ViewerReady"
pldhash: for the table at address 0xa8e408a8, the given entrySize of 48 probably favors chaining over double hashing.
++DOCSHELL 0xa8e40840 == 8
++DOMWINDOW == 14 (0xa8e40a68) [serial = 18] [outer = (nil)]
++DOMWINDOW == 15 (0xa8e40c58) [serial = 19] [outer = 0xa8e40a30]
--DOCSHELL 0xb0d4c5e0 == 7
--DOCSHELL 0xb0d4c9c0 == 6
** (firefox-bin:13625): DEBUG: ~totemPlugin [0xa51c0540]
** (firefox-bin:13625): DEBUG: ~totemNarrowSpacePlayer [0xa52feb20]
../../../mozilla/memory/jemalloc/jemalloc.c:4215: Failed assertion: "arena != NULL"

Program ./dist/bin/firefox-bin (pid = 13625) received signal 6.
Stack:
UNKNOWN 0xb770b410
abort+0x00000182 [/lib/libc.so.6 +0x0002DE42]
UNKNOWN [./dist/bin/firefox-bin +0x000127E0]
UNKNOWN [./dist/bin/firefox-bin +0x00012C59]
free+0x000000F5 [./dist/bin/firefox-bin +0x00016507]
moz_free+0x0000001D [./dist/bin/libmozalloc.so +0x00000C89]
NS_Free_P+0x0000001D [./dist/bin/libxpcom_core.so +0x000C8EC1]
UNKNOWN [/scratch/work/builds/mozilla-central/mozilla-central.09-09-08.16-30/obj/dist/bin/components/libgklayout.so +0x0026ECFE]
UNKNOWN [/scratch/work/builds/mozilla-central/mozilla-central.09-09-08.16-30/obj/dist/bin/components/libgklayout.so +0x0026F170]
UNKNOWN [/scratch/work/builds/mozilla-central/mozilla-central.09-09-08.16-30/obj/dist/bin/components/libgklayout.so +0x00276B55]
UNKNOWN [/scratch/work/builds/mozilla-central/mozilla-central.09-09-08.16-30/obj/dist/bin/components/libgklayout.so +0x00279CC1]
nsRunnable::Release()+0x000000BC [./dist/bin/libxpcom_core.so +0x00038368]
UNKNOWN [/scratch/work/builds/mozilla-central/mozilla-central.09-09-08.16-30/obj/dist/bin/components/libgklayout.so +0x0026D18B]
UNKNOWN [./dist/bin/libxpcom_core.so +0x000B5590]
UNKNOWN [./dist/bin/libxpcom_core.so +0x000B4656]
NS_ProcessNextEvent_P(nsIThread*, int)+0x00000085 [./dist/bin/libxpcom_core.so +0x0003893A]
UNKNOWN [/scratch/work/builds/mozilla-central/mozilla-central.09-09-08.16-30/obj/dist/bin/components/libwidget_gtk2.so +0x0006596E]
UNKNOWN [/scratch/work/builds/mozilla-central/mozilla-central.09-09-08.16-30/obj/dist/bin/components/libtoolkitcomps.so +0x00016689]
XRE_main+0x000026B0 [./dist/bin/libxul.so +0x00026B8F]
UNKNOWN [./dist/bin/firefox-bin +0x00001B47]
__libc_start_main+0x000000E6 [/lib/libc.so.6 +0x00016CE6]
Sleeping for 300 seconds.
}


And here's the backtrace trace at the crash, in that debug build:
{
#5  <signal handler called>
#6  0xb770b422 in __kernel_vsyscall ()
#7  0xb66a7951 in *__GI_raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#8  0xb66aae42 in *__GI_abort () at abort.c:92
#9  0x0805a7e0 in arena_dalloc (arena=0x0, chunk=0xa5a00000, ptr=0xa5a5a5a5) at ../../../mozilla/memory/jemalloc/jemalloc.c:4215
#10 0x0805ac59 in idalloc (ptr=0xa5a5a5a5) at ../../../mozilla/memory/jemalloc/jemalloc.c:4243
#11 0x0805e507 in free (ptr=0xa5a5a5a5) at ../../../mozilla/memory/jemalloc/jemalloc.c:6053
#12 0xb7327c89 in ?? ()
#13 0xb73f3ec1 in ?? ()
#14 0xb3e35cfe in ~nsPluginInstanceOwner (this=0xb0d19550, __in_chrg=<value optimized out>) at ../../../mozilla/layout/generic/nsObjectFrame.cpp:2544
#15 0xb3e36170 in nsPluginInstanceOwner::Release (this=0xb0d19550) at ../../../mozilla/layout/generic/nsObjectFrame.cpp:2590
#16 0xb3e3db55 in ~nsRefPtr (this=0xa4f19114, __in_chrg=<value optimized out>) at ../../dist/include/nsAutoPtr.h:969
#17 0xb3e40cc1 in ~nsStopPluginRunnable (this=0xa4f19100, __in_chrg=<value optimized out>) at ../../../mozilla/layout/generic/nsObjectFrame.cpp:2151
#18 0xb7363368 in ?? ()
#19 0xb3e3418b in nsStopPluginRunnable::Release (this=0xa4f19100) at ../../../mozilla/layout/generic/nsObjectFrame.cpp:2172
#20 0xb73e0590 in ?? ()
#21 0xb73df656 in ?? ()
#22 0xb736393a in ?? ()
#23 0xb4dd396e in nsBaseAppShell::Run (this=0xb0a62600) at ../../../../mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp:178
#24 0xb37dc689 in nsAppStartup::Run (this=0xb0844550) at ../../../../../mozilla/toolkit/components/startup/src/nsAppStartup.cpp:192
#25 0xb76b7b8f in ?? ()
#26 0x08049b47 in main (argc=4, argv=0xbfc649d4) at ../../../mozilla/browser/app/nsBrowserApp.cpp:158
}
Comment 4 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-06-29 21:40:18 PDT
Yeah, firefox-bin definitely ought not to be crashing here.

FWIW on my 10.4 installation I have the "QuickTime Plug-in 7.6.6" dholbert describes and I repro'd the crash on the latest nightly.  Looking into it.
Comment 5 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-06-29 21:55:45 PDT
The problem is shown in comment 3

###!!! ASSERTION: attribute/parameter array not setup correctly for NPAPI
plugins: '!values[count]', file
../../../../../mozilla/modules/plugin/base/src/nsNPAPIPluginInstance.cpp, line
1131
[snip]
argv[0] type video/quicktime
argv[1] data 
argv[2] id media-player
argv[3] style 
argv[4] height 1
argv[5] width 1
argv[6] PARAM 
argv[7] autoplay false
argv[8] controller false
argv[9] hide true
argv[10] EnableJavaScript true
argv[11] postdomevents true
argv[12]  

We're telling the plugin that argn and argv have 13 elements, but only 12 are valid.  On my machine, VLC crashes immediately trying to access the 13th element, but when the crash occurs depends on the random pointers that argn[12]/argv[12] end up containing (dholbert saw one much layer apparently during trying to free the argument arrays).
Comment 6 Daniel Holbert [:dholbert] (largely AFK until June 28) 2010-06-29 22:02:58 PDT
marking as security-sensitive at cjones' suggestion, just to be cautious
Comment 7 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-06-29 22:05:22 PDT
Comments 4 and 5 imply that web content may be able to trigger frees of random stack values.  I'd like to keep this closed until I figure out what the specific bug is, and which plugins/platforms it applies to.
Comment 8 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-06-29 22:10:23 PDT
(In reply to comment #7)
> random stack values.

Er, random values in the heap.
Comment 9 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-06-29 22:58:21 PDT
Created attachment 455075 [details] [diff] [review]
Don't try to copy |data=""| to |src=""|
Comment 10 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-06-29 23:00:50 PDT
This bug exists for all platforms and for both out-of-process and in-process plugins, and web content can trigger it.  The bug potentially results in two random heap values being free()d.  Off to sg for triage.
Comment 11 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-06-30 10:55:54 PDT
Created attachment 455182 [details]
(WARNING: CRASHER) Small testcase

Note that this test won't crash 100% of the time, but most (though still not 100%) of the time one should see

###!!! ASSERTION: attribute/parameter array not setup correctly for NPAPI plugins: '!values[count]', file /home/cjones/mozilla/mozilla-central/modules/plugin/base/src/nsNPAPIPluginInstance.cpp, line 1131

on stderr.
Comment 12 Daniel Holbert [:dholbert] (largely AFK until June 28) 2010-06-30 10:58:12 PDT
FWIW, I confirmed that "Small testcase" crashes for me (using today's nightly on Ubuntu 10.10).  Crash report ID 0ab7e9f7-39a2-4531-959d-8f9802100630
Comment 13 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-06-30 11:16:48 PDT
1.9.1 is susceptible to this too.
Comment 14 Daniel Holbert [:dholbert] (largely AFK until June 28) 2010-06-30 11:24:15 PDT
I've confirmed the crash using "small testcase" on latest 1.9.2 nightly:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.7pre) Gecko/20100630 Namoroka/3.6.7pre

Requesting blocking1.9.3 and blocking1.9.2, based on that. (see comment 10 for justification)

(In reply to comment #13)
> 1.9.1 is susceptible to this too.

Have you actually been able to crash a 1.9.1 build with this, though?  I couldn't, with the latest 1.9.1 release, trying both the testcase & URL with many reloads.
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.10) Gecko/20100504 Firefox/3.5.10

So if 1.9.1 is indeed affected, it seems to me like we need a different testcase in order to test the bug & it's fixedness on that branch... (or maybe it's debug-only there? or maybe there's something on my machine keeping me from crashing there?)
Comment 15 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-06-30 11:38:46 PDT
(In reply to comment #14)
> (In reply to comment #13)
> > 1.9.1 is susceptible to this too.
> 
> Have you actually been able to crash a 1.9.1 build with this, though?

Aha --- in 1.9.1, the arrays in question were allocated with PR_Calloc() (zeroing), and onward with NS_Alloc().  There's still an off-by-one error in 1.9.1, but that should only lead to NULL derefs in plugins.
Comment 16 Daniel Holbert [:dholbert] (largely AFK until June 28) 2010-06-30 11:49:52 PDT
(In reply to comment #15)
> There's still an off-by-one error in
> 1.9.1, but that should only lead to NULL derefs in plugins.

Or rather, the *potential* for null derefs in plugins, right? (I just tested "small testcase" in a 1.9.1 debug build, and it didn't crash -- but presumably it could have, depending on how the plugin was coded?)
Comment 17 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-06-30 11:58:11 PDT
Right.  We tell the plugin there are N elements in name and value arrays, but the (N-1)th elements are NULL in both.  Plugins should be checking for NULL values already, but I wouldn't be surprised if some don't check for NULL names.
Comment 18 Daniel Veditz [:dveditz] 2010-07-02 10:19:20 PDT
blocking "needed" on 1.9.2 when we get a reviewed trunk fix. "wanted" nice-to-have on 1.9.1 to fix the crash but not blocking since it seems unexploitable.
Comment 19 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-07-12 16:24:01 PDT
http://hg.mozilla.org/mozilla-central/rev/9bd68d16390e
Comment 20 Bob Clary [:bc:] 2010-07-13 02:25:42 PDT
This looks to be the same issue as bug 574533
Comment 21 Daniel Holbert [:dholbert] (largely AFK until June 28) 2010-07-13 13:39:00 PDT
Verified fixed in today's nightly:
Mozilla/5.0 (X11; Linux i686; en-US; rv:2.0b2pre) Gecko/20100713 Minefield/4.0b2pre
Comment 22 Josh Aas 2010-07-20 18:35:21 PDT
*** Bug 574533 has been marked as a duplicate of this bug. ***
Comment 23 Josh Aas 2010-07-20 19:29:50 PDT
Created attachment 458879 [details] [diff] [review]
1.9.2 fix v1.0
Comment 24 Benjamin Smedberg [:bsmedberg] 2010-07-22 12:36:19 PDT
*** Bug 580874 has been marked as a duplicate of this bug. ***
Comment 25 christian 2010-07-22 12:47:20 PDT
Comment on attachment 458879 [details] [diff] [review]
1.9.2 fix v1.0

a=LegNeato for 1.9.2.8. Please land this on BOTH mozilla-1.9.2 default and GECKO1927_20100701_RELBRANCH (in case we do a quick respin).
Comment 26 Josh Aas 2010-07-22 12:49:06 PDT
Created attachment 459544 [details] [diff] [review]
1.9.1 fix v1.0
Comment 27 Benjamin Smedberg [:bsmedberg] 2010-07-22 12:49:39 PDT
Can this test be added as a crashtest or mochitest, please?
Comment 28 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-07-22 13:01:00 PDT
Created attachment 459546 [details] [diff] [review]
Test (based on m-c tip)
Comment 29 Mike Beltzner [:beltzner, not reading bugmail] 2010-07-22 13:07:17 PDT
Comment on attachment 458879 [details] [diff] [review]
1.9.2 fix v1.0

a=beltzner for 1.9.1

Again, we'll want this on both the mozilla-1.9.1 default and the GECKO19111_20100701_RELBRANCH in case we decide to chemspill for it.
Comment 30 Josh Aas 2010-07-22 13:19:54 PDT
I'm still testing the 1.9.1 patch I posted, should be done soon.
Comment 31 Josh Aas 2010-07-22 13:46:54 PDT
pushed to mozilla-1.9.2

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/96ae337c41fb
Comment 32 [On PTO until 6/29] 2010-07-22 14:11:20 PDT
I see no crash at http://itunes.apple.com/us/podcast/apple-keynotes/id275834665
on 1.9.1.11 on OS X 10.6.
Comment 33 Josh Aas 2010-07-22 14:11:20 PDT
pushed to mozilla-1.9.2 GECKO1927_20100701_RELBRANCH

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/fd971f28dfd1

I forgot to add the bug # in the comment, sorry. I don't know how to correct that, let me know if there is a good way to do it.
Comment 34 [On PTO until 6/29] 2010-07-22 14:13:04 PDT
Small testcase attached doesn't crash 1.9.1.11 on OS X.
Comment 35 [On PTO until 6/29] 2010-07-22 14:14:40 PDT
(Sorry for the comment spam)

Small testcase doesn't crash 1.9.2.7 either on OS X 10.6.

http://itunes.apple.com/us/podcast/apple-keynotes/id275834665 does crash 1.9.2.7 on OS X 10.6 though.
Comment 36 Daniel Holbert [:dholbert] (largely AFK until June 28) 2010-07-22 14:59:29 PDT
(In reply to comment #34)
> Small testcase attached doesn't crash 1.9.1.11 on OS X.

That's known.

Through code inspection, cjones found 1.9.1 to be susceptible to this **in theory**, given a plugin with the "right" behavior -- but we haven't actually seen any plugins with that behavior. (This is discussed in comment 15 through comment 17)

The "URL crashes but testcase doesn't crash" issue from comment 35 is interesting, though -- cjones, do you know why that might happen?
Comment 37 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-07-22 15:09:10 PDT
(In reply to comment #36)
> (In reply to comment #34)
> The "URL crashes but testcase doesn't crash" issue from comment 35 is
> interesting, though -- cjones, do you know why that might happen?

The crash isn't 100% reproducible; it's a free() of a random pointer value on the heap.  See comment 11.  It's worth trying the tests 5-10 times, navigating to other sites in between, to have different heap states when the bad free() happens.
Comment 38 [:Cww] 2010-07-22 15:26:55 PDT
FWIW, I'm told this breaks all iTunes Music Store affiliate links and makes them crash so we should probably get a fix out ASAP.
Comment 39 Josh Aas 2010-07-22 15:33:35 PDT
pushed to 1.9.1 including GECKO19111_20100701_RELBRANCH

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/2f748ad22bbb
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0c9d8da0f2ed
Comment 40 Daniel Holbert [:dholbert] (largely AFK until June 28) 2010-07-22 16:01:33 PDT
(In reply to comment #38)
> FWIW, I'm told this breaks all iTunes Music Store affiliate links and makes
> them crash

To be clear: by the above, Cww is saying that people are being affected by this bug in Firefox 3.6.7.  (It's still fixed in Firefox 3.6.8 as of comment 31, though.)
Comment 41 Daniel Holbert [:dholbert] (largely AFK until June 28) 2010-07-22 16:11:56 PDT
(In reply to comment #36)
> (In reply to comment #34)
> > Small testcase attached doesn't crash 1.9.1.11 on OS X.
> 
> That's known.

Oh, sorry -- I take back comment 36.  It looks like the PR_Calloc-vs-NS_Alloc difference that cjones mentioned in comment 15 has gone away as of 1.9.1.11 -- bug 572985's patch changed that in 1.9.1.11, so I'd expect that this would affect 1.9.1.11 after all.
Comment 42 Daniel Holbert [:dholbert] (largely AFK until June 28) 2010-07-22 16:21:40 PDT
*** Bug 574762 has been marked as a duplicate of this bug. ***
Comment 43 Daniel Holbert [:dholbert] (largely AFK until June 28) 2010-07-22 17:28:01 PDT
(In reply to comment #41)
> Oh, sorry -- I take back comment 36.  It looks like the PR_Calloc-vs-NS_Alloc
> difference that cjones mentioned in comment 15 has gone away as of 1.9.1.11 --
> bug 572985's patch changed that in 1.9.1.11

Aha! I take back comment 41 and reassert comment 36.  It turns out the 1.9.1 version of bug 572985's patch was more restricted than the 1.9.2 & trunk patches there -- in particular, the 1.9.1 patch there did NOT contain the PR_Calloc --> NS_Alloc "cleanup" changes. (the changes that triggered this bug)

So, once again: we should *not* expect this bug to affect 1.9.1.11 in the same way that it affected 1.9.2 & trunk. (Though its fix is still useful on 1.9.1 in theory, as described in comment 36)

Sorry for the confusion.
Comment 44 Daniel Veditz [:dveditz] 2010-07-22 19:32:13 PDT
Comment on attachment 459544 [details] [diff] [review]
1.9.1 fix v1.0

Approved for 1.9.0.20, a=dveditz
Comment 45 Daniel Veditz [:dveditz] 2010-07-22 19:33:05 PDT
Comment on attachment 458879 [details] [diff] [review]
1.9.2 fix v1.0

Moving approval1.9.1.12 to the 1.9.1-specific patch
Comment 46 chris hofmann 2010-07-23 05:51:22 PDT
sounds like steps to verify should include checking that the #12 top crash in mentioned in bug 581020 goes away when the update gets pushed.
Comment 47 Benjamin Smedberg [:bsmedberg] 2010-07-23 07:47:28 PDT
*** Bug 580600 has been marked as a duplicate of this bug. ***
Comment 48 Mike Beltzner [:beltzner, not reading bugmail] 2010-07-23 09:01:00 PDT
I've lost track with all the comments, could someone please confirm:

 - we do expect the <object> and <embed> pattern to cause crashes on 1.9.1.11
 - we do not expect those crashes to be exploitable

If that's the case, we'll probably want to expedite a 1.9.1.12 as well, right?
Comment 49 [On PTO until 6/29] 2010-07-23 09:49:05 PDT
We've seen no crashes with 1.9.1.11 as far as I know.
Comment 50 [On PTO until 6/29] 2010-07-23 10:06:35 PDT
Verified with 1.9.2.8 on Ubuntu 10.4, OS X 10.6.4, and Windows 7 Pro that the attached testcase and http://itunes.apple.com/us/podcast/apple-keynotes/id275834665 do not crash anymore.
Comment 51 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-07-23 10:51:38 PDT
(In reply to comment #48)
> I've lost track with all the comments, could someone please confirm:
> 
>  - we do expect the <object> and <embed> pattern to cause crashes on 1.9.1.11

It's possible, but only theoretical at this point.

>  - we do not expect those crashes to be exploitable

If the theoretical problem arises in the field, it'll result in a NULL pointer deref, and that's extremely hard to exploit.
Comment 52 Benjamin Smedberg [:bsmedberg] 2010-07-23 12:04:34 PDT
*** Bug 581520 has been marked as a duplicate of this bug. ***
Comment 53 christian 2010-07-23 15:36:50 PDT
Just FYI, people are pointing out that the query in the 3.6.8 release notes returns no bugs. Is it safe to open this bug up yet?
Comment 54 Kohei Yoshino (was: yoshino@mozilla-japan.org) 2010-07-24 08:34:40 PDT
The 3.6.8 release notes should point
http://www.mozilla.org/security/announce/2010/mfsa2010-48.html
instead of the bug list.
Comment 55 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-07-24 16:00:16 PDT
Comment on attachment 459544 [details] [diff] [review]
1.9.1 fix v1.0

Josh, do you mind if I go ahead and land this on 1.9.0 when I'm landing the bugs for which I requested 1.9.0 approval?

(In reply to comment #33)
> I forgot to add the bug # in the comment, sorry. I don't know how to correct
> that, let me know if there is a good way to do it.

FYI, I wrote a basic hook that enforces that when committing: http://hg.mozilla.org/users/alqahira_ardisson.org/misc/file/tip/bugnum  (One day I'll remember to blog about it.)
Comment 56 Mike Beltzner [:beltzner, not reading bugmail] 2010-07-26 06:36:39 PDT
Can this bug be opened if it's fixed on 1.9.2? AIUI, it's not exploitable on 1.9.1, correct?
Comment 57 Josh Aas 2010-07-26 17:38:09 PDT
Smokey - fine with me to put this on 1.9.0.
Comment 58 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-07-30 12:06:08 PDT
Checking in layout/generic/nsObjectFrame.cpp;
/cvsroot/mozilla/layout/generic/nsObjectFrame.cpp,v  <--  nsObjectFrame.cpp
new revision: 1.661; previous revision: 1.660
done
Comment 59 Mike Beltzner [:beltzner, not reading bugmail] 2010-08-03 13:24:35 PDT
Did the testcase here get checked in? I don't see a checkin comment.

Note You need to log in before you can comment on or make changes to this bug.