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

[Adam Domurad]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: jsfix-unittest3.patch
Type: text/x-patch
Size: 9812 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130107/125da2d5/jsfix-unittest3.patch