1308023 - Crash in strcmp | PluginInfoIsFlash

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

This bug was filed from the Socorro interface and is 
report bp-55459599-2bc0-4878-924c-2aa142161005.
=============================================================

Regression from bug 1269807. Looks like [1] needs a null check.

[1] https://hg.mozilla.org/mozilla-central/annotate/ea104eeb14cc/dom/plugins/base/nsPluginHost.cpp#l2099

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: Null check info.fDescription in PluginInfoIsFlash.

For me, it crashes with the following plugin, that has no description.

  fName: Wacom Pressure Plug-In
  fDescription: (null)
  fVariantCount: 0
  fFileName: WacomNetscape.plugin
  fFullPath: /Library/Internet Plug-Ins/WacomNetscape.plugin
  fVersion: 2.1.0-1
  fSupportsAsyncRender: 0

https://crash-stats.mozilla.com/report/index/1995a541-790f-4ad6-a496-823f32161006

https://dxr.mozilla.org/mozilla-central/rev/e8fa13708c070d1fadf488ed9d951464745b4e17/dom/plugins/base/nsPluginsDirDarwin.cpp#453
>   if (bundle) {
>     CFTypeRef description = ::CFBundleGetValueForInfoDictionaryKey(bundle, CFSTR("WebPluginDescription"));
>     if (description && ::CFGetTypeID(description) == ::CFStringGetTypeID())
>       info.fDescription = CFStringRefToUTF8Buffer(static_cast<CFStringRef>(description));
>   }

According to the code, info.fDescription can be null.
So just added null check before strcmp.

Comment 2

[Benjamin Smedberg]

Will be fixed by bug 1307501: this should have been fName instead of fDescription anyway.

Comment 3

[Tooru Fujisawa [:arai]]

nice to hear that :)

Comment 4

[Marcia Knous [:marcia]]

Adding the Mac signature - I filed Bug 1308182 but I guess it is a dupe of this bug.

Comment 6

[Benjamin Smedberg]

Fixed by bug 1307501.

Comment 7

[Frank Burleigh]

I continue to see this crash in startup of 10/9/2016 Nightly: _platform_strcmp. My most recent crash:
https://crash-stats.mozilla.com/report/index/6a83e4a4-272a-4fb1-b50e-be14c2161009

I do have a bunch of plugins, including the latest Flash, a Shockwave, and the Wacom Pressure Plugin referenced above (all "Never activate" or "Ask to activate"). Creating a fresh profile doesn't help.

Additionally, 10/6/2016 Nightly cannot open using fresh profiles made with Nightlies after it.

Comment 8

[Tooru Fujisawa [:arai]]

reopening per comment #17, and bug 1308770.

according to the code, info.fName is also not-guaranteed to be non-null, at least on windows and osx


https://dxr.mozilla.org/mozilla-central/rev/d72cf6ecebaf707c159f6697d9f5f6f6a6feb7e1/dom/plugins/base/nsPluginsDirWin.cpp#375
>     info.fName = GetKeyValue(verbuf, L"ProductName", lang, cp);

https://dxr.mozilla.org/mozilla-central/source/dom/plugins/base/nsPluginsDirWin.cpp#69-110
> static char* GetKeyValue(void* verbuf, const WCHAR* key,
>                          UINT language, UINT codepage)
> {
>   WCHAR keybuf[64]; // plenty for the template below, with the longest key
>                     // we use (currently "FileDescription")
>   const WCHAR keyFormat[] = L"\\StringFileInfo\\%04X%04X\\%ls";
>   WCHAR *buf = nullptr;
>   UINT blen;
> 
>   if (_snwprintf_s(keybuf, ArrayLength(keybuf), _TRUNCATE,
>                    keyFormat, language, codepage, key) < 0)
>   {
>     NS_NOTREACHED("plugin info key too long for buffer!");
>     return nullptr;
>   }
> 
>   if (::VerQueryValueW(verbuf, keybuf, (void **)&buf, &blen) == 0 ||
>       buf == nullptr || blen == 0)
>   {
>     return nullptr;
>   }
> 
>   return PL_strdup(NS_ConvertUTF16toUTF8(buf, blen).get());
> }


https://dxr.mozilla.org/mozilla-central/rev/d72cf6ecebaf707c159f6697d9f5f6f6a6feb7e1/dom/plugins/base/nsPluginsDirDarwin.cpp#446
>   // Get fName
>   if (bundle) {
>     CFTypeRef name = ::CFBundleGetValueForInfoDictionaryKey(bundle, CFSTR("WebPluginName"));
>     if (name && ::CFGetTypeID(name) == ::CFStringGetTypeID())
>       info.fName = CFStringRefToUTF8Buffer(static_cast<CFStringRef>(name));
>   }


So, null-check should be there.

Comment 9

[Tooru Fujisawa [:arai]]

(In reply to Tooru Fujisawa [:arai] from comment #8)
> reopening per comment #17, and bug 1308770.

I meant comment #7

Comment 10

[Tooru Fujisawa [:arai]]

Attachment: Null check info.fName in PluginInfoIsFlash.

Just same as previous patch, but now checking info.fName instead of info.fDescription.

Comment 12

[Frank Burleigh]

Since at least one of us experiences this startup crash on OS X, shouldn't "platform" be adjusted?

Comment 13

[Andrew McCreight [:mccr8]]

[Tracking Requested - why for this release]: This is currently in the top 5 crashes for Nightly, and that's with another very frequent new regression.

Comment 14

[Andrew McCreight [:mccr8]]

If bsmedberg can't get to this review right away, maybe jimm or somebody could review it instead? It would be nice to have this small patch to fix a Nightly top crash.

Comment 15

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0ca22c468bbe3be6f6e40b611893483f6b0ab0c7
Bug 1308023 - Null check info.fName in PluginInfoIsFlash. r=jimm

Comment 16

[Marcia Knous [:marcia]]

Tracking 52+ - top crash for nightly that will need to be addressed.

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0ca22c468bbe