A nested class is any class whose declaration occurs within the body of another class or interface [JLS 2015]. The use of a nested class is error prone unless the semantics are well understood. A common notion is that only the nested class may access the contents of the outer class. Not only does the nested class have access to the private fields of the outer class, but the same fields can be accessed by any other class within the package when the nested class is declared public or if it contains public methods or constructors. As a result, the nested class must not expose the private members of the outer class to external classes or packages.
According to The Java Language Specification (JLS), §8.3, "Field Declarations" [JLS 2015]:
Note that a private field of a superclass might be accessible to a subclass (for example, if both classes are members of the same class). Nevertheless, a private field is never inherited by a subclass.
Noncompliant Code Example
This noncompliant code example exposes the private (x,y) coordinates through the getPoint() method of the inner class. Consequently, the AnotherClass class that belongs to the same package can also access the coordinates.
class Coordinates {
private int x;
private int y;
public class Point {
public void getPoint() {
System.out.println("(" + x + "," + y + ")");
}
}
}
class AnotherClass {
public static void main(String[] args) {
Coordinates c = new Coordinates();
Coordinates.Point p = c.new Point();
p.getPoint();
}
}
Compliant Solution
Use the private access specifier to hide the inner class and all contained methods and constructors.
class Coordinates {
private int x;
private int y;
private class Point {
private void getPoint() {
System.out.println("(" + x + "," + y + ")");
}
}
}
class AnotherClass {
public static void main(String[] args) {
Coordinates c = new Coordinates();
Coordinates.Point p = c.new Point(); // Fails to compile
p.getPoint();
}
}
Compilation of AnotherClass now results in a compilation error because the class attempts to access a private nested class.
Risk Assessment
The Java language system weakens the accessibility of private members of an outer class when a nested inner class is present, which can result in an information leak.
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
|---|---|---|---|---|---|
OBJ08-J | Medium | Probable | Medium | P8 | L2 |
Automated Detection
Automated detection of nonprivate inner classes that define nonprivate members and constructors that leak private data from the outer class is straightforward.
| Tool | Version | Checker | Description |
|---|---|---|---|
| CodeSonar | 8.1p0 | JAVA.CLASS.ICSBS | Inner Class Should be Static (Java) |
| Parasoft Jtest | 2024.1 | CERT.OBJ08.INNER | Make all member classes "private" |
Related Guidelines
Bibliography
[JLS 2015] | §8.1.3, Inner Classes and Enclosing Instances |
Section 2.3, "Inner Classes" | |
Securing Java: Getting Down to Business with Mobile Code |
9 Comments
Dhruv Mohindra
OWASP and the Securing Java book advise against using it period. But since it is a facility which is often useful, I think it is ok to use it provided the usage is correct. Comments?
Dhruv Mohindra
These paragraphs are relevant, picked up from guideline 1-1 of Sun's Secure Coding document. The last line does say it is indeed hard to carry out the injection attacks conditional on proper code isolation on class loading. The feasibility of this attack needs more research.
"Also note that the use of nested classes can automatically cause the accessibility of members in both the nested class and its enclosing class to widen from private to package-private. This occurs because the javac compiler adds new static package-private methods to the generated class file to give nested classes direct access to referenced private members in the enclosing class and vice versa. Any nested class declared private is also converted to package-private by the compiler. While javac disallows the new package-private methods from being called at compile-time, the methods - in fact, any protected or package-private class or member - can be exploited at run-time using a package insertion attack (an attack where hostile code declares itself to be in the same package as the target code). In the presence of nested classes, this attack is particularly pernicious because it gives the attacker access to class members originally declared private by a developer.
Package insertion attacks can be difficult to achieve in practice. In the Java virtual machine, class loaders are responsible for defining packages. For a successful attack to occur, hostile code must be loaded by same class loader instance as the target code. As long as services that perform class loading properly isolate unrelated code (the Java Plugin, for example, loads unrelated applets into separate class loader instances), untrusted code can not access package-private members declared in other classes, even if it declares itself to be in the same package."
David Svoboda
The CCE does not compile (on Linux/SunJava6), because you can't access Coordinate.Point in AnotherClass.
Dhruv Mohindra
Right. It is not meant to compile so as to be compliant. The idea was to block access to the private members.
Thomas Hawtin
I think the title should refer to nested classes here. (IIRC, in the JRE 1.1 language spec ammendments used inner classes to refer to what JLS 2nd Ed calls nested classes. Hence the confusion.)
If malicous code can be placed in the same package and access sensitive code then we have a problem. The class loader mechnaism is used to prevent this from happening. Failure to do so is a security vulnerability.
Dhruv Mohindra
Correct, renamed to nested since they include static classes as well (and static classes are also noncompliant).
Dhruv Mohindra
I think the compliant solution does not foil the runtime package insertion attack. The compiler generated synthetic method would still give package-private access to the private fields. I guess, the real the solution is to ensure that untrusted code is isolated. There may be issues if this code can obtain instances of the target classloader by some means and then load arbitrary classes (by defining it so as to belong to the same package). The question is - shall we recommend that the classloader must always isolate unrelated code and also show how or simply leave it at the recommendation?
Code signing could also be recommended.
Thomas Hawtin
You shouldn't be able to inject malicious classes into the same package as sensitive classes.
If you don't trust your container to enforce this rule, sealing the package ("Sealed: true" in the manifest) will prevent URLClassLoader loading classes from other sources into the same package.
Dhruv Mohindra
Thanks. This reminds me of a related idea for a rule that says 'do not sign encrypted data'; that is, when integrity and confidentiality both have to be provided. In other words, create and sign a
SignedObjectfirst and then use it to create aSealedObject.