[rfc][icedtea-web] DeploymentPropertiesAreExposed reproducer fix

[Jiri Vanek]

On 09/17/2013 02:26 AM, Jacob Wisor wrote:
> Hello,
>
> Omair Majid wrote:
>> On 09/16/2013 10:02 AM, Jiri Vanek wrote:
>>> Yes it is interesting. From this point of view (and as Jacob picked up
>>> easily fake-able) both
>>>>> + public final static String USER_CONFIG_HOME;
>>>>> public final static String USER_CACHE_HOME;
>>> and also all other for-testing-package-private should become private and
>>> in tests used via reflection only...
>>>
>>> Any opinion?
>>
>> Please read this carefully:
>> http://docs.oracle.com/javase/specs/jvms/se7/html/jvms-5.html#jvms-5.4.4
>>
>> In the normal netx runtime, Defaults will be loaded by the standard java
>> classloader (sun.misc.Launcher.AppClassLoader) but the jnlp/applet will
>> be loaded by net.sourceforge.jnlp.runtime.JNLPClassLoader. Even if an
>> applet claims it was the net.source.jnlp.* package, the runtime packages
>> are different. A package-private Defaults.USER_CONFIG_HOME will not be
>> visible to an applet.
>
> Hmm, after studying the Java language specification, the Java virtual machine specification, the ClassLoader's reference documentation, and JNLPClassLoader's source code for an evening I am not completely convinced that JNLPClassLoader's current implementation is free of a potential security hole in this case. Just to be clear, my impression is based on reading and academic review only. I did not write any test to exploit this potential security hole which could prove either way.
>
> As I see it, JNLPClassLoader.loadClass(String) blindly attempts to load any given class name and does not prevent loading classes from net.sourceforge.jnlp.*. Although e.g. net.sourceforge.jnlp.config.Defaults probably won't be found by the JNLPClassLoader it will be found by its parent class loader (ClassLoader.getSystemClassLoader()) simply because this is what JNLPClassLoader's code tries to do first.
>
>  > public class JNLPClassLoader extends URLClassLoader {
>  > public synchronized Class<?> loadClass(String name) throws ClassNotFoundException {
>  > […]
>  > // filter out 'bad' package names like java, javax
>  > // validPackage(name);
>
> This suggests that there had been an attempt or had been thought about filtering out classes in /restricted/ packages, but this does not seem to be in effect anymore or not yet. So, at least loading arbitrary IcedTea-Web classes by client applets seems possible. Of course, this does not necessarily mean that these classes may be initialized or fields of currently loaded and instantiated classes by parent class loaders (or the system class loader) are accessible per se. But, I do assume this because different class loaders always return the same Class for a name N as well as resolutions of references to instances of classes and fields always result in the same entity. Hence has a class been loaded and its static
> fields initialized already an access attempt (for a given name N) is always resolved to exactly the same field (JVM Spec §5.4.3).
> Furthermore, I also did not find any text in the specifications nor the reference documentation that would support the claim that class loaders define separate class or namespace domains. All it says is that "Every Class object contains a reference to the ClassLoader that defined it." (ClassLoader reference) and "At run time, a class or interface is determined not by its name alone, but by a pair: its binary name (§4.2.1) and its defining class loader. Each such class or interface belongs to a single run-time package. The run-time package of a class or interface is determined by the package name and defining class loader of the class or interface." (JVM Spec §5.3). I may be wrong but this does not read to me as
> if class loaders were defining separate class or namespace domains. There does indeed exist a hard requirement for JVMs to satisfy ClassLoaderL1.loadClass(name) == ClassLoaderL2.loadClass(name) in order to prevent substitution or replacement of classes but there seems to be no restriction on loading and/or accessing instances of classes loaded by different class loaders.
>
> As always, I may be proved wrong. Unfortunately, I do not have the time for writing an exploit test for this problem so I have to leave this to somebody else, open to debate, or as is.
>
> Thank you for reading!
>
> Jacob


Thank you both for investigations, but we can sleep well:

package net.sourceforge.jnlp.config;

public class Main {

     public static void main(String[] args) {
         System.out.println("XP13" + DeploymentConfiguration.DEPLOYMENT_CONFIG_DIR);
         System.out.println("XP13" + Defaults.USER_CACHE_HOME);
         System.out.println("XP13" + Defaults.USER_CONFIG_HOME);
     }

}


die with attached exception
I know that it is just basic approach (and the reprodcuer in testsuite is doing the same) but at least something :)


J.
	
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: log
Url: http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130917/1d34b60c/log.ksh