Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: rearranged text/code for better readability

...

This noncompliant code example derives some functional behavior from the implementation of the class java.lang.StringBuffer, prior to JDK v1.5. A class SensitiveClass is defined which contains a character array used to internally hold a filename, and along with a Boolean shared variable, initialized to false. 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, to maintain the array's consistency with the returned String object. Consequently, 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.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 then 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. As 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 class's invariants.

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

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

  protectedfinal String get(){
    if(!shared){	
      shared = true;
      return String.valueOf(filename);
    } else {
      throw new ErrorIllegalStateException("ErrorFailed to gettingget instance");
    }
  }
  
  protectedfinal void printFilename(){
    System.out.println(String.valueOf(filename));
  }
}

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, to maintain the array's consistency with the returned String object. Consequently, 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.

This class can be exploited by a malicious class (shown below) that subclasses the non-final SensitiveClass and provides a public clone() method.

Code Block
bgColor#ffcccc
class MaliciousSubclass extends SensitiveClass implements Cloneable {	
  protected MaliciousSubclass(String filename) {
    super(filename);
  }
  
  @Override 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'
  }
}

It proceeds to create its own instance (ms1) and produces a second one (ms2), by cloning the first. It then 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. As 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 class's invariants.

Compliant Solution

Sensitive classes should not implement the Cloneable interface. If the class extends from a superclass that implements Cloneable (and is consequently cloneable), its 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 to avoid malicious subclassing. This stops an attacker from subclassing the sensitive class and creating copies of the subclassbecause it inherits the clone() method from Object.

Code Block
bgColor#ccccff
final class SensitiveClass {
  // ...
  public SensitiveClass Clone() throws CloneNotSupportedException {
    throw new CloneNotSupportedException();
  }
}

It is also required to declare SensitiveClass final to avoid malicious subclassing. This stops an attacker from subclassing the sensitive class and creating copies of the subclass.

An alternative is to declare the clone() method final so that it cannot be overridden. The implementation must still throw a CloneNotSupportedException.

...

Failure to make sensitive classes noncloneable can severely violate class invariants and provide malicious subclasses with the opportunity to exploit the code to create new instances of objects, even in the presence of the default security manager (in the absence of custom security checks).

...