Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: REM cost reform

Calling overridable methods from the clone() method is insecure for two reasons. First, a malicious subclass may could override the method and affect the behavior of the clone() method. Second, a trusted subclass may could observe (and potentially modify) the cloned object in an uninitialized a partially initialized state before its construction has concluded. Consequently, it is possible to In either case, the subclass could leave the clone as well as , the object being cloned, or both in an inconsistent state. Consequently, clone() methods may invoke only methods that are final or private. 

This rule is closely related to MET05-J. Ensure that constructors do not call overridable methods.

Noncompliant Code Example

This noncompliant code example shows two classes, BadClone CloneExample and Sub. BadClone The class CloneExample calls an overridable method doSomething(). The overridden method sets the value of the cookies whereas ; the overriding method sets the values of the domain names. At runtime, because of polymorphism, the The doSomething() method of the subclass Sub}} is erroneously executed twice at runtime because of polymorphism. The first invocation comes from CloneExample.clone(), and the other comes from Sub.clone(). Consequently, the values of the cookies are never initialized, whereas the domains are initialized twice.

Furthermore, the subclass not only sees . Not only does the subclass see the clone in an inconsistent state , its {{doSomething() method modifies it in a way 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 undesirably.

Code Block
bgColor#FFcccc

class BadCloneCloneExample implements Cloneable {
  HttpCookie[] cookies;
  
  BadCloneCloneExample(HttpCookie[] c) {
    cookies = c;
  }
 
  public Object clone() throws CloneNotSupportedException {		
    final BadCloneCloneExample clone = (BadCloneCloneExample) 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();
    }

    // deepDeep 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 BadCloneCloneExample {
  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);
    }
    BadCloneCloneExample bc = new Sub(hc);
    bc.clone();
  }
}

If When an overridable method is invoked on a shallow copy of the object, the original object is also modified.

Compliant Solution

This compliant solution declares both the doSomething() and the deepCopy() methods final., preventing overriding of these methods:

Code Block
bgColor#ccccff
class CloneExample implements Cloneable {
  final void doSomething() {
    // ...
  }
  final HttpCookie[] deepCopy() {
    // ...
  }

  // ...
}

Alternative solutions that prevent invocation of overridden methods include declaring these methods private or final or declaring the class containing these methods final.

Exceptions

MET06-J-EX0: It is permitted to call a superclass's method via super.method(...), since such calls will not be dynamically dispatched to methods defined by a subclass. In fact, calling super.clone() is expected behaviorAlternatively, it is permissible to declare the methods private or to declare the class final. Eliminating the method calls by congregating the code together is also permissible.

Risk Assessment

Calling overridable methods on the clone under construction , can leave its state or that of the 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 of the clone, the object being cloned, inconsistentor both.

Rule

Severity

Likelihood

Remediation Cost

Detectable

Repairable

Priority

Level

MET07

MET06-J

medium

Medium

Probable

probable

Yes

low

No

P12

P8

L1

L2

Automated Detection

...

Automated detection is straightforward.

ToolVersionCheckerDescription
Parasoft Jtest

Include Page
Parasoft_V
Parasoft_V

CERT.MET06.CLONEMake your 'clone()' method "final" for security
SpotBugs

Include Page
SpotBugs_V
SpotBugs_V

MC_OVERRIDABLE_METHOD_CALL_IN_CLONEImplemented (since 4.5.0)

Bibliography

[Bloch 2008]

Item 11, "Override clone Judiciously"

[Gong 2003]



...

Image Added Image Added Image Added

TODO

Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

Other Languages

This rule appears in the C++ Secure Coding Standard as ARR40-CPP. Use a Valid Ordering Rule.

References

Wiki Markup
\[[Bloch 08|AA. Java References#Bloch 08]\] Item 11: Override {{clone}} judiciously
\[[Gong 03|AA. Java References#Gong 03]\]

MET06-J. Do not call overridable methods from a privileged block      12. Methods (MET)      MET08-J. Do not use the clone method to copy untrusted method parameters