Page History

Returning references to internal mutable members of a class can seriously compromise the security of an application because of the resulting sub par encapsulation properties and susceptibility to corruption of the class's state. A caller can modify the private data if instead of defensive copies of mutable class members, direct references to them are returned.

Returning references that refer to private data to untrusted code can be more pernicious than returning the references to trusted code. If a class provides copy functionality that trusted code can use to pass defensive copies of the instance to untrusted code, see guideline OBJ10-J. Provide mutable classes with copy functionality to allow passing instances to untrusted code safely) the implementing class may violate this guideline. However, the burden is now transferred to the trusted code as it is expected to reliably use the copy functionality before operating on the instance or passing it to untrusted code.

Note that the performance costs of violating this guideline (or using copy functionality such as clone()) might be significantly more than using an accessor method that returns a copy of a single mutable member. This is because a well designed copy method returns a copy of the complete object, including all mutable components) as opposed to that of a single member, which makes it relatively sloweran application's security, both by breaking encapsulation and also by providing the opportunity to corrupt the internal state of the class (whether accidentally or maliciously). Performing a defensive copy before returning a reference to mutable internal state ensures that the caller can modify only the copy; he cannot modify the original internal state.

Pugh \[[Pugh 2009|AA. Bibliography#Pugh 09]\] cites a vulnerability discovered by the Findbugs static analysis tool in the early betas of jdk 1.7. The class {{sun.security.x509.InvalidityDateExtension}} returned a {{Date}} instance through a {{public}} accessor, without creating defensive copies.

Noncompliant Code Example (Mutable Member Containing Immutable Objects)

This In this noncompliant code example defines a , class that ReturnRef contains a private Hashtable instance field. The hash table stores immutable data of sensitive nature (SSN numbers). A getter method getValues() gives the caller access to the hash table by returning a reference to it. When invoked by untrusted code, entries may An untrusted caller can use the getter method to gain access to the hashtable; consequently, hashtable entries can be maliciously added, removed or replaced.

...

Compliant Solution (Shallow Copying)

Do not provide a getter method like getValues() that exposes Make defensive copies of private internal mutable object state without defensively copying the state. This applies largely to members containing mutable state, however, if the internal state is of sensitive nature even immutable data may not be returned to maintain the encapsulation property (the latter condition is not mandatory for compliance). Deep copies of mutable data are required to be returned whereas it suffices to return shallow copies of mutable fields containing immutable data. For mutable fields that contain immutable data, a shallow copy is sufficient. Fields that refer to mutable data generally require a deep copy (see below).

This compliant solution creates and returns a shallow copy of the hash table containing immutable SSN numbers. As a result, any attempts to modify the original hash table are ineffective.

...

If the hash table contained references to mutable data such as a series of Date objects, every one each of those objects must also be copied by using a copy constructor or method. For further details, refer to guidelines FIO00-J. Defensively copy mutable inputs and mutable internal components and OBJ10-J. Provide mutable classes with copy functionality to allow passing instances to untrusted code safely. Note that the keys of a hash table need not be deep copied; shallow copying of the references suffices because a hash table's contract dictates that it cannot hold duplicate keys.

...

class MutableClass {
  private Date d;

  public MutableClass() {
    d = new Date();
  }

  public Date getDate() {
    return d;
  }
}

An untrusted caller will be able to manipulate this instance as it exposes internal mutable components can manipulate the instance because returning the reference exposes the internal mutable component beyond the trust boundaries of the class.

Compliant Solution (Shallow Copying Using clone() for Final Classes)

Do not carry out defensive copying using Defensive copies performed during execution of a constructor must avoid use of the clone() method in constructors, when the (non-system) class in question can be subclassed by untrusted code. This will limit the malicious code from returning a crafted object when the object's restriction prevents execution of a maliciously-crafted overriding of the clone() method is invoked.

Despite this adviceNevertheless, this compliant solution recommends returning a clone of the Date object . While this should not be done in constructors, it is permissible to use this technique in accessors. This is because there is no danger of a malicious subclass from the getDate() accessor method. This is an acceptable solution because a malicious subclass cannot extending the internal mutable Date object (; Date is a system class and system class, the internal instance was locally constructed using the system class's constructor, so the operation is consequently safe.

protected Date getDate() {
  return (Date)d.clone();
}

Arrays This guideline also applies to arrays of primitive types are not exempt from this guideline. As . Because Java arrays are objects in Java, if their own right, when a caller receives a reference to an array is returned, the caller may , he can freely modify the its contents of the original array. Shallow copies of . Creating a shallow copy of an arrays of primitive types are safe to return. provides a distinct array instance; although the caller can modify the returned array, he cannot affect the original array.

Classes that have public setter methods must follow the related advice found If the class has a public setter method it must follow related advice as given in guideline FIO00-J. Defensively copy mutable inputs and mutable internal components. Note that a setter method may setter methods can (and usually should) perform input validation and sanitization before setting the internal fields. On the other hand, returning references to internal objects may not require the caller to incorporate any of these defensive measures.

Noncompliant Code Example (Mutable Member Array)

This In this noncompliant code example consists of , the getDate() accessor method returns an array of Date objects.

class MutableClass {
  private Date[] date;

  public MutableClass() {
    for(int i = 0; i < 20; i++)
      date[i] = new Date();
    }

  public Date[] getDate() {
    return date; // or return date.clone()
  }
}

It does not defensively copy fails to make a defensive copy of the array before returning it. A Because the array contains references to Date objects that are themselves mutable, a shallow copy of the array may also be inappropriate in this case because Date is mutable(as discussed at the end of the previous compliant example) would be insufficient.

Compliant Solution (Deep Copying)

This compliant solution creates a deep copy of the date array and returns the copy instead of rather than returning the internal date array.

class MutableClass {
  private Date[] date;

  public MutableClass() {
    for(int i = 0; i < 20; i++) {
      date[i] = new Date();
    }
  }

  public Date[] getDate() {
    Date[] dates = new Date[20];
    for(int i = 0; i < 20; i++) {
      dates[i] = (Date) date[i].clone();
    }
    return dates;
  }
}

...

Returning references to internal object state (mutable or immutable) can render an application susceptible to information leaks and to corruption of its objects' states and violate any that consequently violates class invariants. Control flow may can also be affected in some cases.

Automated Detection

TODOSound automated detection is infeasible; heuristic checks may be useful.

Related Vulnerabilities

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

...

\[[API 2006|AA. Bibliography#API 06]\] [method clone()|http://java.sun.com/javase/6/docs/api/java/lang/Object.html#clone()]
\[[Security 2006|AA. Bibliography#Security 06]\]
\[[Bloch 2008|AA. Bibliography#Bloch 08]\] Item 39: Make defensive copies when needed
\[[SCGGoetz 20072006|AA. Bibliography#SCGBibliography#Goetz 0706]\] Guideline 2-1 Create a copy of mutable inputs and outputs 3.2. Publication and Escape: Allowing Internal Mutable State to Escape
\[[Gong 2003|AA. Bibliography#Gong 03]\] 9.4 Private Object State and Object Immutability
\[[Haggar 2000|AA. Bibliography#Haggar 00]\] [Practical Java Praxis 64: Use clone for Immutable Objects When Passing or Receiving Object References to Mutable Objects|http://www.informit.com/articles/article.aspx?p=20530]
\[[Goetz 2006|AA. Bibliography#Goetz 06]\] 3.2. Publication and Escape: Allowing Internal Mutable State to Escape
\[[Gong 2003|AA. Bibliography#Gong 03]\] 9.4 Private Object State and Object Immutability
\[[MITRE 2009|AA. Bibliography#MITRE 09]\] [CWE ID 375|http://cwe.mitre.org/data/definitions/375.html] "Passing Mutable Objects to an Untrusted Method"
\[[SCG 2007|AA. Bibliography#SCG 07]\] Guideline 2-1 Create a copy of mutable inputs and outputs
\[[Security 2006|AA. Bibliography#Security 06]\]

...

OBJ10-J. Provide mutable classes with copy functionality to allow passing instances to untrusted code safely      08. Object Orientation (OBJ)      OBJ12-J. Use checked collections against external code