610525 - JavaScript-to-Java (nsISecureEnv) calls sometimes made with no JavaScript on stack

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Steven Michaud [:smichaud] (Retired)]

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 0.9.7.4)
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 0.9.7.4 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
assumption.

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
comment.

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)]

Attachment: Fix

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
stack.

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
stack).

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
constructor.

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Is this the security hole exposed by the logging from the site breakage in bug 607678?

Comment 3

[Steven Michaud [:smichaud] (Retired)]

> Is this the security hole exposed by the logging from the site
> breakage in bug 607678?

Yes.

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)]

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.

http://people.mozilla.com/~stmichaud/bmo/firefox-3.6.13pre-bugzilla610525.en-US.mac.dmg

Comment 5

[Steven Michaud [:smichaud] (Retired)]

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)]

JEP 0.9.7.4 "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 0.9.7.3 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)]

I think we should reclassify this bug as "[sg:low]".

Comment 8

[Daniel Veditz [:dveditz]]

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)]

> but we still need to fix it because it's the fix for
> regressions introduced by bug 598453

Yes.

Comment 10

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 489033 [details] [diff] [review]
Fix

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)]

Comment on attachment 489033 [details] [diff] [review]
Fix

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.

r=jst

Comment 12

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 489033 [details] [diff] [review]
Fix

Landed on the 1.9.2 branch with jst's requested changes:
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/f1ae505ad72d

Comment 13

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 489033 [details] [diff] [review]
Fix

Landed on the 1.9.1 branch:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ff0b80c67371

Comment 14

[Steven Michaud [:smichaud] (Retired)]

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)]

Comment on attachment 489033 [details] [diff] [review]
Fix

> 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)]

Yes, thanks.

Comment 17

[Al Billings [:abillings - ex-MoCo]]

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)]

> 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
   (http://jnlp.dev.concord.org/test-japplet.html).

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

   WINDOW IS: [object DOMWindow]