Bug 20267 – Crash on some pages due to a plugin

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Pascal Terjan]

I get a crash on http://www.mandriva.com/en/community and someone reported the same on another page on http://bugzilla.gnome.org/show_bug.cgi?id=542804

I get it both with midori and epiphany using r35417, the following trace is from epiphany.

#0  0xb6717bfb in strlen () from /lib/i686/libc.so.6
No symbol table info available.
#1  0xb7818a0e in WebCore::String::fromUTF8 (string=0x696469 <Address 0x696469 out of bounds>) at WebCore/platform/text/String.cpp:590
No locals.
#2  0xb7a568e7 in WebCore::PluginPackage::fetchInfo (this=0xb3365000) at WebCore/plugins/gtk/PluginPackageGtk.cpp:78
	mimeData = (gchar **) 0x8e31280
	description = {m_impl = {m_ptr = 0xb33af708}}
	extensions = (gchar **) 0x8e34058
	extVector = {m_size = 0, m_buffer = {<WTF::VectorBufferBase<WebCore::String>> = {<WTFNoncopyable::Noncopyable> = {<No data fields>}, 
      m_buffer = 0x0, m_capacity = 16}, <No data fields>}}
	NP_GetMIMEDescription = (NP_GetMIMEDescriptionFuncPtr) 0xb3104c80 <NP_GetMIMEDescription>
	NPP_GetValue = (NPP_GetValueProcPtr) 0xb3104c50 <NP_GetValue>
	mimeDescs = (gchar **) 0x8e33fd8
	buffer = 0x1c0 <Address 0x1c0 out of bounds>
	err = 25705
#3  0xb783275c in WebCore::PluginPackage::createPackage (path=@0xb38ba3c8, lastModified=@0xbf86e3bc) at WebCore/plugins/PluginPackage.cpp:149
No locals.
#4  0xb782bbdf in WebCore::PluginDatabase::refresh (this=0xb4aa9ea0) at WebCore/plugins/PluginDatabase.cpp:109
	lastModified = 1215766805
	package = {m_ptr = 0x0}
	pluginSetChanged = false
	paths = {m_impl = {static m_minTableSize = <optimized out>, static m_maxLoad = <optimized out>, static m_minLoad = <optimized out>, 
    m_table = 0xb38ba300, m_tableSize = 64, m_tableSizeMask = 63, m_keyCount = 7, m_deletedCount = 0}}
	pathsWithTimes = {m_impl = {static m_minTableSize = <optimized out>, static m_maxLoad = <optimized out>, 
    static m_minLoad = <optimized out>, m_table = 0xb33b5400, m_tableSize = 64, m_tableSizeMask = 63, m_keyCount = 5, m_deletedCount = 0}}
#5  0xb782c9a9 in WebCore::PluginDatabase::installedPlugins () at WebCore/plugins/PluginDatabase.cpp:44
	plugins = (WebCore::PluginDatabase *) 0xb4aa9ea0
#6  0xb7a55ae7 in WebCore::PluginData::initPlugins (this=0xb332f080) at WebCore/plugins/gtk/PluginDataGtk.cpp:32
	db = (WebCore::PluginDatabase *) 0x696469
#7  0xb78289ed in PluginData (this=0xb332f080, page=0xb4aa8d90) at WebCore/plugins/PluginData.cpp:32
No locals.
#8  0xb77d5c42 in WebCore::Page::pluginData (this=0xb4aa8d90) at WebCore/plugins/PluginData.h:49
No locals.
#9  0xb7827bec in WebCore::MimeTypeArray::getPluginData (this=0xb34fb558) at WebCore/plugins/MimeTypeArray.cpp:91
	p = (class WebCore::Page *) 0x696469
#10 0xb7827c0f in WebCore::MimeTypeArray::length (this=0xb34fb558) at WebCore/plugins/MimeTypeArray.cpp:41
	data = (WebCore::PluginData *) 0x8df99a8
#11 0xb7b17b4f in WebCore::JSMimeTypeArray::getValueProperty (this=0xb35e25e0, exec=0xbf86ec9c, token=0)
    at DerivedSources/JSMimeTypeArray.cpp:112
No locals.
#12 0xb7c1a634 in KJS::Machine::privateExecute (this=0xb4b2bc40, flag=KJS::Machine::Normal, exec=0xbf86ec9c, registerFile=0xb4b2bc58, 
    r=0xb36191f8, scopeChain=0xb33ae350, codeBlock=0xb33b4000, exception=0xbf86ed00) at JavaScriptCore/kjs/PropertySlot.h:61
	dst = 0
	ident = (KJS::Identifier &) @0xb33b7ab4: {_ustring = {m_rep = {m_ptr = 0xb4b90a80}}}
	result = (class KJS::JSValue *) 0x8df99a8
	exceptionValue = (class KJS::JSValue *) 0x0
	handlerVPC = (class KJS::Instruction *) 0x0
	registerBase = (class KJS::Register *) 0xb3619000
	k = (class KJS::Register *) 0xb33b7a50
	tickCount = 250
#13 0xb7c20318 in KJS::Machine::execute (this=0xb4b2bc40, programNode=0xb33b1640, exec=0xb388d640, scopeChain=0xb385f4b0, thisObj=0xb3600000, 
    exception=0xbf86ed00) at JavaScriptCore/VM/Machine.cpp:735
	oldSize = 0
	newSize = <value optimized out>
	lastGlobalObject = (class KJS::JSGlobalObject *) 0xb3600020
	globalObject = (class KJS::JSGlobalObject *) 0xb3600020
	callFrame = <value optimized out>
	r = (class KJS::Register *) 0xb361902c
	newExec = {<WTFNoncopyable::Noncopyable> = {<No data fields>}, m_globalObject = 0xb3600020, m_globalThisValue = 0xb3600000, 
  m_exception = 0x0, m_globalData = 0xb4aa3320, m_prev = 0xb388d640, m_registerFile = 0xb4b2bc58, m_scopeChain = 0xb33ae350, 
  m_callFrame = 0xb36191c0}
	result = <value optimized out>
#14 0xb7ca2d82 in KJS::Interpreter::evaluate (exec=0xb388d640, scopeChain=@0xb38456dc, sourceURL=@0xbf86ed70, startingLineNumber=6906985, 
    source={m_ptr = 0x8df99a8}, thisValue=0xb3600000) at JavaScriptCore/kjs/interpreter.cpp:83
	sourceId = 10
	errLine = -1
	errMsg = {m_rep = {m_ptr = 0xb7f19700}}
	programNode = {m_ptr = 0xb33b1640}
	thisObj = (class KJS::JSObject *) 0x8df99a8
	exception = (class KJS::JSValue *) 0x0
	result = (class KJS::JSValue *) 0xb388d640
#15 0xb753f34f in WebCore::ScriptController::evaluate (this=0xb4ab0928, sourceURL=@0xbf86ee1c, baseLine=6906985, str=@0x696469)
    at WebCore/bindings/js/ScriptController.cpp:92
	exec = (class KJS::ExecState *) 0xb388d640
	savedSourceURL = (const WebCore::String *) 0x0
	lock = {<WTFNoncopyable::Noncopyable> = {<No data fields>}, m_lockingForReal = false}
#16 0xb774ff92 in WebCore::FrameLoader::executeScript (this=0xb4ab06a4, url=@0xbf86ee1c, baseLine=1, script=@0xbf86f048)
    at WebCore/loader/FrameLoader.cpp:790
	wasRunningScript = false
	result = <value optimized out>
#17 0xb7713963 in WebCore::HTMLTokenizer::scriptExecution (this=0xb4af8400, str=@0xbf86f048, state=
      {static EntityShift = <optimized out>, m_bits = 0}, scriptURL=@0xbf86f044, baseLine=1) at WebCore/html/HTMLTokenizer.cpp:546
	url = {m_impl = {m_ptr = 0xb4b1a348}}
	savedPrependingSrc = (WebCore::SegmentedString *) 0xbf86f120
	prependingSrc = {m_pushedChar1 = 0, m_pushedChar2 = 0, m_currentString = {m_length = 0, m_current = 0x0, m_string = {m_impl = {
        m_ptr = 0x0}}, m_doNotExcludeLineNumbers = true}, m_currentChar = 0x0, m_substrings = {m_start = 0, m_end = 0, 
    m_buffer = {<WTF::VectorBufferBase<WebCore::SegmentedSubstring>> = {<WTFNoncopyable::Noncopyable> = {<No data fields>}, m_buffer = 0x0, 
        m_capacity = 0}, <No data fields>}}, m_composite = false}
#18 0xb771b908 in WebCore::HTMLTokenizer::notifyFinished (this=0xb4af8400) at WebCore/html/HTMLTokenizer.cpp:1994
	cs = (class WebCore::CachedScript *) 0xb4b1d960
	scriptSource = {m_impl = {m_ptr = 0xb4b1a480}}
	errorOccurred = 72
	cachedScriptUrl = {m_impl = {m_ptr = 0xb4b1a348}}
	n = {m_ptr = 0xb4aa35a0}
	finished = false
#19 0xb7732093 in WebCore::CachedScript::addClient (this=0xb4b1d960, c=0xb4af8408) at WebCore/loader/CachedScript.cpp:58
No locals.
#20 0xb77163b0 in WebCore::HTMLTokenizer::scriptHandler (this=0xb4af8400, state={static EntityShift = <optimized out>, m_bits = 6906985})
    at WebCore/html/HTMLTokenizer.cpp:474
	savedRequestingScript = false
	doScriptExec = false
	followingFrameset = false
	cs = (class WebCore::CachedScript *) 0xb4b1d960
	scriptCode = {m_impl = {m_ptr = 0xb4aa42b8}}
	savedPrependingSrc = (WebCore::SegmentedString *) 0x0
	prependingSrc = {m_pushedChar1 = 0, m_pushedChar2 = 0, m_currentString = {m_length = 0, m_current = 0x0, m_string = {m_impl = {
        m_ptr = 0x0}}, m_doNotExcludeLineNumbers = true}, m_currentChar = 0x0, m_substrings = {m_start = 0, m_end = 0, 
    m_buffer = {<WTF::VectorBufferBase<WebCore::SegmentedSubstring>> = {<WTFNoncopyable::Noncopyable> = {<No data fields>}, m_buffer = 0x0, 
        m_capacity = 0}, <No data fields>}}, m_composite = false}
#21 0xb7716f6f in WebCore::HTMLTokenizer::parseSpecial (this=0xb4af8400, src=@0xb4af8d4c, state=
      {static EntityShift = <optimized out>, m_bits = 128}) at WebCore/html/HTMLTokenizer.cpp:334
	ch = 62
	lastDecodedEntityPosition = -1
#22 0xb771924c in WebCore::HTMLTokenizer::parseTag (this=0xb4af8400, src=@0xb4af8d4c, state=
      {static EntityShift = <optimized out>, m_bits = 148871592}) at WebCore/html/HTMLTokenizer.cpp:1517
	tagName = {m_string = {m_impl = {m_ptr = 0xb4ab7018}}}
	isSelfClosingScript = false
	beginTag = true
	n = {m_ptr = 0xb4aa35a0}
	cBufferPos = 0
	lastIsSlash = false
#23 0xb7719e47 in WebCore::HTMLTokenizer::write (this=0xb4af8400, str=@0xbf86f464, appendData=true) at WebCore/html/HTMLTokenizer.cpp:1735
	cc = 39336
	source = {m_pushedChar1 = 0, m_pushedChar2 = 0, m_currentString = {m_length = 0, m_current = 0x0, m_string = {m_impl = {m_ptr = 0x0}}, 
    m_doNotExcludeLineNumbers = true}, m_currentChar = 0x0, m_substrings = {m_start = 0, m_end = 0, 
    m_buffer = {<WTF::VectorBufferBase<WebCore::SegmentedSubstring>> = {<WTFNoncopyable::Noncopyable> = {<No data fields>}, 
        m_buffer = 0xb34c0688, m_capacity = 0}, <No data fields>}}, m_composite = false}
	wasInWrite = false
	processedCount = 2
	startTime = 1217763892.2606039
	frame = (class WebCore::Frame *) 0xb4aa60b8
#24 0xb7713760 in WebCore::HTMLTokenizer::timerFired (this=0xb4af8400) at WebCore/html/HTMLTokenizer.cpp:1814
No locals.
#25 0xb771bb0f in WebCore::Timer<WebCore::HTMLTokenizer>::fired (this=0xb4af851c) at WebCore/platform/Timer.h:99
No locals.
#26 0xb7826c39 in WebCore::TimerBase::fireTimers (fireTime=1217763889.214427, firingTimers=@0xbf86f534) at WebCore/platform/Timer.cpp:347
	timer = (class WebCore::TimerBase *) 0xb4af851c
	interval = 0
#27 0xb7826d23 in WebCore::TimerBase::sharedTimerFired () at WebCore/platform/Timer.cpp:368
	fireTime = 1217763889.214427
	firingTimers = {m_size = 6, 
  m_buffer = {<WTF::VectorBufferBase<WebCore::TimerBase*>> = {<WTFNoncopyable::Noncopyable> = {<No data fields>}, m_buffer = 0xb4aa2e80, 
      m_capacity = 16}, <No data fields>}}
	firingTimersSet = {m_impl = {static m_minTableSize = <optimized out>, static m_maxLoad = <optimized out>, 
    static m_minLoad = <optimized out>, m_table = 0xb4aac600, m_tableSize = 64, m_tableSizeMask = 63, m_keyCount = 0, m_deletedCount = 6}}
#28 0xb7a6d6ab in timeout_cb () at WebCore/platform/gtk/SharedTimerGtk.cpp:48
No locals.
#29 0xb684e7d0 in g_idle_dispatch (source=0x8e0ff60, callback=0x696469, user_data=0x0) at gmain.c:4173
No locals.
#30 0xb685079a in IA__g_main_context_dispatch (context=0x88ecac8) at gmain.c:2068
No locals.
#31 0xb6853eb8 in g_main_context_iterate (context=0x88ecac8, block=1, dispatch=1, self=0x88c7048) at gmain.c:2701
	max_priority = 0
	timeout = 0
	some_ready = 1
	nfds = 11
	allocated_nfds = <value optimized out>
	fds = (GPollFD *) 0x8958dc0
	__PRETTY_FUNCTION__ = "g_main_context_iterate"
#32 0xb68543cb in IA__g_main_loop_run (loop=0x8939ce8) at gmain.c:2924
	self = (GThread *) 0x88c7048
	__PRETTY_FUNCTION__ = "IA__g_main_loop_run"
#33 0xb6fd2b4f in IA__gtk_main () at gtkmain.c:1172
	tmp_list = (GList *) 0x27
	functions = (GList *) 0x0
	init = (GtkInitFunction *) 0x0
	loop = (GMainLoop *) 0x8939ce8
#34 0x080699ca in main (argc=Cannot access memory at address 0x1
) at ephy-main.c:742
	program = <value optimized out>
	option_context = <value optimized out>
	option_group = <value optimized out>
	proxy = <value optimized out>
	error = (GError *) 0x0
	user_time = 3213297796
	env = <value optimized out>
	enable_pango = <value optimized out>

[Kalle Vahlman]

Created attachment 22892 [details]
Fix handling of badly formatted and empty plugin mime descriptions

The backtrace looks to be the same crash I encountered with the new Maemo release (Diablo), which was due to the Nokia's browser plugin including a trailing '; ' in their return value for NP_GetMIMEDescription().

The GTK+ PluginPackage code first splits by ';', then by ':' and assumes that the latter always succeeds to find three elements and thus crashing when there's less.

The patch fixes it to only accept well-formatted (ie. three elements separated by ':' for each ';' block) mime descriptions.

[Alp Toker]

(In reply to comment #1)
> Created an attachment (id=22892) [edit]
> Fix handling of badly formatted and empty plugin mime descriptions
> 
> The backtrace looks to be the same crash I encountered with the new Maemo
> release (Diablo), which was due to the Nokia's browser plugin including a
> trailing '; ' in their return value for NP_GetMIMEDescription().
> 
> The GTK+ PluginPackage code first splits by ';', then by ':' and assumes that
> the latter always succeeds to find three elements and thus crashing when
> there's less.
> 
> The patch fixes it to only accept well-formatted (ie. three elements separated
> by ':' for each ';' block) mime descriptions. 
> 

I think this no longer crashes on trunk since String::fromUTF8() was changed to return a null WebCore string rather than crashing on null input a few days ago.

However, if g_strsplit() returns fewer than 3 elements the code will still be accessing mimeData[1] and mimeData[2] which may be pointing to uninitialized memory, so the fix is still a good idea.

The patch will skip over mime descs that have fewer than three elements while the version in PluginPackageWin looks like it will continue successfully if there is no description or extension list -- maybe this patch should be modified to cater for that condition too?

[Kalle Vahlman]

(In reply to comment #2)
> The patch will skip over mime descs that have fewer than three elements while
> the version in PluginPackageWin looks like it will continue successfully if
> there is no description or extension list -- maybe this patch should be
> modified to cater for that condition too?

I wonder if that's a common or legal practise...

I suppose omitting description would be tolerable, but what's the functionality if you don't specify extensions? Does it mean '*' implicitly? I couldn't find a good source of information on how the mime types actually should be handled.

Mozilla's docs just state the three fields, claim that the separator is ';' instead of ':' and do not mention omitting them:

http://developer.mozilla.org/en/Gecko_Plugin_API_Reference/Plug-in_Development_Overview#Unix

The other place where I could find mention of the format is here:

http://gplflash.sourceforge.net/gplflash2_blog/npapi.html#SEC9

but the only difference is that there the format seems to match what is really used.

I'd hesitate to allow badly formatted mime descriptions just based on the win port allowing it. After all, it's not too hard  to return 'my/mime::" instead of just "my/mime"... Though if "my/mime:*:" is equal in functionality to "my/mime", I see no point disallowing it either.

So I'll gladly remake the patch for that but I'd first like to hear a good reason for it ;)

[Darin Adler]

Comment on attachment 22892 [details]
Fix handling of badly formatted and empty plugin mime descriptions

r=me

[Mark Rowe (bdash)]

Landed in r35884.