Versions Compared

Key

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

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 is able to modify the private data if instead of defensive copies of mutable class members, direct references to them are not 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 (OBJ36-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.

...

Code Block
bgColor#FFCCCC
class ReturnRef {
  // internalInternal state, may contain sensitive data
  Hashtable<Integer,String> ht = new Hashtable<Integer,String>(); 
 
  private ReturnRef() {
    ht.put(1, "123-45-6666");
  }
 
  public Hashtable<Integer,String> getValues(){
    System.out.println(ht.get(1));    
    return ht;
  }
 
  public static void main(String[] args) {
    ReturnRef rr = new ReturnRef();
    Hashtable<Integer, String> ht1 = rr.getValues(); // printsPrints sensitive data 123-45-6666
    ht1.remove(1); // untrustedUntrusted caller can remove entries
    Hashtable<Integer, String> ht2 = rr.getValues(); // nowNow prints null, original entry is removed!
  }	
}

Compliant Solution

Do 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 highlights how creates and returns a shallow copy of the hash table containing immutable SSN numbers can be created and returned. As a result, any attempts to modify the original hash table are ineffective.

Code Block
bgColor#ccccff
private Hashtable<Integer,String> getValues(){
  System.out.println(ht.get(1));
  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 remove entries only from the copy
  Hashtable<Integer,String> ht2 = rr.getValues(); // prints non sensitive data     
}

...

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 the 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;
  }
}

...

Do not carry out defensive copying using the clone() method in constructors, where 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 it 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 , therefore and consequently safe).

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

...

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 allow 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]\]:

...

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 class invariants. Control flow may also be affected in dire some cases.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

OBJ37- J

high

probable

medium

P12

L1

...