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. Often this is not an acceptable way of creating new objects. 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 code 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, to be consistent 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.
...
Code Block | ||
---|---|---|
| ||
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 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.
...
An alternative is to declare the clone()
method final
so that it cannot be overridden.
Risk Assessment
Failure to make sensitive classes noncloneable can severely violate the class invariants and provide malicious subclasses the opportunity to exploit the code to create new instances of objects, without security manager checks (by default).
Recommendation | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
MSC37 MSC32- J | medium | probable | medium | P8 | L2 |
Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this rule on the CERT website.
References
Wiki Markup |
---|
\[[Mcgraw 98|AA. Java References#Mcgraw 98]\] \[[Wheeler 03|AA. Java References#Wheeler 03]\] 10.6. Java \[[MITRE 09|AA. Java References#MITRE 09]\] [CWE ID 498|http://cwe.mitre.org/data/definitions/498.html] "Information Leak through Class Cloning", [CWE ID 491|http://cwe.mitre.org/data/definitions/491.html] "Public cloneable() Method Without Final (aka 'Object Hijack')" |
...