Do java's Inner classes pose a security risk?

Question

Recently the security team on my project released a secure code guidelines document, designed to be used as part of our code reviews. The first thing that struck me was an item that said "Do not use Inner classes". I thought this seemed like a very heavy handed and sweeping statement. Inner classes are good if used correctly right?, but i did a bit of googling and found this, quoted here for convenience.

Rule 5: Don't Use Inner Classes

Some Java language books say that inner classes can only be accessed by the outer classes that enclose them. This is not true. Java byte code has no concept of inner classes, so inner classes are translated by the compiler into ordinary classes that happen to be accessible to any code in the same package. And Rule 4 says not to depend on package scope for protection.

But wait, it gets worse. An inner class gets access to the fields of the enclosing outer class, even if these fields are declared private. And the inner class is translated into a separate class. In order to allow this separate class access to the fields of the outer class, the compiler silently changes these fields from private to package scope! It's bad enough that the inner class is exposed, but it's even worse that the compiler is silently overruling your decision to make some fields private. Don't use inner classes if you can help it. (Ironically, the new Java 2 doPrivileged() API usage guidelines suggest that you use an inner class to write privileged code. That's one reason we don't like the doPrivileged() API.)

My questions are

1. Does this behaviour still exist in java 5 / 6?
2. Is this actually a security risk, given that any class, other than the outer and inner classes, that tried to access the outer class' private members would not compile?
3. Does it pose enough of a security risk to warant the 'guideline' 'Do not use inner classes'?

Answer 1

This information is a around a decade out of date. The widespread use of anonymous inner classes with AccessController.doPrivileged should be a clue. (If you don't like the API, consider the proportion of try-finally blocks that are incorrectly missing in the JDK.)

The policy is that no two class can share the same package if they are loaded by different class loaders or have different certificates. For more protection, mark packages as sealed in the manifest of your jars. So, from a security standpoint, "Rule 4" is bogus and hence also this rule.

In any case, working out security policies you should understand what you are protecting against. These sorts of policies are for handling mobile code (code that moves) that may have different levels of trust. Unless you are handling mobile code, or your code is going into a library that may be required to, there is very little point in these sorts of precautions. However, it is almost always a good idea to use a robust programming style, for instance copying and validating arguments and return values.

Answer 2

Does this behaviour still exist in java 5 / 6?

Not exactly as described; I've never seen a compiler where this was true:

In order to allow this separate class access to the fields of the outer class, the compiler silently changes these fields from private to package scope!

Instead IIRC Sun Java 3/4 created an accessor rather than modifying the field.

Sun Java 6 (javac 1.6.0_16 ) creates a static accessor:

public class InnerExample {     private int field = 42;       private class InnerClass {         public int getField () { return field; };     }      private InnerClass getInner () {          return new InnerClass();     }      public static void main (String...args) {         System.out.println(new InnerExample().getInner().getField());     } }   $ javap -classpath bin -private InnerExample Compiled from "InnerExample.java" public class InnerExample extends java.lang.Object{     private int field;     public InnerExample();     private InnerExample$InnerClass getInner();     public static void main(java.lang.String[]);     static int access$000(InnerExample); }   $ javap -classpath bin -c -private InnerExample static int access$000(InnerExample);   Code:    0:   aload_0    1:   getfield    #1; //Field field:I    4:   ireturn 

Is this actually a security risk, given that any class, other than the outer and inner classes, that tried to access the outer class' private members would not com[p]ile?

I'm speculating a bit here, but if you compile against the class it doesn't, but if you add the access$000 then you can compile code which uses the accessor.

import java.lang.reflect.*;  public class InnerThief {     public static void main (String...args) throws Exception {         for (Method me : InnerExample.class.getDeclaredMethods()){             System.out.println(me);             System.out.printf("%08x\n",me.getModifiers());         }          System.out.println(InnerExample.access$000(new InnerExample()));     } } 

The interesting thing is that the synthesised accessor has modifier flags 00001008 where if you add a package level static method it has flags 00000008. There's nothing in the second edition of the JVM spec for that flag value, but it seems to prevent the method being seen by javac.

So it appears that there's some security feature there, but I can't find any documentation on it.

(hence this post in CW, in case someone does know what 0x1000 means in a class file)