MSC05-J. Make sensitive classes noncloneable

Java's Object cloning mechanism allows an attacker to manufacture new instances of a class, without executing its constructor. The new instances are made by copying the memory images of existing objects. Although this is sometimes an acceptable way of creating new objects, it often is not. By misusing the clone feature, an attacker can manufacture multiple instances of a singleton class, create serious thread-safety issues by subclassing and cloning the subclass, bypass security checks within the constructor and violate the invariants of critical data.

Noncompliant Code Example

This noncompliant example derives some functional behavior from the implementation of the class java.lang.StringBuffer, prior to JDK v1.5. A SensitiveClass class is defined which contains a character array used to internally hold a filename, and a Boolean shared variable. When a client requests a String instance by invoking the get() method, the shared flag is set. Operations that can modify the array are subsequently prohibited in order to be consistent with the returned String object. Therefore, the replace() method designed to replace all elements of the array with an 'x', cannot execute normally when the flag is set. Java's cloning feature provides a way to illegally work around this constraint even though SensitiveClass does not implement the Cloneable interface.

Here, a malicious class subclasses the non-final SensitiveClass and provides a public clone() method. It proceeds to create its own instance (ms1) and produces a second one (ms2), by cloning the first. It subsequently obtains a new String filename object by invoking the get() method on the first instance. At this point, the shared flag is set to true. Since the second instance (ms2) does not have its shared flag set to true, it is possible to alter the first instance ms1 using the replace() method. This downplays any security efforts and severely violates the object's invariants.

class SensitiveClass {
  private char[] filename;
  private Boolean shared = false;
 
  protected SensitiveClass(String filename) {
    this.filename = filename.toCharArray();
  }

  protected void replace(){
    if(!shared)
      for(int i=0;i<filename.length;i++) {
    	filename[i]= 'x';
    }
  }

  protected String get(){
    if(!shared){	
      shared = true;
     return String.valueOf(filename);
    } else
     throw new Error("Error getting instance");
  }
  
  protected void printFilename(){
    System.out.println(String.valueOf(filename));
  }
}

class MaliciousSubclass extends SensitiveClass implements Cloneable {	
  protected MaliciousSubclass(String filename) {
    super(filename);
  }
  
  public MaliciousSubclass Clone() {  // well-behaved clone() method
    MaliciousSubclass s = null;
    try {
      s = (MaliciousSubclass)super.clone();	        
    }catch(Exception e) { System.out.println("not cloneable"); }
    return s;
  }

  public static void main(String[] args){
    MaliciousSubclass ms1 = new MaliciousSubclass("file.txt");
    MaliciousSubclass ms2 = ms1.Clone(); // creates a copy 
    String s = ms1.get(); // returns filename
    System.out.println(s); // filename is "file.txt"
    ms2.replace(); // replaces all characters with x'
    // both ms1.get() and ms2.get() will subsequently return filename = 'xxxxxxxx'
    ms1.printFilename(); // filename becomes 'xxxxxxxx' 
    ms2.printFilename(); // filename becomes 'xxxxxxxx'
  }
}

Compliant Solution

Sensitive classes should not implement the Cloneable interface. If the class extends from a superclass that implements Cloneable (and is therefore cloneable), it's clone() method should throw a CloneNotSupportedException. This exception must be caught and handled by the client code. A sensitive class that does not implement Cloneable must also follow this advice.

It is also required to declare SensitiveClass final so as to avoid malicious subclassing. This will stop an artful attacker from subclassing the sensitive class and creating several copies of the subclass, with the intention of introducing thread-safety issues.

final SensitiveClass {
  // ...
  public SensitiveClass Clone() throws CloneNotSupportedException {
    throw new CloneNotSupportedException();
  }
}

Risk Assessment

References

[[Mcgraw 98]]
[[Wheeler 03]] 10.6. Java
[[MITRE 09]] CWE ID 498 "Information Leak through Class Cloning", CWE ID 491 "Public cloneable() Method Without Final (aka 'Object Hijack')"

MSC04-J. Be aware of JVM Monitoring and Managing      49. Miscellaneous (MSC)      MSC30-J. Generate truly random numbers