Beginning on October 25th, 2016, Persona will no longer be an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 610525 - JavaScript-to-Java (nsISecureEnv) calls sometimes made with no JavaScript on stack
: JavaScript-to-Java (nsISecureEnv) calls sometimes made with no JavaScript on ...
[sg:low] [qa-needs-STR]
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: 1.9.2 Branch
: All All
: -- normal (vote)
: ---
Assigned To: Steven Michaud [:smichaud] (Retired)
: Benjamin Smedberg [:bsmedberg]
Depends on:
Blocks: 606737 607678
  Show dependency treegraph
Reported: 2010-11-08 16:15 PST by Steven Michaud [:smichaud] (Retired)
Modified: 2011-01-27 17:30 PST (History)
5 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Fix (15.65 KB, patch)
2010-11-08 16:48 PST, Steven Michaud [:smichaud] (Retired)
jst: review+
Details | Diff | Splinter Review

Description Steven Michaud [:smichaud] (Retired) 2010-11-08 16:15:12 PST
This is a problem in OJI and browser-side LiveConnect, which is
present on the current live branches (1.9.2 and 1.9.1), but no longer
on the trunk.

First some background:

nsISecureEnv is an interface implemented by OJI plugins to allow calls
to be made from JavaScript to Java (JavaScript-to-Java LiveConnect
calls).  OJI plugins (both Sun's and the JEP before version
assume that nsISecureEnv calls will always be made with JavaScript on
the stack, because they use the current script principal's origin to
configure the Java code's security environment.

(The JEP uses nsIScriptSecurityManager::GetSubjectPrincipal() to get
the current script's principal, then calls nsIPrincipal::GetOrigin()
to get its origin.)

If by some chance there is no current script principal (if there is no
JavaScript on the stack), the JEP before version assumed that
the Java code being called via the nsISecureEnv interface should run
with unlimited privileges.  And I suspect even the most recent
versions of Sun's OJI plugin for Windows and Linux also make this

nsISecureEnv methods (used for JavaScript-to-Java calls) are normally
called with JavaScript on the stack.  But there's a bug in the
implementation of the nsILiveconnect API (used for Java-to-JavaScript
calls) that can cause nsISecureEnv methods to inadvertently be called
without any JavaScript on the stack.

That's this bug -- which I'll explain in more detail in my next

So this bug is potentially a security hole.  But (as I'll also explain
in a later comment) I strongly doubt there's any way to exploit it.
Comment 1 Steven Michaud [:smichaud] (Retired) 2010-11-08 16:48:19 PST
Created attachment 489033 [details] [diff] [review]

Java-to-JavaScript calls (made using the nsILiveconnect API) can be
made "spontaneously" -- from Java code that wasn't called from
JavaScript code, and therefore when there is no JavaScript on the

To get around this, nsCLiveconnect.cpp's implementations of the
nsILiveconnect methods create an AutoPushJSContext object.  This
object's constructor checks for JavaScript on the stack, and if none
is found fakes it (by adding a dummy frame with a principal to the

The AutoPushJSContext constructor takes a JSContext parameter.
Normally we only learn after a call to jsj_enter_js() which JSContext
we need to use.  But the first time jsj_enter_js() is called, it
initializes a bunch of Java classes, which requires the use of
nsISecureEnv (JavaScript-to-Java) methods.  And if this first call to
jsj_enter_js() happens during an nsILiveconnect (Java-to-JavaScript)
call, there will be no JavaScript on the stack.

My patch gets around this problem by creating the AutoPushJSContext
object before the call to jsj_enter_js().  It uses a new function
(JSContextForPluginInstance()) to find out which JSContext will be
returned by jsj_enter_js(), and passes this to the AutoPushJSContext
Comment 2 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-11-08 19:31:03 PST
Is this the security hole exposed by the logging from the site breakage in bug 607678?
Comment 3 Steven Michaud [:smichaud] (Retired) 2010-11-08 19:50:20 PST
> Is this the security hole exposed by the logging from the site
> breakage in bug 607678?


I'm not yet finished writing up this bug.

Tomorrow I'll add more comments, post some builds made with my patch,
and tell people about them at bug 607678.
Comment 4 Steven Michaud [:smichaud] (Retired) 2010-11-09 16:02:36 PST
Here's a test build made with my patch from comment #1.  Since the
tryservers no longer do branch builds, this is a build I made myself.
Comment 5 Steven Michaud [:smichaud] (Retired) 2010-11-09 16:45:43 PST
I've confirmed this bug's security hole also effects the 1.9.1 branch
on Linux (and presumably Windows) -- though only when using Sun's old
OJI plugin.

Interestingly, the proximate cause (on Linux and Windows) is actually
in our OJI code -- the nsCSecurityContext constructors assume that no
script principal (i.e. no JavaScript on the stack) means that Java
code should run with full privileges.  And this assumption may not be
"wrong" -- nsScriptSecurityManager::SubjectPrincipalIsSystem() does
the same thing.
Comment 6 Steven Michaud [:smichaud] (Retired) 2010-11-09 17:06:39 PST
JEP "fixed" this bug's security hole on OS X in FF versions
since 3.6.11 and 3.5.14 (and in Camino 2.0.5).  (Though it also
triggered the breakage reported in bug 606737 and bug 607678.)

But as I mentioned in comment #0, it's very unlikely that this bug's
security hole is exploitable -- even using OS X distros that bundle
JEP or earlier, or using the OJI plugin in FF 3.5.X on Linux
and Windows.

The only Java code called (via the nsISecureEnv interface) with full
privileges is code that's called from jsj_enter_js().  As best I can
tell, jsj_enter_js() doesn't call any Java code except the first time
it's run (to help initialize Java-to-JavaScript LiveConnect).  This
code does nothing but load commonly used classes and find their fields
and methods (most of it is in jsj.c's init_netscape_java_classes() and
init_java_VM_reflection()) -- which is entirely harmless.  And there
doesn't seem to be any way for "the user" to run Java code of his/her
own choice.
Comment 7 Steven Michaud [:smichaud] (Retired) 2010-11-09 17:07:38 PST
I think we should reclassify this bug as "[sg:low]".
Comment 8 Daniel Veditz [:dveditz] 2010-11-10 10:36:34 PST
OK, sg:low it is -- but we still need to fix it because it's the fix for regressions introduced by bug 598453
Comment 9 Steven Michaud [:smichaud] (Retired) 2010-11-10 10:43:56 PST
> but we still need to fix it because it's the fix for
> regressions introduced by bug 598453

Comment 10 Steven Michaud [:smichaud] (Retired) 2010-11-10 13:50:08 PST
Comment on attachment 489033 [details] [diff] [review]

I don't really know who should review my patch -- OJI and
(browser-side) LiveConnect are (infamously) obsolete and un-owned.

Dbaron:  I noticed your name on some of the revisions to the
AutoPushJSContext code, so I'm asking you to review.  Please let me
know if you think someone else should do it.
Comment 11 Johnny Stenback (:jst, 2010-11-11 15:03:56 PST
Comment on attachment 489033 [details] [diff] [review]

JSContextForPluginInstance() could use nsCOMPtr's instead of doing manual reference counting, but if that's for some reason hard I'm fine with doing it by hand here, as the patch does.

Comment 12 Steven Michaud [:smichaud] (Retired) 2010-11-15 08:25:02 PST
Comment on attachment 489033 [details] [diff] [review]

Landed on the 1.9.2 branch with jst's requested changes:
Comment 13 Steven Michaud [:smichaud] (Retired) 2010-11-15 09:03:36 PST
Comment on attachment 489033 [details] [diff] [review]

Landed on the 1.9.1 branch:
Comment 14 Steven Michaud [:smichaud] (Retired) 2010-11-15 09:07:20 PST
Smokey, does this need to land on the 1.9.0 branch?

I assume that it does, but please let us know (so I can request approval).
Comment 15 Steven Michaud [:smichaud] (Retired) 2010-11-15 09:16:07 PST
Comment on attachment 489033 [details] [diff] [review]

> Smokey, does this need to land on the 1.9.0 branch?

I've answered my own question -- this patch does need to land on the 1.9.0 branch, since current Camino releases are off that branch, and Camino 2.0.5 is effected by this bug (and bug 606737 and bug 607678).
Comment 16 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-11-15 14:34:58 PST
Yes, thanks.
Comment 17 Al Billings [:abillings] 2010-11-17 16:23:46 PST
What are the STR for this bug for verification of this fix on 1.9.2 and 1.9.1?
Comment 18 Steven Michaud [:smichaud] (Retired) 2010-11-17 16:58:30 PST
> What are the STR for this bug for verification of this fix on 1.9.2
> and 1.9.1?

The following has worked best for me:

1) Run Applications : Utilities : Java Preferences and

   a) Click on the Advanced tab.
   b) Under Java Console select Show console.

2) Load the testcase from bug 606737

3) Look for the following in the Java Console output:

   WINDOW IS: [object DOMWindow]

Note You need to log in before you can comment on or make changes to this bug.