Skip to end of metadata
Go to start of metadata

The Java classes used by a program are not necessarily loaded upon program startup. Many Java Virtual Machines (JVMs) load classes only when they need them.

If untrusted code is permitted to load classes, it may possess the ability to load a malicious class. This is a class that shares a fully-qualified name with a benign class that is required by trusted code. When the trusted code tries to load its benign class, the JVM provides it with the malicious class instead. As a result, if a program permits untrusted code to load classes, it must first preload any benign classes it needs. Once loaded, these benign classes cannot be replaced by untrusted code.

Noncompliant Code Example (Tomcat)

This noncompliant code example shows a vulnerability present in several versions of the Tomcat HTTP web server (fixed in version 6.0.20) that allows untrusted web applications to override the default XML parser used by the system to process web.xml, context.xml and tag library descriptor (TLD) files of other web applications deployed on the Tomcat instance. Consequently, untrusted web applications that install a parser could view and/or alter these files under certain circumstances.

The noncompliant code example shows the code associated with initialization of a new Digester instance in the org.apache.catalina.startup.ContextConfig class. "A Digester processes an XML input stream by matching a series of element nesting patterns to execute Rules that have been added prior to the start of parsing" [Tomcat 2009]. The code to initialize the Digester follows:

protected static Digester webDigester = null;

if (webDigester == null) {
  webDigester = createWebDigester();
}

The createWebDigester() method is responsible for creating the Digester. This method calls createWebXMLDigester(), which invokes the method DigesterFactory.newDigester(). This method creates the new digester instance and sets a boolean flag useContextClassLoader to true.

// This method exists in the class DigesterFactory and is called by 
// ContextConfig.createWebXmlDigester().
// which is in turn called by ContextConfig.createWebDigester()
// webDigester finally contains the value of digester defined
// in this method.
public static Digester newDigester(boolean xmlValidation,
                                   boolean xmlNamespaceAware,
                                   RuleSet rule) {
  Digester digester = new Digester();
  // ...
  digester.setUseContextClassLoader(true);
  // ...
  return digester;
}

The useContextClassLoader flag is used by Digester to decide which ClassLoader to use when loading new classes. When true, it uses the WebappClassLoader, which is untrusted because it loads whatever classes are requested by various web applications.

public ClassLoader getClassLoader() {
  // ...
  if (this.useContextClassLoader) {
    // Uses the context class loader which was previously set
    // to the WebappClassLoader
    ClassLoader classLoader =
        Thread.currentThread().getContextClassLoader();
  }
  return classloader;
}

The Digester.getParser() method is subsequently called by Tomcat to process web.xml and other files:

// Digester.getParser() calls this method. It is defined in class Digester
public SAXParserFactory getFactory() {
  if (factory == null) {
    factory = SAXParserFactory.newInstance(); // Uses WebappClassLoader
    // ...
  }
  return (factory);
}

The underlying problem is that the newInstance() method is being invoked on behalf of a web application's class loader, the WebappClassLoader, and it loads classes before Tomcat has loaded all the classes it needs. If a web application has loaded its own Trojan javax.xml.parsers.SAXParserFactory, when Tomcat tries to access a SAXParserFactory, it accesses the Trojan SaxParserFactory installed by the web application rather than the standard Java SAXParserFactory that Tomcat depends on.

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.

Compliant Solution (Tomcat)

In this compliant solution, Tomcat initializes the SAXParserFactory when it creates the Digester. This guarantees that the SAXParserFactory is constructed using the container's class loader rather than the WebappClassLoader.

The webDigester is also declared final. This prevents any subclasses from assigning a new object reference to webDigester. (See rule OBJ10-J. Do not use public static nonfinal fields for more information.) It also prevents a race condition where another thread could access webDigester before it is fully initialized. (See rule OBJ11-J. Be wary of letting constructors throw exceptions for more information.)

protected static final Digester webDigester = init();

protected Digester init() {
  Digester digester = createWebDigester();
  // Does not use the context Classloader at initialization
  digester.getParser(); 
  return digester;
}

Even if the Tomcat server continues to use the WebappClassLoader to create the parser instance when attempting to process the web.xml and other files, the explicit call to getParser() in init() ensures that the default parser has been set during prior initialization and cannot be replaced. Because this is a one-time setting, future attempts to change the parser are futile.

Risk Assessment

Allowing untrusted code to load classes enables untrusted code to replace benign classes with Trojan classes.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

SEC03-J

high

probable

medium

P12

L1

Automated Detection

ToolVersionCheckerDescription
Parasoft Jtest9.5SECURITY.BV.ACLImplemented

Related Guidelines

Secure Coding Guidelines for the Java Programming Language, Version 3.0

Guideline 6-3. Safely invoke standard APIs that bypass SecurityManager checks depending on the immediate caller's class loader

Android Implementation Details

On Android, the use of DexClassLoader or PathClassLoader requires caution.

Bibliography

[CVE 2011]

CVE-2009-0783

[Gong 2003]

Section 4.3.2, Class Loader Delegation Hierarchy

[JLS 2005]

§4.3.2, The Class Object

[Tomcat 2009]

Bug ID 29936, API Class org.apache.tomcat.util.digester.Digester, Security fix in v 6.0.20

 


            

19 Comments

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

  2. 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).

    1. Yes, both 6-2 and 6-4 have a somewhat common lesson. I added a cross reference to "Beware of standard APIs that perform access checks against the immediate caller" which is another recommendation. That one also demonstrates reflection; should hopefully address the points and subtleties.

  3. 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.

  4. 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?

    1. I conducted a simple experiment with Class.newInstance:

      1. Two jar files - trusted.jar and untrusted.jar on the same classpath. The file trusted.jar has permissions to create a ClassLoader while untrusted.jar does not.

      2. trusted.jar has a class that has a method createInstance() that returns a new instance of the Class object argument using Class.newInstance(). It also returns its own Class object through another vulnerable public method.

      3. The test was to check whether untrusted code can obtain a Class object of trusted code, get its ClassLoader through getClassLoader(), use he 3-arg Class.forname to get a Class object of a custom class loader (by loading it with the classloader obtained earlier with getClassLoader()) and proceed to invoke the createInstance() method present in trusted code, supplying the custom classloader argument to it.

      4. The idea was to check whether this is allowed when the untrusted caller's classloader is the same as the classloader of the trusted code. The result is ironical, because a SecurityException does get thrown at the line where untrusted code attempts to call the trusted method createInstance(). It's not quite bypassing security checks as this guideline purports.

      This needs more tests but feel free to comment meanwhile.

      1. On closer inspection, I believe CVE-2009-0783 was a violation of this guideline. I've added a NCE and CS. The use of java.lang.Thread.getContextClassLoader and java.lang.Class.newInstance seems to be the underlying problem.

  5. 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.

    1. If your Attack.MyClassLoader class is a subclass of ClassLoader, its constructor is going to be invoking the ClassLoader constructor, which isn't allowed unless you have permissions, in which case, you don't, because your subclass's constructor is untrusted.

      I don't see any other reason you could be getting an exception from this code.

      Both ClassLoader constructors list this exception:
      SecurityException- If a security manager exists and itscheckCreateClassLoadermethod doesn't allow creation of a new class loader.

  6. 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?

  7. 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.

  8. 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.

    1. Done. More issues with this rule:

      Q: The tree and associated table in the intro do an excellent job of illustrating when one class can bypass a security check when invoking one of these methods on another class. It's clearly a complex tree-traversal interaction, which is better explained with diagrams than with words. The question is, why was Java built that way? Why the complexity?

      Understanding the rationale is necessary in knowing if the normative text is sufficient. I think it is correct, but more may be needed. And trying to determine when this rule is being followed also seems very difficult.

      TODO: The first NCCE is simple enough, as is its CS. Need to add a CS that implements getInstance() with its own security check, mitigating the bypassing of the builtin security check.

      TODO: The Tomcat NCCE/CS need a clearer explanation, and cleanup too.

      1. I don't know whether I agree that "same of descendent" is a "complex tree-traversal interaction". Most of the immediate caller class stuff appears to be a hang-over from the Java 1.1 security model (which made sense, but was impractical). It seems reasonable that code can access the class loaders it created or had created for it (although, I'd argue using getClassLoader shows that you are probably doing something wrong). Class loaders are considered capabilities, so their access needs to be restricted from unrelated and higher-level code sources.

        The compliant solution i currently severely broken. It's a protected mutable static. Mutable statics should be very rare. The init method has a race condition. You might as well initialise the static at class initialisation time (if that's too soon, put it in a nested class or enum).

        1. I've gutted this rule; it now specifically focuses on the Tomcat vul. Also rewrote the Tomcat vul example; hopefully cleaner now. And fixed the bugs you cited in the CS.

          While informative, I simply couldn't find anything normative in the intro, and its complexity is daunting. Also couldn't find any examples in this rule or SEC05-J worth keeping (they all violate other rules). except the Tomcat vul.

  9. 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?

    1. I rewrote the paragraph to resolve your issues.

  10. "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.

    1. I moved that paragraph to the end of the noncompliant example (which discusses Class.newInstance in more detail).