Calling overridable methods from the clone() method is insecure. First, a malicious subclass may override the method and affect the behavior of the clone() method. Second, a trusted subclass can observe (and potentially modify) the cloned object in a partially - initialized state before its construction has concluded. Consequently, the subclass can leave either the clone, the object being cloned, or both in an inconsistent state.
...
This noncompliant code example shows two classes, BadClone and Sub. The class BadClone calls an overridable method doSomething(). The overridden method sets the value of the cookies; the overriding method sets the values of the domain names. The doSomething() method of the subclass Sub is erroneously executed twice at runtime , because of polymorphism. The first invocation comes from BadClone.clone(), and the other comes from Sub.clone(). Consequently, the cookies never get their values initialized, while their domains are initialized twice.
Furthermore, the subclass not only sees the clone in an inconsistent state , but also modifies the clone in a manner that creates inconsistent copies. This is because the deepCopy() method occurs after the call to the doSomething() method, and the overriding doSomething() implementation erroneously modifies the object.
| Code Block | ||
|---|---|---|
| ||
class CloneExample implements Cloneable {
HttpCookie[] cookies;
CloneExample(HttpCookie[] c) {
cookies = c;
}
public Object clone() throws CloneNotSupportedException {
final CloneExample clone = (CloneExample) super.clone();
clone.doSomething(); // Invokes overridable method
clone.cookies = clone.deepCopy();
return clone;
}
void doSomething() { // Overridable
for (int i = 0; i < cookies.length; i++) {
cookies[i].setValue("" + i);
}
}
HttpCookie[] deepCopy() {
if (cookies == null) {
throw new NullPointerException();
}
// deep copy
HttpCookie[] cookiesCopy = new HttpCookie[cookies.length];
for (int i = 0; i < cookies.length; i++) {
// Manually create a copy of each element in array
cookiesCopy[i] = (HttpCookie) cookies[i].clone();
}
return cookiesCopy;
}
}
class Sub extends CloneExample {
Sub(HttpCookie[] c) {
super(c);
}
public Object clone() throws CloneNotSupportedException {
final Sub clone = (Sub) super.clone();
clone.doSomething();
return clone;
}
void doSomething() { // Erroneously executed
for (int i = 0;i < cookies.length; i++) {
cookies[i].setDomain(i + ".foo.com");
}
}
public static void main(String[] args) throws CloneNotSupportedException {
HttpCookie[] hc = new HttpCookie[20];
for (int i = 0 ; i < hc.length; i++){
hc[i] = new HttpCookie("cookie" + i,"" + i);
}
CloneExample bc = new Sub(hc);
bc.clone();
}
}
|
...
Calling overridable methods on the clone under construction can expose class internals to malicious code or violate class invariants by exposing the clone to trusted code in a partially - initialized state, affording the opportunity to corrupt the state either of the clone, or of the object being cloned, or of both.
Guideline | Severity | Likelihood | Remediation Cost | Priority | Level |
|---|---|---|---|---|---|
MET07-J | medium | probable | low | P12 | L1 |
...