Returning references to internal mutable members of a class can compromise an 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.
h2. Noncompliant Code Example
This noncompliant code example shows a {{getDate()}} accessor method that returns the sole instance of the {{private}} {{Date}} object.
{code:bgColor=#FFcccc}
class MutableClass {
private Date d;
public MutableClass() {
d = new Date();
}
public Date getDate() {
return d;
}
}
{code}
An untrusted caller can manipulate a private {{Date}} object because returning the reference exposes the internal mutable component beyond the trust boundaries of {{MutableClass}}.
h2. Compliant Solution ({{clone()}})
This compliant solution returns a clone of the {{Date}} object from the {{getDate()}} accessor method. This is safe because while {{Date}} can be extended by an attacker, the {{Date}} object returned by {{getDate()}} is controlled by {{MutableClass}} and is known not to be a malicious subclass.
{code:bgColor=#ccccff}
protected Date getDate() {
return (Date)d.clone();
}
{code}
Note that defensive copies performed during execution of a constructor must avoid use of the {{clone()}} method when the class in question may be subclassed by untrusted code. This restriction prevents execution of a maliciously-crafted overriding of the {{clone()}} method. For more details, see [OBJ05-J. Limit extensibility of classes and methods with invariants to trusted subclasses only].
Classes that have {{public}} setter methods must follow the related advice found in guideline [FIO00-J. Defensively copy mutable inputs and mutable internal components]. Note that setter methods can (and usually should) perform input validation and sanitization before setting internal fields.
h2. Noncompliant Code Example (Mutable Member Array)
This guideline also applies to arrays of primitive types. Because Java arrays are objects in their own right, when a caller receives a reference to an array, they can freely modify its contents. Creating a shallow copy of an array of primitive types provides a distinct array instance; although the caller can modify the returned array, they cannot affect the original array.
In this noncompliant code example, the {{getDate()}} accessor method returns an array of {{Date}} objects. The method fails to make a defensive copy of the array before returning it. Because the array contains references to {{Date}} objects that are themselves mutable, a shallow copy of the array would be insufficient, as an attacker can modify the {{Date}} objects in the array.
{code:bgColor=#FFcccc}
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()
}
}
{code}
h2. Compliant Solution (Deep Copy)
This compliant solution creates a deep copy of the {{date}} array and returns the copy, thereby protecting both the {{date}} array, and the individual {{Date}} objects.
{code:bgColor=#ccccff}
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;
}
}
{code}
h2. Noncompliant Code Example (Mutable Member Containing Immutable Objects)
In this noncompliant code example, class {{ReturnRef}} contains a {{private}} {{Hashtable}} instance field. The hash table stores immutable data of a sensitive nature (SSN numbers). A getter method {{getValues()}} gives the caller access to the hash table by returning a reference to it. An untrusted caller can use the getter method to gain access to the hashtable; consequently, hashtable entries can be maliciously added, removed or replaced. Furthermore, these modifications can be done by multiple threads, providing ample opportunities for race conditions.
{code:bgColor=#FFCCCC}
class ReturnRef {
// Internal state, may contain sensitive data
private Hashtable<Integer,String> ht = new Hashtable<Integer,String>();
private ReturnRef() {
ht.put(1, "123-45-6666");
}
public Hashtable<Integer,String> getValues(){
return ht;
}
public static void main(String[] args) {
ReturnRef rr = new ReturnRef();
Hashtable<Integer, String> ht1 = rr.getValues(); // Prints sensitive data 123-45-6666
ht1.remove(1); // Untrusted caller can remove entries
Hashtable<Integer, String> ht2 = rr.getValues(); // Now prints null, original entry is removed
}
}
{code}
In returning a reference to the {{ht}} hashtable, this example also hinders efficient garbage collection. See [OBJ13-J. Write garbage collection friendly code] for more info.
h2. Compliant Solution (Shallow Copy)
Make defensive copies of private internal mutable object state. 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. Consequently, the original hash table remains private, and so any attempts to modify it are ineffective.
{code:bgColor=#ccccff}
class ReturnRef {
// ...
private Hashtable<Integer,String> getValues(){
return (Hashtable<Integer, String>)ht.clone(); // shallow copy
}
public static void main(String[] args) {
ReturnRef rr = new ReturnRef();
Hashtable<Integer,String> ht1 = rr.getValues(); // prints non sensitive data
ht1.remove(1); // untrusted caller can only modify copy
Hashtable<Integer,String> ht2 = rr.getValues(); // prints non sensitive data
}
}
{code}
When a hash table contains references to mutable data such as a series of {{Date}} objects, 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 its keys must produce consistent results to the {{equals()}} and {{hashCode()}} functions. Mutable objects whose {{equals()}} or {{hashCode()}} method results may be modified are not suitable keys.
This example is also more amenable to efficient garbage collection, as described in [OBJ13-J. Write garbage collection friendly code].
h2. Exceptions
*OBJ11-EX1:* According to Sun's Secure Coding Guidelines document \[[SCG 2007|AA. Bibliography#SCG 07]\]
{quote}
if a class merely serves as a container for mutable inputs or outputs (the class does not directly operate on them), it may not be necessary to create defensive copies. For example, arrays and the standard collection classes do not create copies of caller-provided values. If a copy is desired so updates to a value do not affect the corresponding value in the collection, the caller must create the copy before inserting it into the collection, or after receiving it from the collection.
{quote}
{mc} this seems to be a weasal exception ~DS
*OBJ11-EX2:* If the performance of the copy method is within reasonable bounds and the class clearly documents its use, this guideline may be violated. (See guideline [OBJ10-J. Provide mutable classes with copy functionality to allow passing instances to untrusted code safely].)
{mc}
*OBJ11-EX2:* If the caller exposes anWhen callers expose to a method only unmodifiable viewviews of thean object, the program need not defensively copy the objectmethod may freely use the unmodifiable view without defensive copying. This decision should be made early in the design of the API. Note that ifnew somecallers otherof codesuch wants to use this class in the future, it methods must also expose only unmodifiable views. (See guideline [SEC14-J. Provide sensitive mutable classes with unmodifiable wrappers].)
h2. Risk Assessment
Returning references to internal object state (mutable or immutable) can render an application susceptible to information leaks and to corruption of its objects' states that consequently violates class invariants. Control flow can also be affected in some cases.
|| Guideline || Severity || Likelihood || Remediation Cost || Priority || Level ||
| OBJ11-J | high | probable | medium | {color:red}{*}P12{*}{color} | {color:red}{*}L1{*}{color} |
h3. Automated Detection
Sound automated detection is infeasible; heuristic checks may be useful.
h3. Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this guideline on the [CERT website|https://www.kb.cert.org/vulnotes/bymetric?searchview&query=FIELD+KEYWORDS+contains+OBJ37-J].
h2. Other Languages
This guideline appears in the C++ Secure Coding Standard as [cplusplus:OOP35-CPP. Do not return references to private data].
h2. Bibliography
\[[API 2006|AA. Bibliography#API 06]\] [method clone()|http://java.sun.com/javase/6/docs/api/java/lang/Object.html#clone()]
\[[Bloch 2008|AA. Bibliography#Bloch 08]\] Item 39: Make defensive copies when needed
\[[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
\[[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]
\[[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]\]
----
[!The CERT Oracle Secure Coding Standard for Java^button_arrow_left.png!|OBJ10-J. Provide mutable classes with copy functionality to allow passing instances to untrusted code safely] [!The CERT Oracle Secure Coding Standard for Java^button_arrow_up.png!|04. Object Orientation (OBJ)] [!The CERT Oracle Secure Coding Standard for Java^button_arrow_right.png!|VOID OBJ12-J. Use checked collections against external code]
| 