...
Returning
...
references
...
to
...
internal
...
mutable
...
members
...
of
...
a
...
class
...
can
...
compromise
...
an
...
application's
...
security,
...
both
...
by
...
breaking
...
encapsulation
...
and
...
also
...
by
...
providing
...
the
...
opportunity
...
to
...
corrupt
...
the
...
internal
...
state
...
of
...
the
...
class
...
(whether
...
accidentally
...
or
...
maliciously).
...
Performing
...
a
...
defensive
...
copy
...
before
...
returning
...
a
...
reference
...
to
...
a
...
mutable
...
internal
...
state
...
ensures
...
that
...
the
...
caller
...
can
...
only
...
modify
...
the
...
copy
...
and
...
not
...
the
...
original
...
internal
...
state.
...
| Wiki Markup |
|---|
Pugh \[[Pugh 2009|AA. Bibliography#Pugh 09]\] cites a vulnerability discovered by the Findbugs static analysis tool in the early betas of JDK 1.7 where the {{sun.security.x509.InvalidityDateExtension}} class returned a {{Date}} instance through a {{public}} accessor without creating defensive copies. |
...
Noncompliant
...
Code
...
Example
...
This
...
noncompliant
...
code
...
example
...
shows
...
a
...
getDate()
...
accessor
...
method
...
that
...
returns
...
the
...
sole
...
instance
...
of
...
the
...
private
...
Date
...
object.
| Code Block | ||||
|---|---|---|---|---|
| =
| |||
} class MutableClass { private Date d; public MutableClass() { d = new Date(); } public Date getDate() { return d; } } {code} |
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()
...
)
...
This
...
compliant
...
solution
...
returns
...
a
...
clone
...
of
...
the
...
Date
...
object
...
from
...
the
...
getDate()
...
accessor
...
method.
...
This
...
is
...
safe
...
because
...
while
...
Date
...
can
...
be
...
extended
...
by
...
an
...
attacker,
...
the
...
Date
...
object
...
returned
...
by
...
getDate()
...
is
...
controlled
...
by
...
MutableClass
...
and
...
is
...
known
...
not
...
to
...
be
...
a
...
malicious
...
subclass.
| Code Block | ||||
|---|---|---|---|---|
| =
| |||
} protected Date getDate() { return (Date)d.clone(); } {code} |
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.
...
For
...
more
...
details,
...
see
...
rule
...
"
...
...
...
...
...
...
...
...
...
...
...
...
...
...
."
...
Classes
...
that
...
have
...
public
...
setter
...
methods
...
must
...
follow
...
the
...
related
...
advice
...
found
...
in
...
rule
...
"
...
...
...
...
...
...
...
...
...
...
...
."
...
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
...
would
...
be
...
insufficient
...
because
...
an
...
attacker
...
can
...
modify
...
the
...
Date
...
objects
...
in
...
the
...
array.
| Code Block | ||||
|---|---|---|---|---|
| =
| |||
} class MutableClass { private Date[] date; public MutableClass() { for (int i = 0; i < 20; i++) date[i] = new Date(); } public Date[] getDate() { return date; // or return date.clone() } } {code} h2. Compliant Solution |
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 | ||
|---|---|---|
| ||
the {{date}} array and returns the copy, thereby protecting both the {{date}} array and the individual {{Date}} objects. {code: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; } } {code} h2. Noncompliant Code Example |
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
...
(SSNs).
...
A
...
getter
...
method
...
getValues()
...
gives
...
the
...
caller
...
access
...
to
...
the
...
hash
...
table
...
by
...
returning
...
a
...
reference
...
to
...
it.
...
An
...
untrusted
...
caller
...
can
...
use
...
the
...
getter
...
method
...
to
...
gain
...
access
...
to
...
the
...
hash
...
table;
...
as
...
a
...
result,
...
hash
...
table
...
entries
...
can
...
be
...
maliciously
...
added,
...
removed,
...
or
...
replaced.
...
Furthermore,
...
these
...
modifications
...
can
...
be
...
performed
...
by
...
multiple
...
threads,
...
providing
...
ample
...
opportunities
...
for
...
race
...
conditions.
| Code Block | ||||
|---|---|---|---|---|
| =
| |||
} 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 } } {code} |
In
...
returning
...
a
...
reference
...
to
...
the ht hash table,
...
this
...
example
...
also
...
hinders
...
efficient
...
garbage
...
collection.
...
See
...
rule
...
"
...
...
...
...
...
...
"
...
for
...
more
...
information.
...
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
...
copy.
...
This
...
compliant
...
solution
...
creates
...
and
...
returns
...
a
...
shallow
...
copy
...
of
...
the
...
hash
...
table
...
containing
...
immutable
...
SSNs.
...
Consequently,
...
the
...
original
...
hash
...
table
...
remains
...
private,
...
and
...
any
...
attempts
...
to
...
modify
...
it
...
are
...
ineffective.
| Code Block | ||
|---|---|---|
| ||
{code:bgColor=#ccccff} class ReturnRef { // ... private Hashtable<Integer,String> getValues(){ 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 only modify copy Hashtable<Integer,String> ht2 = rr.getValues(); // prints non-sensitive data } } {code} |
When
...
a
...
hash
...
table
...
contains
...
references
...
to
...
mutable
...
data
...
such
...
as
...
a
...
series
...
of
...
Date
...
objects,
...
each
...
of
...
those
...
objects
...
must
...
also
...
be
...
copied
...
by
...
using
...
a
...
copy
...
constructor
...
or
...
method.
...
For
...
further
...
details,
...
refer
...
to
...
rules
...
"
...
...
...
...
...
...
...
...
...
...
"
...
and
...
"
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
."
Note that the keys of a hash table do not need to be deep copied; shallow copying of the references suffices because a hash table's contract dictates that its keys must produce consistent results to the equals() and hashCode() functions. Mutable objects whose equals() or hashCode() method results may be modified are not suitable keys.
This example is also more amenable to efficient garbage collection, as described in rule "OBJ11-J. Write garbage-collection-friendly code."
Exceptions
| Wiki Markup |
|---|
*OBJ05-EX0:* According to Sun's Secure Coding Guidelines \[[SCG 2007|AA. Bibliography#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.
...
OBJ05-EX1:
...
When callers expose only unmodifiable views of an object to a method, the method may freely use the unmodifiable view without defensive copying. This decision should be made early in the design of the API. Note that new callers of such methods must also expose only unmodifiable views. (See rule "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' states, which, consequently, violates class invariants. Control flow can also be affected in some cases.
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
|---|---|---|---|---|---|
OBJ05-J | high | probable | medium | P12 | L1 |
Automated Detection
Sound automated detection is infeasible; heuristic checks could be useful.
Related Guidelines
CWE-375 "Returning a Mutable Object to an Untrusted Caller" | |
Guideline 2-1 Create a copy of mutable inputs and outputs |
Bibliography
<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="540e41aa-6e54-43f5-8655-d0a16a1d66e8"><ac:plain-text-body><![CDATA[ | [[API 2006 | AA. Bibliography#API 06]] | [method clone() | http://java.sun.com/javase/6/docs/api/java/lang/Object.html#clone()] | ]]></ac:plain-text-body></ac:structured-macro> |
<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="fa823fac-c48d-40e7-b2c2-6fea2145c402"><ac:plain-text-body><![CDATA[ | [[Bloch 2008 | AA. Bibliography#Bloch 08]] | Item 39: Make defensive copies when needed | ]]></ac:plain-text-body></ac:structured-macro> | |
<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="fb2ecde8-e93e-4003-8a0e-9402e76cadf7"><ac:plain-text-body><![CDATA[ | [[Goetz 2006 | AA. Bibliography#Goetz 06]] | 3.2. Publication and Escape: Allowing Internal Mutable State to Escape | ]]></ac:plain-text-body></ac:structured-macro> | |
<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="98c01502-b7c1-41cf-94a6-1528c63bb441"><ac:plain-text-body><![CDATA[ | [[Gong 2003 | AA. Bibliography#Gong 03]] | 9.4 Private Object State and Object Immutability | ]]></ac:plain-text-body></ac:structured-macro> | |
<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="fad0d724-190a-462a-8a89-188c0367aac4"><ac:plain-text-body><![CDATA[ | [[Haggar 2000 | AA. Bibliography#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] | ]]></ac:plain-text-body></ac:structured-macro> |
<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="7ef739fa-10a3-4a3d-b810-d1debd0446ff"><ac:plain-text-body><![CDATA[ | [[Security 2006 | AA. Bibliography#Security 06]] |
| ]]></ac:plain-text-body></ac:structured-macro> |
...
OBJ04-J. Provide mutable classes with copy functionality to allow passing instances to untrusted code safely 04. Object Orientation (OBJ)