715693 - Prevent plugins from using JSAPI incorrectly

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[Brian Hackett [Laid off!]]

As illustrated in the dependent bugs, C++ plugins are free to call JSAPI methods directly, without entering the right compartment and without hitting any (debug only) same compartment assertions.  Doing JS code while in the wrong compartment is really bad and can cause crashes either shortly or much later, and if a plugin does this it should either crash the browser immediately or (even better) not be possible in the first place.  This could be done either by using release mode same compartment assertions in public APIs, or by finding a way to hide unsafe API symbols from plugins.

Comment 1

[Boris Zbarsky [:bzbarsky]]

The problem is that any symbol visible to libxul is also visible to plugins, I'd think, since they link to the JS engine in pretty much the same way.

If we linked JS into libxul we could possibly make those symbols hidden visibility, right?

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Boris Zbarsky (:bz) from comment #1)
> The problem is that any symbol visible to libxul is also visible to plugins,
> I'd think, since they link to the JS engine in pretty much the same way.

Right.

> If we linked JS into libxul we could possibly make those symbols hidden
> visibility, right?

Right.  But we can't do that on Windows ...

Comment 3

[Brian Hackett [Laid off!]]

(In reply to Boris Zbarsky (:bz) from comment #1)
> The problem is that any symbol visible to libxul is also visible to plugins,
> I'd think, since they link to the JS engine in pretty much the same way.
> 
> If we linked JS into libxul we could possibly make those symbols hidden
> visibility, right?

An alternative discussed over IRC was to only export one symbol from JS, which is a function returning a big dispatch table which libxul would load into global function pointers on startup.  If the libraries are dynamically linked I would (wildly) guess that invoking a function pointer will be at least as fast as doing a cross library call.  The function exported by JS could be structured such that it can only be called once (presumably by libxul, else things will horribly break immediately), which should keep plugins from getting direct access to JSAPI.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Another option is to add private JS apis that Gecko itself would use that are visible and make the public APIs that the plug-ins are presumably using hidden-visibility in the Firefox libmozjs builds.  Of course if we no longer use those APIs from Gecko, we could also just make them use runtime compartment checks and leave them visible...  This would work unless the plug-ins started using said private APIs.

I wonder what the effect is on the plug-in of not being able to find the symbols in question.  I guess it depends on whether they link to them directly or whether they dlopen stuff.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Something we discussed on IRC today was moving all of our shared libs into the omnijar and using our own dynamic linker on all platforms.  That would keep anyone from poking at our external symbols.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Another though from IRC discussion: do something like cairo-rename.h, but for various other APIs and generated at compile time if the right configure flag is set, with the RHS of the #define randomly-generated to some extent.  How much of an extent depends on how painful we want breakpad integration to be.  This would change the symbol names in the libraries we actually ship, and in particular require reverse-engineering, not just compiling against our headers, to link against them.  Won't stop someone determined to link against us, but might ring some "I shouldn't be doing this" alarm bells, especially if we make the generated symbol names include "dont_link_to_me" or something.

Comment 7

[Benjamin Smedberg]

In the past we have explicitly decided to *allow* binaries to link against our JSAPI, so that they could use/implement special xpconnect "stuff", including the new jsval IDL type.

Obviously using JSAPI is "hard" and many people will get it wrong, but frankly many people get XPCOM wrong also, so I'm not sure why this concern is different than any other concern we have with binary components being loaded into our process.

I don't think that making it impossible to use just the JSAPI is the right solution. We should either remove support for binary components entirely, or understand that its very valuable but potentially unsafe. We've gradually been working on making XPCOM APIs "safer" by adding runtime threading checks to the prefservice, for example. Perhaps we should be doing the same for JSAPI if there isn't a noticeable performance penalty?

Comment 8

[Brendan Eich [:brendan]]

We don't yank binary component APIs on short or zero notice, without product-level thinking (partners or third parties we care about use these). Anyway, APIs can always be abused as bsmedberg says -- this isn't reason to yank, by itself.

We added new rules to the JS API. Did we doc? Deprecate the old APIs? Add safeties to the old entry points? We can well afford to. We probably can't afford to throw third parties under the bus in the current market.

/be

Comment 9

[Tom S [:evilpie]]

Plugin support was removed.