Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

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.

...

Wiki Markup
Pugh \[[Pugh 2009|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 (

...

Mutable Member Containing Immutable Objects)

This noncompliant code example defines a class that 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 be maliciously added, removed or replaced.

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 (

...

Shallow Copying)

Do not provide a getter method like getValues() that exposes 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.

...

If the hash table contained references to mutable data such as a series of Date objects, every one of those objects must 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.

Noncompliant Code Example (

...

Mutable Member)

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.

Compliant Solution (

...

Shallow Copying Using clone() for

...

Final Classes)

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();
}

...

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 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 noncompliant code example consists of an array of Date objects.

...

It does not defensively copy the array before returning it. A shallow copy of the array may also be inappropriate in this case because Date is mutable.

Compliant Solution (

...

Deep Copying)

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

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

Exceptions

Wiki Markup
*OBJ11-EX1:* According to Sun's Secure Coding Guidelines document \[[SCG 2007|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.

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.)

OBJ11-EX3: 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 must also expose unmodifiable views. (See guideline SEC14-J. Provide sensitive mutable classes with unmodifiable wrappers.)

Risk Assessment

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

Rule Guideline

Severity

Likelihood

Remediation Cost

Priority

Level

OBJ11-J

high

probable

medium

P12

L1

...