| Summary: | Crash on some pages due to a plugin | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Pascal Terjan <pterjan> | ||||
| Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | alp, zuh | ||||
| Priority: | P2 | Keywords: | Gtk | ||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | PC | ||||||
| OS: | Linux | ||||||
| Attachments: |
|
||||||
|
Description
Pascal Terjan
2008-08-03 04:46:45 PDT
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.
(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? (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 ;) Comment on attachment 22892 [details]
Fix handling of badly formatted and empty plugin mime descriptions
r=me
|