SEC03-J. Do not load trusted classes after allowing untrusted code to load arbitrary classes

If you must use reflection, prefer java.beans which adds additional restrictions.

Note, in the Sun 2.0 guidelines, Class.newInstance is covered by both guideline 6-2 (for the Class.getConstructor/class-loader aspect) and 6-4 (for Constructor.newInstance/Java language access).

Upon examining this guideline more closely, I consider it to be a more serious threat that could be classified as a rule instead of a recommendation.

The NCCE does not trigger a security check. One question is, what happens when the immediate caller's classloader is the child of the class object's classloader. (See below)

This (guess) is what I think could be happening and would like some comments on it -

1. The immediate caller of newInstance is the current class's code, with its classloader as AppClassLoader.
2. The Class Object is dateClass (For clarifying the naming convention) and it is loaded by the primordial (bootstrap) classloader.
3. This example obtains an instance of the Date object in the untrusted method, so when createInstance is called, the primordial class loader just has to return an instance that it has found previously.
4. Here, AppClassLoader is not the delegation ancestor of the primordial class loader (it is rather a distant child). So why is a security check that disallows the use of createInstance not invoked? (It is not invoked in 3 cases as specified in the text, but is this also included?)

It boils down to accepting malicious classes as inputs to create their instances from trusted classloaders. Any other perils?

Maybe we need an example showing the abuse of getParent too.

This particular NCE is void. From the documentation of newInstance(), a security exception results when -

the caller's class loader is not the same as or an ancestor of the class loader for the current class and invocation of s.checkPackageAccess() denies access to the package of this class.

The first part is true for this NCE but the latter part is not as it loads a trusted class (thus there is no security exception, in compliance with the documentation). There can only be a problem if by design, custom sensitive classes can be loaded via the primordial class loader. Other mistakes can revolve around accepting malicious input (classloader instances) and setting them as parents of the legit child classloader during its creation. Then the malicious class loader can get away with loading anything that the child classloader can. Any others?

Here is the demo code for my comment on Aug 05 (this concerns propagating sensitive objects to untrusted code):

This package Safe is bundled as trusted.jar which is given AllPermission.

package Safe;

public class TrustedCode {
  public TrustedCode() {  }
  public static Class retClass() {
    return TrustedCode.class; // returns class object
  }
  public static Object createInstance(Class<?> someClass) {
    try { 
      System.out.println(TrustedCode.class.getClassLoader());  
      return someClass.newInstance();  // returns new instance of supplied class object         
    } catch (InstantiationException ie) { ie.printStackTrace(); }
      catch (IllegalAccessException iae) { iae.printStackTrace(); }
      return null;
  }       
}

The package Attack is bundled as untrusted.jar and given no permissions. The same jar file also contains the class MyClassLoader, a custom class loader implementation.

package Attack;

import Safe.TrustedCode;

public class AttackTrustedCode {
  public static void main(String[] args) throws ClassNotFoundException {		     
    Class<?> attacker = TrustedCode.retClass();
    ClassLoader cl = attacker.getClassLoader();
    Class<?> myclassloader = Class.forName("Attack.MyClassLoader", true, cl);
    System.out.println(cl); // prints the same as TrustedCode (AppClassLoader)
    Object o = TrustedCode.createInstance(myclassloader);
  }
}

AttackTrustedCode cannot create an instance of a MyClassLoader as it does not have the required permissions. However, TrustedCode has the permissions. If AttackTrustedCode obtains a class object from TrustedCode, gets its class loader and exploits TrustedCode's createInstance() method to create a new instance, the security manager should allow it because the immediate caller's (TrustedCode) class loader is the same as that of the Class object (someClass). But there is a security exception where newInstance() is called. If instead of MyClassLoader, a trusted java.util.Date object is created, there is no exception.

My intuition is that if trusted.jar was loaded independently by a different class loader (instead of through untrusted.jar), there might be no exception. I'll appreciate any comments.

We seem to have several related recommendations in this guideline:

1. API methods that are potentially invoked by untrusted code must never transitively invoke any of the methods listed at the top.
2. Trusted code should discard or clean any arguments provided by untrusted code (e.g. taint analysis). More specifically do not accept Class, ClassLoader or Thread instances from untrusted code.
3. Do not leak the results of any of the listed methods back to untrusted code (escape analysis needed to assure this).
4. Broadly speaking, don't let untrusted code transitively invoke any API that winds up doing reflection. Unless the arguments and results are cleaned and hidden as in previous items.

Is this a correct characterization?

I like the above characterization though I'm unsure we want to split them as it will be a lot of space. It would also be useful if someone can go over the described tomcat vuln.

Long Poke's reasoning sounds correct which brings us back to the question of how an attacking class can create an object that it does not have the permissions to create, by exploiting permissions given to another class that uses unsafe APIs. At the very least MyClassLoader needs to exist in the trusted code for this to work with the necessary permissions. If all this works as expected, we can replace the current NCE/AS examples with the code in my comment. The current example is slightly hard to follow and can just be mentioned elsewhere in the guideline in that case.

TODO: Need to report on complete-ness of list of API methods (either complete it or list is as incomplete). Comes for SCG07...add local ref.

Question 1:

If the trusted code has not already loaded these classes, subsequent attempts may result in untrusted classes being substituted for the sensitive classes.

1. "subsequent attempts" means loading trusted code?
2. If so, such an attempt seems to be unnecessary to have sensitive classes substituted by untrusted classes.

Question 2:
What is the difference between sensitive class and trusted class in this rule?

"Note that the Class.newInstance() method requires the class to contain a no-argument constructor. If this requirement is not satisfied, a runtime exception results, which indirectly prevents a security breach."

Is this a leftover from the re-write.  Its seems to have no relation to the rule.