...
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.Consider the following noncompliant class definition. Unless someone knows the secret password, objects cannot be created. This is because the constructor for the class checks the input against the expected password before allowing object construction. It also performs a security check. The calling code uses a doPrivileged
block to create an instance of SensitiveClass
. Object creation beyond this block is prohibited by the security manager. However, since the SensitiveClass
advertises a public
clone()
method, it is possible to circumvent this restriction. This clearly violates the principle of least privilege. Yet another blemish is the inconsistent security checking.
Code Block | ||
---|---|---|
| ||
class SensitiveClass implements Cloneable { { private char[] filename; private Boolean shared = false; protected SensitiveClass(String passwdfilename) { // perform security manager check System.out.println("SensitiveClass construction done!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 useprintFilename(){ System.out.println("In method use()"String.valueOf(filename)); } } class MaliciousSubclass extends SensitiveClass implements Cloneable { protected MaliciousSubclass(String filename) { super(filename); } public SensitiveClass Clone() { // well-behaved clone() method SensitiveClass s = null; try { s = (SensitiveClass)super.clone(); }catch(Exception e) { System.out.println("not cloneable"); } return s; } } class Foo { public static protected void privilegedmain(String[] args) { finalMaliciousSubclass SensitiveClass[]ms1 sc = new SensitiveClass[2]; AccessController.doPrivileged(new PrivilegedAction() { MaliciousSubclass("file.txt"); MaliciousSubclass ms2 public Object= run() { sc[0] = new SensitiveClass("password"MaliciousSubclass) ms1.Clone(); // objectcreates creationa withcopy the password String s = sc[0].usems1.get(); //allowed returns filename return nullSystem.out.println(s); // filename is "file.txt" ms2.replace(); } });// replaces all characters with x' // sc[1] = sc[0].Clone(); // object creation without the passwordboth ms1.get() and ms2.get() will subsequently return filename = 'xxxxxxxx' sc[1].usems1.printFilename(); // thisfilename shouldbecomes not be allowed'xxxxxxxx' } public static void main(String[] args) { Foo f = new Foo(); f.privileged();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.
...
bgColor | #ccccff |
---|
...
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.
Code Block | ||
---|---|---|
| ||
final SensitiveClass {
// ...
public SensitiveClass Clone() throws CloneNotSupportedException {
throw new CloneNotSupportedException();
}
}
|
Risk Assessment
Recommendation | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
MSC05-J | medium | probable | medium | P8 | L2 |
...