[rfc][icedtea-web] Fix for PR1198, JSObject passed incorrectly to Javascript

[Jiri Vanek]

I think this one (3)  is little bit better.

J.

ps: ignore my previous  email. Its answer should be to this one.

On 01/07/2013 06:38 PM, Adam Domurad wrote:
> On 01/07/2013 11:44 AM, Jiri Vanek wrote:
>> On 01/07/2013 04:47 PM, Adam Domurad wrote:
>>> On 01/07/2013 09:53 AM, Jiri Vanek wrote:
>>>>> [.. rest snipped..]
>>>> Isn't missing updated  junit patch here?
>>>
>>> Sorry, was awaiting your advice -- if you did not like this security-manager-in-constructor change
>>> than the unit test would not be valid, a quite different approach would have to be used.
>>>
>>>>
>>>> Thanx for looking into it.
>>>> J.
>>>>
>>>
>>>
>>> patch attached,
>>> -Adam
>>>
>>> jsfix-unittest2.patch
>>>
>>>
>>> diff --git a/plugin/icedteanp/java/netscape/javascript/JSObjectUnboxPermission.java
>>> b/plugin/icedteanp/java/netscape/javascript/JSObjectUnboxPermission.java
>>> new file mode 100644
>>> --- /dev/null
>>> +++ b/plugin/icedteanp/java/netscape/javascript/JSObjectUnboxPermission.java
>>> @@ -0,0 +1,49 @@
>>> +/* JSObjectUnboxPermission.java
>>> +   Copyright (C) 2012  Red Hat
>>> +
>>> +This file is part of IcedTea.
>>> +
>>> +IcedTea is free software; you can redistribute it and/or modify
>>> +it under the terms of the GNU General Public License as published by
>>> +the Free Software Foundation; either version 2, or (at your option)
>>> +any later version.
>>> +
>>> +IcedTea is distributed in the hope that it will be useful, but
>>> +WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> +General Public License for more details.
>>> +
>>> +You should have received a copy of the GNU General Public License
>>> +along with IcedTea; see the file COPYING.  If not, write to the
>>> +Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>>> +02110-1301 USA.
>>> +
>>> +Linking this library statically or dynamically with other modules is
>>> +making a combined work based on this library.  Thus, the terms and
>>> +conditions of the GNU General Public License cover the whole
>>> +combination.
>>> +
>>> +As a special exception, the copyright holders of this library give you
>>> +permission to link this library with independent modules to produce an
>>> +executable, regardless of the license terms of these independent
>>> +modules, and to copy and distribute the resulting executable under
>>> +terms of your choice, provided that you also meet, for each linked
>>> +independent module, the terms and conditions of the license of that
>>> +module.  An independent module is a module which is not derived from
>>> +or based on this library.  If you modify this library, you may extend
>>> +this exception to your version of the library, but you are not
>>> +obligated to do so.  If you do not wish to do so, delete this
>>> +exception statement from your version. */
>>> +
>>> +package netscape.javascript;
>>> +
>>> +import java.security.BasicPermission;
>>> +
>>> +/**
>>> + * Permission to access internal reference of JSObject
>>> + */
>>> +public class JSObjectUnboxPermission extends BasicPermission {
>>> +    public JSObjectUnboxPermission() {
>>> +        super("JSObjectUnbox");
>>> +    }
>>> +}
>>> diff --git a/plugin/icedteanp/java/sun/applet/PluginAppletSecurityContext.java
>>> b/plugin/icedteanp/java/sun/applet/PluginAppletSecurityContext.java
>>> --- a/plugin/icedteanp/java/sun/applet/PluginAppletSecurityContext.java
>>> +++ b/plugin/icedteanp/java/sun/applet/PluginAppletSecurityContext.java
>>> @@ -53,7 +53,6 @@ import java.security.Permissions;
>>>   import java.security.PrivilegedAction;
>>>   import java.security.ProtectionDomain;
>>>   import java.util.ArrayList;
>>> -import java.util.Arrays;
>>>   import java.util.Hashtable;
>>>   import java.util.List;
>>>   import java.util.Map;
>>> @@ -241,16 +240,6 @@ public class PluginAppletSecurityContext
>>>       public PluginAppletSecurityContext(int identifier) {
>>>           this.identifier = identifier;
>>>
>>> -        // We need a security manager.. and since there is a good chance that
>>> -        // an applet will be loaded at some point, we should make it the SM
>>> -        // that JNLPRuntime will try to install
>>> -        if (System.getSecurityManager() == null) {
>>> -            JNLPRuntime.initialize(/* isApplication */false);
>>> -            JNLPRuntime.setDefaultLaunchHandler(new DefaultLaunchHandler(System.err));
>>> -        }
>>> -
>>> -        JNLPRuntime.disableExit();
>>> -
>>>           URL u = null;
>>>           try {
>>>               u = new URL("file://");
>>> diff --git a/plugin/icedteanp/java/sun/applet/PluginMain.java
>>> b/plugin/icedteanp/java/sun/applet/PluginMain.java
>>> --- a/plugin/icedteanp/java/sun/applet/PluginMain.java
>>> +++ b/plugin/icedteanp/java/sun/applet/PluginMain.java
>>> @@ -73,6 +73,7 @@ import java.net.ProxySelector;
>>>   import java.util.Enumeration;
>>>   import java.util.Properties;
>>>
>>> +import net.sourceforge.jnlp.DefaultLaunchHandler;
>>>   import net.sourceforge.jnlp.config.DeploymentConfiguration;
>>>   import net.sourceforge.jnlp.runtime.JNLPRuntime;
>>>   import net.sourceforge.jnlp.security.JNLPAuthenticator;
>>> @@ -106,6 +107,15 @@ public class PluginMain {
>>>               // must be called before JNLPRuntime.initialize()
>>>               JNLPRuntime.setRedirectStreams(redirectStreams);
>>>
>>> +            // We need a security manager for PluginAppletSecurityContext,
>>> +            // so we ensure it is initialized
>>> +            if (System.getSecurityManager() == null) {
>>> +                JNLPRuntime.initialize(/* isApplication */false);
>>> +                JNLPRuntime.setDefaultLaunchHandler(new DefaultLaunchHandler(System.err));
>>> +            }
>>> +
>>> +            JNLPRuntime.disableExit();
>>> +
>>>               PluginAppletSecurityContext sc = new PluginAppletSecurityContext(0);
>>>               sc.prePopulateLCClasses();
>>
>> Well, I'm still not comfortable with this change.
>> I'm not sure which security parts can be affected:-/ I really would like to know Jana's opinion here.
>> Do you mind to move it to separate method?
>
> I have attached a patch that is more refactoring-proof. It is based on bypassing installation via
> package-private constructor. Note there is no security risk if this is 'bypassed' once it has
> already occurred.
>> [.. snip ..]
>>>
>>
> Thanks,
> -Adam
>