365986 - Bad sample code for NPAPI crashes Firefox on Windows Vista

Status: RESOLVED INCOMPLETE

(Core Graveyard :: Plug-ins, defect)

Description

[Robert E. Burke]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.1.4322; .NET CLR 2.0.50727)
Build Identifier: Firefox/1.5 and earlier

On this page: http://www.mozilla.org/projects/plugins/
there is a download link for this tar: http://download.developer.beonex.com/mozilla/npruntime.tgz

In that tar, there is a file, plugin.cpp, that has two lines, 347 and 360, which are:

    STRINGZ_TO_NPVARIANT(strdup("foo return val"), *result);

and

  STRINGZ_TO_NPVARIANT(strdup("default method return val"), *result);

In both cases, "result" is a pointer to an NPVariant.  The problem here is using strdup, which copies a string into a newly allocated buffer.  It should instead use NPN_MemAlloc to allocate a buffer, then copy the string into that buffer, then use that buffer in the STRINGZ_TO_NPVARIANT macro.  This is because the NPVariant will eventally be released via NPN_ReleaseVariantValue, which in turn calls NPN_FreeMem on NPVariants of type NPVARIANTTYPE_STRING.  NPN_MemAlloc and NPN_FreeMem are correctly matched allocation/deallocation routines, while strdup and NPN_FreeMem are not.

The bad thing about this is that it appears to work fine on Windows 2000 and Windows XP, but if someome writes plugin code like this, it will crash Firefox on Windows Vista with an access violation.

I am sure I am not the only one who used the sample code as the "correct" way to code an NPAPI plugin.  The sample code should be fixed, or the link to it removed.  It would be great to have some more comprehensive (and correct) sample code for using NPAPI.

Reproducible: Always

Steps to Reproduce:
1. Create an NPAPI plugin with a method or property that reurns a string.
2. In returning the NPVariant for that string, use something like: STRINGZ_TO_NPVARIANT(strdup("my string"), *result);
3. Run plugin in Firefox on Vista, invoke said propery or method.
Actual Results:  
Firefox crashes with an access violation.

Expected Results:  
The sample code should reflect the correct way to return a string NPVariant.

Comment 1

[timeless]

interesting. however, that file while linked to from mozilla.org doesn't appear to be part of our cvs repository.

Comment 2

[Ben Bucksch (:BenB)]

heh, thanks for the note. I didn't remember putting the link there. It was at a time when the source was not available at mozilla.org, not even in CVS. It now is in CVS. I can't confirm the crash, but the fact that the page is linking to the correct source. That needs to be fixed.

Robert (reporter), there should be sample source for npruntime plugins in the Mozilla source code, IIRC in modules/plugin/ somewhere. Can you try that and report whether it has the same crash.

Comment 3

[Robert E. Burke]

I downloaded ftp://ftp.mozilla.org/pub/mozilla.org/mozilla/releases/mozilla1.8b1/source/mozilla-source-1.8b1.tar.gz.  I could find no references to STRINGZ_TO_NPVARIANT other than in npruntime.h, so I am not sure where the NPAPI samples are.

Comment 4

[Ben Bucksch (:BenB)]

In fact, I just checked, and it seems it's *still* (2 years later) not in the source tree. That's bug 254980, making blocker. See also bug 283878 (not sure if that's a duplication).

Comment 5

[Ben Bucksch (:BenB)]

FWIW, it would be at one of these locations:
http://lxr.mozilla.org/mozilla/source/modules/plugin/samples/
http://lxr.mozilla.org/mozilla/source/modules/plugin/tools/sdk/samples/

Comment 6

[Ben Bucksch (:BenB)]

Oh, and, (sorry for the spam), don't let yourself be confused about "scriptable" etc. samples, these are old style APIs. Look for a directory called "npruntime". I'd hope. That's what the patch in bug 254980 adds.

Comment 7

[Robert E. Burke]

I did not find any npapi code in the locations you mentioned.

The patch in attachment https://bugzilla.mozilla.org/attachment.cgi?id=213723 looks good to me, at least as far as properly allocating string NPVariants.  

It might be nice to have a macro that actually properly allocates the string buffer, copies the source string into it, and assigns it to the NPVariant, so I 
can write 

STRINGZ_DUPLICATE_TO_NPVARIANT(buffer, *result);

instead of:

uint32_t len = strlen(buffer) + 1;
char *copy = (char*)NPN_MemAlloc(len);
strncpy(copy, buffer, len);
STRINGZ_TO_NPVARIANT(copy, *result);

I know I can write my own macro or function to do this, but seeing how *everyone* will need to do this to properly return string NPVariants from properties and methods, I think it is a good candidate to be pushed into the framework.

Comment 8

[Nilo Menezes]

I just built the Npruntime sample plugin with Visual Studio 8 SP1. It works with Minefield and Opera, but crashed with Firefox 2.0.0.1

I'm testing with Windows XP

Comment 9

[Nilo Menezes]

I downloaded the Gecko SDK from Mozilla's Website. Now, the same code works with Firefox 2, Opera and MineField.
I think there is something wrong with the headers files of the version in the trunk.

I hope this helps.

Comment 10

[Benjamin Smedberg]

Resolving old bugs which are likely not relevant any more, since NPAPI plugins are deprecated.