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