Returning references to internal object state ( mutable or immutable fields and members, alike) 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 invariants inflicted by the clients of the codedata. A caller is able to modify the private
data if defensive copies of mutable class members are not returned.
Returning references to private
data to untrusted code can be more pernicious than returning 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. 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.
Noncompliant Code Example
This noncompliant piece of 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
SSN numbers). A getter method getValues()
is also provided to give gives the caller access to the hash table by returning a reference to it. This is dangerous since hostile code can surreptitiously view the contents of the object's private data. Not only that, it can also add, remove or modify the hash table entries at its willWhen invoked by untrusted code, entries may be maliciously added, removed or replaced.
Code Block | ||
---|---|---|
| ||
class ReturnRef { // internal state, may contain sensitive data Hashtable<Integer,String> ht = new Hashtable<Integer,String>(); private ReturnRef() { ht.put(1, "123-45-6666"); } privatepublic 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(); // 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 removed! } } |
Compliant Solution
Do not provide a getter method like getValues()
that exposes private
internal object state. This applies largely to both members containing mutable as well as immutable instance fields and objects. The former can allow untrusted code to alter the state of the object while the latter can leak private information by breaking the encapsulation property. data. Deep copies of mutable data are required to be returned whereas it suffices to return shallow copies of mutable fields containing immutable data.
If some internal object contains immutable data that is not of sensitive nature, create and return its shallow copy as the caller cannot modify the originally referenced data. This solution highlights how a shallow copy of the internal hash table containing immutable SSN numbers can be created and returned. ErgoAs a result, any attempts to modify the original hash table run into a dry stretchare 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 } |
At other times, a deep copy may be desirable. This condition applies to all objects whose state comprises mutable data. For instance, if If the hash table contained references to mutable data such as a series of Date
objects, every one of those objects will have to must be copied by using a copy constructor or method. For further details, refer to FIO31-J. Defensively copy mutable inputs and mutable internal components and OBJ36-J. Provide mutable classes with a clone method 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 since because a hash table's contract dictates that it cannot hold duplicate keys.
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 allow the caller to incorporate none of these 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. (OBJ36-J. Provide mutable classes with a clone method to allow passing instances to untrusted code safely)
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 interal 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. (SEC01-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 leakage leaks and corrupt the object's state and invariants. Control flow may also be affected in dire cases.
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
OBJ37- J | high | probable | medium | P12 | L1 |
Automated Detection
TODO
Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this rule on the CERT website.
Other Languages
This rule appears in the C++ Secure Coding Standard as OBJ35-CPP. Do not return references to private data.
References
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
\[[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" |
...