Closed Bug 1308023 Opened 5 years ago Closed 5 years ago

Crash in strcmp | PluginInfoIsFlash

Categories

(Core :: Plug-ins, defect)

Unspecified
All
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox49 --- unaffected
firefox50 --- unaffected
firefox51 --- unaffected
firefox52 + fixed

People

(Reporter: jchen, Assigned: arai)

References

Details

(Keywords: crash, regression, topcrash)

Crash Data

Attachments

(1 file, 1 obsolete file)

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
Flags: needinfo?(benjamin)
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.
Assignee: nobody → arai.unmht
Attachment #8798370 - Flags: review?(benjamin)
Will be fixed by bug 1307501: this should have been fName instead of fDescription anyway.
Depends on: 1307501
Flags: needinfo?(benjamin)
Attachment #8798370 - Flags: review?(benjamin)
nice to hear that :)
Assignee: arai.unmht → nobody
Adding the Mac signature - I filed Bug 1308182 but I guess it is a dupe of this bug.
Crash Signature: [@ strcmp | PluginInfoIsFlash] → [@ strcmp | PluginInfoIsFlash] [@ _platform_strcmp]
Version: unspecified → Trunk
Duplicate of this bug: 1308182
Crash Signature: [@ strcmp | PluginInfoIsFlash] [@ _platform_strcmp] → [@ strcmp | PluginInfoIsFlash] [@ _platform_strcmp] [@ __platform_strcmp]
Crash Signature: [@ strcmp | PluginInfoIsFlash] [@ _platform_strcmp] [@ __platform_strcmp] → [@ strcmp | PluginInfoIsFlash] [@ PluginInfoIsFlash] [@ _platform_strcmp] [@ __platform_strcmp]
Fixed by bug 1307501.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
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.
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
See Also: → 1308770
(In reply to Tooru Fujisawa [:arai] from comment #8)
> reopening per comment #17, and bug 1308770.

I meant comment #7
Just same as previous patch, but now checking info.fName instead of info.fDescription.
Assignee: nobody → arai.unmht
Attachment #8798370 - Attachment is obsolete: true
Status: REOPENED → ASSIGNED
Attachment #8799299 - Flags: review?(benjamin)
Duplicate of this bug: 1308770
Since at least one of us experiences this startup crash on OS X, shouldn't "platform" be adjusted?
OS: Windows 10 → All
[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.
Keywords: topcrash
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.
Attachment #8799299 - Flags: review?(benjamin) → review+
Tracking 52+ - top crash for nightly that will need to be addressed.
https://hg.mozilla.org/mozilla-central/rev/0ca22c468bbe
Status: ASSIGNED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.