It is difficult to control how data members declared public
or protected
are accessed. Attackers can manipulate such members in unexpected ways. Use wrapper accessor methods to expose class members that are beyond the package in which their class is declared. Using wrapper methods enables appropriate monitoring and control of the modification of data members (for example, by defensive copying, validating input, and logging). The wrapper methods can preserve class invariants.
Noncompliant Code Example (Public Primitive Field)
In this noncompliant code example, the data member total
keeps track of the total number of elements as they are added and removed from a container using the methods add()
and remove()
, respectively.
public class Widget { public int total; // Number of elements void add() { if (total < Integer.MAX_VALUE) { total++; // ... } else { throw new ArithmeticException("Overflow"); } } void remove() { if (total > 0) { total--; // ... } else { throw new ArithmeticException("Overflow"); } } }
As a public
data member, total
can be altered by external code, independent of the add()
and remove()
methods. It is a bad practice to expose both mutable and immutable fields from a public
class [[Bloch 2008]].
Compliant Solution (private
)
This compliant solution declares total
as private
and provides a public
accessor so that the required member can be accessed beyond the current package. The add()
and remove()
methods modify its value without violating any class invariants.
Note that accessor functions should be careful about providing references to private mutable objects; see rule "OBJ05-J. Defensively copy private mutable class members before returning their references" for details.
public class Widget { private int total; // Declared private public int getTotal () { return total; } // definitions for add() and remove() remain the same }
It is good practice to use methods such as add()
, remove()
, and getTotal()
to manipulate the private internal state. These methods can perform additional functions, such as input validation and security manager checks, prior to manipulating the state.
Noncompliant Code Example (Public Mutable Field)
This noncompliant code example shows a static
mutable hash map with public
accessibility.
public static final HashMap<Integer, String> hm = new HashMap<Integer, String>();
Compliant Solution (Provide Wrappers and Reduce Accessibility of Mutable Members)
Mutable data members that are static
must be declared private
.
private static final HashMap<Integer, String> hm = new HashMap<Integer, String>(); public static String getElement(int key) { return hm.get(key); }
Depending on the required functionality, wrapper methods may retrieve either a reference to the Hashmap
, a copy of the HashMap
, or a value contained by the Hashmap
. This compliant solution adds a wrapper method to return the value of an element given its index in the Hashmap
.
Exceptions
OBJ01-EX0: According to Sun's Code Conventions document [[Conventions 2009]]:
One example of appropriate
public
instance variables is the case where the class is essentially a data structure, with no behavior. In other words, if you would have used astruct
instead of a class (if Java supportedstruct
), then it's appropriate to make the class' instance variablespublic
.
OBJ01-EX1: "If a class is package-private or is a private
nested class, there is nothing inherently wrong with exposing its data fields — assuming they do an adequate job of describing the abstraction provided by the class. This approach generates less visual clutter than the accessor-method approach, both in the class definition and in the client code that uses it" [[Bloch 2008]]. This exception applies to both mutable and immutable fields.
OBJ01-EX2: Static final fields that contain mathematical constants may be declared public.
Risk Assessment
Failing to declare data members private
can break encapsulation.
Rule |
Severity |
Likelihood |
Remediation Cost |
Priority |
Level |
---|---|---|---|---|---|
OBJ01-J |
medium |
likely |
medium |
P12 |
L1 |
Automated Detection
Detection of public and protected data members is trivial; heuristic detection of the presence or absence of getter and setter wrapper methods is straightforward. However, simply reporting all detected cases without suppressing those cases covered by the exceptions to this rule would produce many false positives and appears unlikely to be satisfactory. Sound detection and application of the exceptions to this rule appears to be infeasible; heuristic techniques may be useful.
Related Guidelines
C++ Secure Coding Standard |
||||
<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="c5ff41d4-627b-48ed-a79a-9cc1622f659a"><ac:plain-text-body><![CDATA[ |
[[SCG 2009 |
AA. Bibliography#SCG 09]] |
Guideline 3-2: "Define wrapper methods around modifiable internal state" |
]]></ac:plain-text-body></ac:structured-macro> |
CWE ID 766, "Critical Variable Declared Public" |
Bibliography
<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="f1cea04f-c89b-4fba-b64b-f2cd93278b7f"><ac:plain-text-body><![CDATA[ |
[[Bloch 2008 |
AA. Bibliography#Bloch 08]] |
Items 13: Minimize the accessibility of classes and members; 14: In public classes, use accessor methods, not public fields |
]]></ac:plain-text-body></ac:structured-macro> |
|
<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="709c19ee-7585-4e13-b95e-10a10a1d809c"><ac:plain-text-body><![CDATA[ |
[[JLS 2005 |
AA. Bibliography#JLS 05]] |
[§6.6 "Access Control" |
http://java.sun.com/docs/books/jls/third_edition/html/names.html#6.6] |
]]></ac:plain-text-body></ac:structured-macro> |
<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="09546dd9-bd8a-46e7-a8ec-5f61bd2cab50"><ac:plain-text-body><![CDATA[ |
[[Long 2005 |
AA. Bibliography#Long 05]] |
§2.2, Public Fields |
]]></ac:plain-text-body></ac:structured-macro> |
OBJ00-J. Limit extensibility of classes and methods with invariants to trusted subclasses only 04. Object Orientation (OBJ) OBJ02-J. Minimize the accessibility of classes and their members