[rfc][icedtea-web] DeploymentPropertiesAreExposed reproducer fix

[Jacob Wisor]

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