Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Parasoft Jtest 2021.1

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 data. 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 defines a clone() method that trusted code can use to pass defensive copies of the instance to untrusted code (OBJ10-J. Provide mutable classes with a clone method 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 call the clone() method before operating on the instance or passing it to untrusted code.

Note that the performance costs of violating this guideline (or calling 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 clone() method returns a copy of the complete object, including all mutable components) as opposed to that of a single member, which makes it relatively slower.

Wiki Markup
Pugh \[[Pugh 09|AA. Java References#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

an application's security, both by breaking encapsulation and by providing the opportunity to corrupt the internal state of the class (whether accidentally or maliciously). As a result, programs must not return references to private mutable classes.

See rule OBJ13-J. Ensure that references to mutable objects are not exposed for details about leaking references to non-private objects.

Noncompliant Code Example

This noncompliant code example shows a getDate() accessor method that returns the sole instance of the private Date object:

Code Block
bgColor#FFcccc
class MutableClass {
  private Date d;

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

  public Date getDate() {
    return d;
  }
}

An untrusted caller can manipulate a private Date object because returning the reference exposes the internal mutable component beyond the trust boundaries of MutableClass.

Compliant Solution (clone())

Returning a reference to a defensive copy of a mutable internal state ensures that the caller cannot modify the original internal state, although the copy remains mutable. This compliant solution returns a clone of the Date object from the getDate() accessor method. Although Date can be extended by an attacker, this approach is safe because the Date object returned by getDate() is controlled by MutableClass and is known to be a nonmalicious subclass.

Code Block
bgColor#ccccff
public Date getDate() {
  return (Date)d.clone();
}

Note that defensive copies performed during execution of a constructor must avoid use of the clone() method when the class could be subclassed by untrusted code. This restriction prevents execution of a maliciously crafted overriding of the clone() method (see OBJ07-J. Sensitive classes must not let themselves be copied for more details).

Classes that have public setter methods, that is, methods whose purpose is to change class fields, must follow the related advice found in OBJ06-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.

Noncompliant Code Example (Mutable Member 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 mutable, a shallow copy of the array is insufficient because an attacker can modify the Date objects in the array.

Code Block
bgColor#FFcccc
class MutableClass {
  private Date[] date;

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

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

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 Block
bgColor#ccccff
class MutableClass {
  private Date[] date;

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

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

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 but sensitive data (for example, social security numbers [SSNs]). The getValues() method This noncompliant code example defines a class that contains a private Hashtable instance field. Here, the hash table itself is designed to hold 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 this method to gain access to the hash table; as a result, hash table entries can be maliciously added, removed, or replaced. Furthermore, multiple threads can perform these modifications, providing ample opportunities for race conditions.

Code Block
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
  }	
}

Compliant Solution

In returning a reference to the ht hash table, this example also hinders efficient garbage collection.

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 copyDo not provide a getter method like getValues() that exposes private internal object state. This applies largely to members containing mutable as well as immutable data. Deep copies of mutable data are required to be returned whereas it suffices to return shallow copies of mutable fields containing immutable data.

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

Code Block
bgColor#ccccff
class ReturnRef {
  // ...
  private Hashtable<Integer,String> getValues(){
    return (Hashtable<Integer, String>) ht.clone(); // shallowShallow copy
  }

  public static void main(String[] args) {
    ReturnRef rr = new ReturnRef();
    // Prints nonsensitive data
    Hashtable<Integer,String> ht1 = rr.getValues(); 
    // Untrusted printscaller can nononly sensitivemodify datacopy
    ht1.remove(1); //
 untrusted caller can remove// entriesPrints onlynonsensitive fromdata
 the copy
  Hashtable<Integer,String> ht2 = rr.getValues(); //
 prints non sensitive data      }
}

If the When a hash table contained contains 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 FIO00 OBJ06-J. Defensively copy mutable inputs and mutable internal components and OBJ10OBJ04-J. Provide mutable classes with a clone method copy functionality to safely allow passing instances to untrusted code safely for further details).

Note that making deep copies of the keys of a hash table need not be deep copiedis unnecessary; shallow copying of the references suffices because a hash table's contract dictates that it cannot hold duplicate keys.

Noncompliant Code Example

This noncompliant code example shows a getDate() accessor method that returns the sole instance of the private Date object. An untrusted caller will be able to manipulate this instance as it exposes internal mutable components beyond the trust boundaries of the class.

Code Block
bgColor#FFcccc

class MutableClass {
  private Date d;

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

  protected Date getDate() {
    return d;
  }
}

Compliant Solution

Do not carry out defensive copying using the clone() method in constructors, when the (non-system) class can be subclassed by untrusted code. This will limit the malicious code from returning a crafted object when the object's clone() method is invoked.

Despite this advice, 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 extending the internal mutable Date object (Date is a system class and consequently safe).

Code Block
bgColor#ccccff

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

Arrays of primitive types are not exempt from this guideline. As arrays are objects in Java, if a reference to an array is returned, the caller may freely modify the contents of the original array. Shallow copies of arrays of primitive types are safe to return.

If the class has a public setter method, this guideline still applies. Note that a setter method may perform input validation and sanitization before setting the internal fields. On the other hand, returning references to internal objects may require the caller to incorporate none of these defensive measures.

Exceptions

Wiki Markup
*EX1:* According to Sun's Secure Coding Guidelines document \[[SCG 07|AA. Java References#SCG 07]\]:

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.

EX2: If the performance of the clone() method is within reasonable bounds and the class clearly documents its use, this guideline may be violated. (OBJ10-J. Provide mutable classes with a clone method to allow passing instances to untrusted code safely)

its keys must produce consistent results to the equals() and hashCode() methods. Mutable objects whose equals() or hashCode() method results may be modified are not suitable keys.

Exceptions

OBJ05-J-EX0: When a method is called with only an immutable view of an object, that method may freely use the immutable view without defensive copyingEX3: If the caller exposes an unmodifiable view of the object, it may not be required to defensively program the class to return copies of internal members. This decision should be made early in the design of the API. Note that if some other code wants to use this class in the future, it new callers of such methods must also expose unmodifiable views. (SEC14-J. Provide sensitive mutable classes with unmodifiable wrappers)only immutable views.

Risk Assessment

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

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

OBJ37

OBJ05-J

high

High

probable

Probable

medium

Medium

P12

L1

Automated Detection

...

TODO

Related Vulnerabilities

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

Other Languages

...

Sound automated detection is infeasible; heuristic checks could be useful.

ToolVersionCheckerDescription
Parasoft Jtest
Include Page
Parasoft_V
Parasoft_V
CERT.OBJ05.CPCL
CERT.OBJ05.MPT
CERT.OBJ05.SMO
CERT.OBJ05.MUCOP
Enforce returning a defensive copy in 'clone()' methods
Do not pass user-given mutable objects directly to certain types
Do not store user-given mutable objects directly into variables
Provide mutable classes with copy functionality
SonarQube
Include Page
SonarQube_V
SonarQube_V
S2384

Mutable members should not be stored or returned directly

Implemented for Arrays, Collections and Dates.

Related Vulnerabilities

Pugh [Pugh 2009] cites a vulnerability discovered by the Findbugs static analysis tool in the early betas of JDK 1.7 in which the sun.security.x509.InvalidityDateExtension class returned a Date instance through a public accessor without creating defensive copies.

Related Guidelines

...

...

Wiki Markup
\[[API 06|AA. Java References#API 06]\] [method clone()|http://java.sun.com/javase/6/docs/api/java/lang/Object.html#clone()]
\[[Security 06|AA. Java References#Security 06]\]
\[[Bloch 08|AA. Java References#Bloch 08]\] Item 39: Make defensive copies when needed
\[[SCG 07|AA. Java References#SCG 07]\] Guideline 2-1 Create a copy of mutable inputs and outputs
\[[Haggar 00|AA. Java References#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 06|AA. Java References#Goetz 06]\] 3.2. Publication and Escape: Allowing Internal Mutable State to Escape
\[[Gong 03|AA. Java References#Gong 03]\] 9.4 Private Object State and Object Immutability
\[[MITRE 09|AA. Java References#MITRE 09]\] [CWE ID 375|http://cwe.mitre.org/data/definitions/375.html] "Passing Mutable Objects to an Untrusted Method"

CWE-375, Returning a Mutable Object to an Untrusted Caller

Bibliography

[API 2014]

Method clone()

[Bloch 2008]

Item 39, "Make Defensive Copies When Needed"

[Goetz 2006]

Section 3.2, "Publication and Escape: Allowing Internal Mutable State to Escape"

[Gong 2003]

Section 9.4, "Private Object State and Object Immutability"

[Haggar 2000]

Practical Java Praxis 64. Use clone for immutable objects when passing or receiving object references to mutable objects

[Pugh 2009]

[Security 2006]



...

Image Added Image Added Image AddedOBJ10-J. Provide mutable classes with a clone method to allow passing instances to untrusted code safely      08. Object Orientation (OBJ)      OBJ09-J. Immutable classes must prohibit extension