Java coding guidelines: 75 recommendations for reliable and secure programs (2014)
Chapter 2. Defensive Programming
Defensive programming is carefully guarded programming that helps you to construct reliable software by designing each component to protect itself as much as possible: for example, by checking that undocumented assumptions remain valid [Goodliffe 2007]. The guidelines in this chapter address areas of the Java language that can help to constrain the effect of an error or help to recover from an error.
Java language mechanisms should be used to limit the scope, lifetime, and accessibility of program resources. Also, Java annotations can be used to document the program, aiding readability and maintenance. Java programmers should be aware of implicit behaviors and avoid unwarranted assumptions about how the system behaves.
A good overall principle for defensive programming is simplicity. A complicated system is difficult to understand, difficult to maintain, and difficult to get right in the first place. If a construct turns out to be complicated to implement, consider redesigning or refactoring it to reduce the complexity.
Finally, the program should be designed to be as robust as possible. Wherever possible, the program should help the Java runtime system by limiting the resources it uses and by releasing acquired resources when they are no longer needed. Again, this can often be achieved by limiting the lifetime and accessibility of objects and other programming constructs. Not all eventualities can be anticipated, so a strategy should be developed to provide a graceful exit of last resort.
22. Minimize the scope of variables
Scope minimization helps developers avoid common programming errors, improves code readability by connecting the declaration and actual use of a variable, and improves maintainability because unused variables are more easily detected and removed. It may also allow objects to be recovered by the garbage collector more quickly, and it prevents violations of Guideline 37, “Do not shadow or obscure identifiers in subscopes.”
Noncompliant Code Example
This noncompliant code example shows a variable that is declared outside the for loop.
public class Scope {
public static void main(String[] args) {
int i = 0;
for (i = 0; i < 10; i++) {
// Do operations
}
}
}
This code is noncompliant because, even though variable i is not intentionally used outside the for loop, it is declared in method scope. One of the few scenarios where variable i needs to be declared in method scope is when the loop contains a break statement, and the value of i must be inspected after conclusion of the loop.
Compliant Solution
Minimize the scope of variables where possible. For example, declare loop indices within the for statement:
public class Scope {
public static void main(String[] args) {
for (int i = 0; i < 10; i++) { // Contains declaration
// Do operations
}
}
}
Noncompliant Code Example
This noncompliant code example shows a variable count that is declared outside the counter() method, although the variable is not used outside the counter() method.
public class Foo {
private int count;
private static final int MAX_COUNT = 10;
public void counter() {
count = 0;
while (condition()) {
/* ... */
if (count++ > MAX_COUNT) {
return;
}
}
}
private boolean condition() {/* ... */}
// No other method references count
// but several other methods reference MAX_COUNT
}
The reusability of the method is reduced because if the method were copied to another class, then the count variable would also need to be redefined in the new context. Furthermore, the analyzability of the counter method would be reduced, as whole program data flow analysis would be necessary to determine possible values for count.
Compliant Solution
In this compliant solution, the count field is declared local to the counter() method:
public class Foo {
private static final int MAX_COUNT = 10;
public void counter() {
int count = 0;
while (condition()) {
/* ... */
if (count++ > MAX_COUNT) {
return;
}
}
}
private boolean condition() {/* ... */}
// No other method references count
// but several other methods reference MAX_COUNT
}
Applicability
Detecting local variables that are declared in a larger scope than is required by the code as written is straightforward and can eliminate the possibility of false positives.
Detecting multiple for statements that use the same index variable is straightforward; it produces false positives only in the unusual case where the value of the index variable is intended to persist between loops.
Bibliography
[Bloch 2001]
Item 29, “Minimize the Scope of Local Variables”
[JLS 2013]
§14.4, “Local Variable Declaration Statements”
23. Minimize the scope of the @SuppressWarnings annotation
When the compiler detects potential type-safety issues arising from mixing raw types with generic code, it issues unchecked warnings, including unchecked cast warnings, unchecked method invocation warnings, unchecked generic array creation warnings, and unchecked conversion warnings [Bloch 2008]. It is permissible to use the @SuppressWarnings("unchecked") annotation to suppress unchecked warnings when, and only when, the warning-emitting code is guaranteed to be type safe. A common use case is mixing legacy code with new client code. The perils of ignoring unchecked warnings are discussed extensively in The CERT® Oracle® Secure Coding Standard for Java™ [Long 2012], “OBJ03-J. Do not mix generic with non-generic raw types in new code.”
According to the Java API Annotation Type SuppressWarnings documentation [API 2013],
As a matter of style, programmers should always use this annotation on the most deeply nested element where it is effective. If you want to suppress a warning in a particular method, you should annotate that method rather than its class.
The @SuppressWarnings annotation can be used in the declaration of variables and methods as well as an entire class. It is, however, important to narrow its scope so that only those warnings that occur in the narrower scope are suppressed.
Noncompliant Code Example
In this noncompliant code example, the @SuppressWarnings annotation’s scope encompasses the whole class:
@SuppressWarnings("unchecked")
class Legacy {
Set s = new HashSet();
public final void doLogic(int a, char c) {
s.add(a);
s.add(c); // Type-unsafe operation, ignored
}
}
This code is dangerous because all unchecked warnings within the class are suppressed. Oversights of this nature can result in a ClassCastException at runtime.
Compliant Solution
Limit the scope of the @SuppressWarnings annotation to the nearest code that generates a warning. In this case, it may be used in the declaration for the Set:
class Legacy {
@SuppressWarnings("unchecked")
Set s = new HashSet();
public final void doLogic(int a, char c) {
s.add(a); // Produces unchecked warning
s.add(c); // Produces unchecked warning
}
}
Noncompliant Code Example (ArrayList)
This noncompliant code example is from an old implementation of java.util .ArrayList:
@SuppressWarnings("unchecked")
public <T> T[] toArray(T[] a) {
if (a.length < size) {
// Produces unchecked warning
return (T[]) Arrays.copyOf(elements, size, a.getClass());
}
// ...
}
When the class is compiled, it emits an unchecked cast warning:
// Unchecked cast warning
ArrayList.java:305: warning: [unchecked] unchecked cast found :
Object[], required: T[]
return (T[]) Arrays.copyOf(elements, size, a.getClass());
This warning cannot be suppressed for just the return statement because it is not a declaration [JLS 2013]. As a result, the programmer suppresses warnings for the entire method. This can cause issues when functionality that performs type-unsafe operations is added to the method at a later date [Bloch 2008].
Compliant Solution (ArrayList)
When it is impossible to use the @SuppressWarnings annotation in an appropriate scope, as in the preceding noncompliant code example, declare a new variable to hold the return value and adorn it with the @SuppressWarnings annotation.
// ...
@SuppressWarnings("unchecked")
T[] result = (T[]) Arrays.copyOf(elements, size, a.getClass());
return result;
// ...
Applicability
Failure to reduce the scope of the @SuppressWarnings annotation can lead to runtime exceptions and break type-safety guarantees.
This rule cannot be statically enforced in full generality; however, static analysis can be used for some special cases.
Bibliography
[API 2013]
Annotation Type SuppressWarnings
[Bloch 2008]
Item 24, “Eliminate Unchecked Warnings”
[Long 2012]
OBJ03-J. Do not mix generic with nongeneric raw types in new code
24. Minimize the accessibility of classes and their members
Classes and class members (classes, interfaces, fields, and methods) are access controlled in Java. Access is indicated by an access modifier (public, protected, or private) or by the absence of an access modifier (the default access, also called package-private access).
Table 2–1 presents a simplified view of the access control rules. An x indicates that the particular access is permitted from within that domain. For example, an x in the class column means that the class member is accessible to code present within the same class in which it is declared. Similarly, the package column indicates that the member is accessible from any class (or subclass) defined in the same package, provided that the class (or subclass) is loaded by the class loader that loaded the class containing the member. The same class loader condition applies only to package-private member access.
Table 2–1. Access control rules
Classes and class members must be given the minimum possible access so that malicious code has the least opportunity to compromise security. As far as possible, classes should avoid exposing methods that contain (or invoke) sensitive code through interfaces; interfaces allow only publicly accessible methods, and such methods are part of the public application programming interface (API) of the class. (Note that this is the opposite of Joshua Bloch’s recommendation to prefer interfaces for APIs [Bloch 2008, Item 16].) One exception to this is implementing anunmodifiable interface that exposes a public immutable view of a mutable object. (See The CERT® Oracle® Secure Coding Standard for Java™ [Long 2012], “OBJ04-J. Provide mutable classes with copy functionality to safely allow passing instances to untrusted code.”) Note that even if a nonfinal class’s visibility is default, it can be susceptible to misuse if it contains public methods. Methods that perform all necessary security checks and sanitize all inputs may be exposed through interfaces.
Protected accessibility is invalid for non-nested classes, but nested classes may be declared protected. Fields of nonfinal public classes should rarely be declared protected; untrusted code in another package can subclass the class, and access the member. Furthermore, protected members are part of the API of the class, and consequently require continued support. When this rule is followed, declaring fields as protected is unnecessary. “OBJ01-J. Declare data members as private and provide accessible wrapper methods” [Long 2012] recommends declaring fields as private.
If a class, interface, method, or field is part of a published API, such as a web service endpoint, it may be declared public. Other classes and members should be declared either package-private or private. For example, non-security-critical classes are encouraged to provide public static factories to implement instance control with a private constructor.
Noncompliant Code Example (Public Class)
This noncompliant code example defines a class that is internal to a system and not part of any public API. Nonetheless, this class is declared public.
public final class Point {
private final int x;
private final int y;
public Point(int x, int y) {
this.x = x;
this.y = y;
}
public void getPoint() {
System.out.println("(" + x + "," + y + ")");
}
}
Even though this example complies with “OBJ01-J. Declare data members as private and provide accessible wrapper methods” [Long 2012], untrusted code could instantiate Point and invoke the public getPoint() method to obtain the coordinates.
Compliant Solution (Final Classes with Public Methods)
This compliant solution declares the Point class as package-private in accordance with its status as not part of any public API:
final class Point {
private final int x;
private final int y;
Point(int x, int y) {
this.x = x;
this.y = y;
}
public void getPoint() {
System.out.println("(" + x + "," + y + ")");
}
}
A top-level class, such as Point, cannot be declared private. Package-private accessibility is acceptable, provided package insertion attacks are avoided. (See “ENV01-J. Place all security-sensitive code in a single JAR and sign and seal it” [Long 2012].) A package insertion attack occurs when, at runtime, any protected or package-private members of a class can be called directly by a class that is maliciously inserted into the same package. However, this attack is difficult to carry out in practice because, in addition to the requirement of infiltrating the package, the target and the untrusted class must be loaded by the same class loader. Untrusted code is typically deprived of such levels of access.
Because the class is final, the getPoint() method can be declared public. A public subclass that violates this rule cannot override the method and expose it to untrusted code, so its accessibility is irrelevant. For nonfinal classes, reducing the accessibility of methods to private or package-private eliminates this threat.
Compliant Solution (Nonfinal Classes with Nonpublic Methods)
This compliant solution declares the Point class and its getPoint() method as package-private, which allows the Point class to be nonfinal and allows getPoint() to be invoked by classes present within the same package and loaded by a common class loader:
class Point {
private final int x;
private final int y;
Point(int x, int y) {
this.x = x;
this.y = y;
}
void getPoint() {
System.out.println("(" + x + "," + y + ")");
}
}
Noncompliant Code Example (Public Class with Public Static Method)
This noncompliant code example again defines a class that is internal to a system and not part of any public API. Nonetheless, this class is declared public.
public final class Point {
private static final int x = 1;
private static final int y = 2;
private Point(int x, int y) {}
public static void getPoint() {
System.out.println("(" + x + "," + y + ")");
}
}
This example also complies with “OBJ01-J. Declare data members as private and provide accessible wrapper methods” [Long 2012], untrusted code could access Point and invoke the public static getPoint() to obtain the default coordinates. The attempt to implement instance control using a private constructor is futile because the public static method exposes internal class contents.
Compliant Solution (Package-Private Class)
This compliant solution reduces the accessibility of the class to package-private.
final class Point {
private static final int x = 1;
private static final int y = 2;
private Point(int x, int y) {}
public static void getPoint() {
System.out.println("(" + x + "," + y + ")");
}
}
Access to the getPoint() method is restricted to classes located within the same package. Untrusted code is prevented from invoking getPoint() and obtaining the coordinates.
Applicability
Granting excessive access breaks encapsulation and weakens the security of Java applications.
A system with an API designed for use (and possibly extended) by third-party code must expose the API through a public interface. The demands of such an API override this guideline.
For any given piece of code, the minimum accessibility for each class and member can be computed so as to avoid introducing compilation errors. A limitation is that the result of this computation may lack any resemblance to what the programmer intended when the code was written. For example, unused members can obviously be declared private. However, such members could be unused only because the particular body of code examined coincidentally lacks references to the members. Nevertheless, this computation can provide a useful starting point for a programmer who wishes to minimize the accessibility of classes and their members.
Bibliography
[Bloch 2008]
Item 13, “Minimize the Accessibility of Classes and Members”
Item 16, “Prefer Interfaces to Abstract Classes”
[Campione 1996]
Access Control
[JLS 2013]
§6.6, “Access Control”
[Long 2012]
ENV01-J. Place all security-sensitive code in a single JAR and sign and seal it
OBJ01-J. Declare data members as private and provide accessible wrapper methods
OBJ04-J. Provide mutable classes with copy functionality to safely allow passing instances to untrusted code
[McGraw 1999]
Chapter 3, “Java Language Security Constructs”
25. Document thread-safety and use annotations where applicable
The Java language annotation facility is useful for documenting design intent. Source code annotation is a mechanism for associating metadata with a program element and making it available to the compiler, analyzers, debuggers, or Java Virtual Machine (JVM) for examination. Several annotations are available for documenting thread-safety or the lack thereof.
Obtaining Concurrency Annotations
Two sets of concurrency annotations are freely available and licensed for use in any code. The first set consists of four annotations described in Java Concurrency in Practice (JCIP) [Goetz 2006], which can be downloaded from http://jcip.net. The JCIP annotations are released under the Creative Commons Attribution License.
The second, larger set of concurrency annotations is available from and supported by SureLogic. These annotations are released under the Apache Software License, Version 2.0, and can be downloaded at www.surelogic.com. The annotations can be verified by the SureLogic JSure tool, and they remain useful for documenting code even when the tool is unavailable. These annotations include the JCIP annotations because they are supported by the JSure tool. (JSure also supports use of the JCIP JAR file.)
To use the annotations, download and add one or both of the aforementioned JAR files to the code’s build path. The use of these annotations to document thread-safety is described in the following sections.
Documenting Intended Thread-Safety
JCIP provides three class-level annotations to describe the programmer’s design intent with respect to thread-safety.
The @ThreadSafe annotation is applied to a class to indicate that it is thread-safe. This means that no sequences of accesses (reads and writes to public fields, calls to public methods) can leave the object in an inconsistent state, regardless of the interleaving of these accesses by the runtime or any external synchronization or coordination on the part of the caller.
For example, the following Aircraft class specifies that it is thread-safe as part of its locking policy documentation. This class protects the x and y fields using a reentrant lock.
@ThreadSafe
@Region("private AircraftState")
@RegionLock("StateLock is stateLock protects AircraftState")
public final class Aircraft {
private final Lock stateLock = new ReentrantLock();
// ...
@InRegion("AircraftState")
private long x, y;
// ...
public void setPosition(long x, long y) {
stateLock.lock();
try {
this.x = x;
this.y = y;
} finally {
stateLock.unlock();
}
}
// ...
}
The @Region and @RegionLock annotations document the locking policy upon which the promise of thread-safety is predicated.
Even when one or more @RegionLock or @GuardedBy annotations have been used to document the locking policy of a class, the @ThreadSafe annotation provides an intuitive way for reviewers to learn that the class is thread-safe.
The @Immutable annotation is applied to immutable classes. Immutable objects are inherently thread-safe; once they are fully constructed, they may be published via a reference and shared safely among multiple threads.
The following example shows an immutable Point class:
@Immutable
public final class Point {
private final int f_x;
private final int f_y;
public Point(int x, int y) {
f_x = x;
f_y = y;
}
public int getX() {
return f_x;
}
public int getY() {
return f_y;
}
}
According to Joshua Bloch [Bloch 2008],
It is not necessary to document the immutability of enum types. Unless it is obvious from the return type, static factories must document the thread safety of the returned object, as demonstrated by Collections.synchronizedMap.
The @NotThreadSafe annotation is applied to classes that are not thread-safe. Many classes fail to document whether they are safe for multithreaded use. Consequently, a programmer has no easy way to determine whether the class is thread-safe. This annotation provides clear indication of the class’s lack of thread-safety.
For example, most of the collection implementations provided in java.util are not thread-safe. The class java.util.ArrayList could document this as follows:
package java.util.ArrayList;
@NotThreadSafe
public class ArrayList<E> extends ... {
// ...
}
Documenting Locking Policies
It is important to document all the locks that are being used to protect shared state. According to Brian Goetz and colleagues [Goetz 2006],
For each mutable state variable that may be accessed by more than one thread, all accesses to that variable must be performed with the same lock held. In this case, we say that the variable is guarded by that lock. (p. 28)
JCIP provides the @GuardedBy annotation for this purpose, and SureLogic provides the @RegionLock annotation. The field or method to which the @GuardedBy annotation is applied can be accessed only when holding a particular lock. It may be an intrinsic lock or a dynamic lock such asjava.util.concurrent.Lock.
For example, the following MovablePoint class implements a movable point that can remember its past locations using the memo array list:
@ThreadSafe
public final class MovablePoint {
@GuardedBy("this")
double xPos = 1.0;
@GuardedBy("this")
double yPos = 1.0;
@GuardedBy("itself")
static final List<MovablePoint> memo
= new ArrayList<MovablePoint>();
public void move(double slope, double distance) {
synchronized (this) {
rememberPoint(this);
xPos += (1 / slope) * distance;
yPos += slope * distance;
}
}
public static void rememberPoint(MovablePoint value) {
synchronized (memo) {
memo.add(value);
}
}
}
The @GuardedBy annotations on the xPos and yPos fields indicate that access to these fields is protected by holding a lock on this. The move() method also synchronizes on this, which modifies these fields. The @GuardedBy annotation on the memo list indicates that a lock on the ArrayListobject protects its contents. The rememberPoint() method also synchronizes on the memo list.
One issue with the @GuardedBy annotation is that it fails to indicate when there is a relationship between the fields of a class. This limitation can be overcome by using the SureLogic @RegionLock annotation, which declares a new region lock for the class to which this annotation is applied. This declaration creates a new named lock that associates a particular lock object with a region of the class. The region may be accessed only when the lock is held. For example, the SimpleLock locking policy indicates that synchronizing on the instance protects all of its state:
@RegionLock("SimpleLock is this protects Instance")
class Simple { ... }
Unlike @GuardedBy, the @RegionLock annotation allows the programmer to give an explicit, and hopefully meaningful, name to the locking policy.
In addition to naming the locking policy, the @Region annotation allows a name to be given to the region of the state that is being protected. That name makes it clear that the state and locking policy belong together, as demonstrated in the following example:
@Region("private AircraftPosition")
@RegionLock("StateLock is stateLock protects AircraftPosition")
public final class Aircraft {
private final Lock stateLock = new ReentrantLock();
@InRegion("AircraftPosition")
private long x, y;
@InRegion("AircraftPosition")
private long altitude;
// ...
public void setPosition(long x, long y) {
stateLock.lock();
try {
this.x = x;
this.y = y;
} finally {
stateLock.unlock();
}
}
// ...
}
In this example, a locking policy named StateLock is used to indicate that locking on stateLock protects the named AircraftPosition region, which includes the mutable state used to represent the position of the aircraft.
Construction of Mutable Objects
Typically, object construction is considered an exception to the locking policy because objects are thread-confined when they are first created. An object is confined to the thread that uses the new operator to create its instance. After creation, the object can be published to other threads safely. However, the object is not shared until the thread that created the instance allows it to be shared. Safe publication approaches discussed in The CERT® Oracle® Secure Coding Standard for Java™ [Long 2012], “TSM01-J. Do not let the this reference escape during object construction,” can be expressed succinctly with the @Unique("return") annotation.
For example, in the following code, the @Unique("return") annotation documents that the object returned from the constructor is a unique reference:
@RegionLock("Lock is this protects Instance")
public final class Example {
private int x = 1;
private int y;
@Unique("return")
public Example(int y) {
this.y = y;
}
// ...
}
Documenting Thread-Confinement Policies
Dean Sutherland and William Scherlis propose annotations that can document thread-confinement policies. Their approach allows verification of the annotations against as-written code [Sutherland 2010].
For example, the following annotations express the design intent that a program has, at most, one Abstract Window Toolkit (AWT) event dispatch thread and several compute threads, and that the compute threads are forbidden to handle AWT data structures or events:
@ThreadRole AWT, Compute
@IncompatibleThreadRoles AWT, Compute
@MaxRoleCount AWT 1
Documenting Wait–Notify Protocols
According to Goetz and colleagues [Goetz 2006],
A state-dependent class should either fully expose (and document) its waiting and notification protocols to subclasses, or prevent subclasses from participating in them at all. (This is an extension of “design and document for inheritance, or else prohibit it” [EJ Item 15].) At the very least, designing a state-dependent class for inheritance requires exposing the condition queues and locks and documenting the condition predicates and synchronization policy; it may also require exposing the underlying state variables. (The worst thing a state-dependent class can do is expose its state to subclasses but not document its protocols for waiting and notification; this is like a class exposing its state variables but not documenting its invariants.) (p. 395)
Wait–notify protocols should be documented adequately. Currently, we are not aware of any annotations for this purpose.
Applicability
Annotating concurrent code helps document the design intent and can be used to automate the detection and prevention of race conditions and data races.
Bibliography
[Bloch 2008]
Item 70, “Document Thread Safety”
[Goetz 2006]
Java Concurrency in Practice
[Long 2012]
TSM01-J. Do not let the this reference escape during object construction
[Sutherland 2010]
“Composable Thread Coloring”
26. Always provide feedback about the resulting value of a method
Methods should be designed to return a value that allows the developer to learn about the current state of the object and/or the result of an operation. This advice is consistent with The CERT® Oracle® Secure Coding Standard for Java™ [Long 2012], “EXP00-J. Do not ignore values returned by methods.” The returned value should be representative of the last known state and should be chosen keeping in mind the perceptions and mental model of the developer.
Feedback can also be provided by throwing either standard or custom exception objects derived from the Exception class. With this approach, the developer can still get precise information about the outcome of the method and proceed to take the necessary actions. To do so, the exception should provide a detailed account of the abnormal condition at the appropriate abstraction level.
APIs should use a combination of these approaches, both to help clients distinguish correct results from incorrect ones and to encourage careful handling of any incorrect results. In cases where there is a commonly accepted error value that cannot be misinterpreted as a valid return value for the method, that error value should be returned; and in other cases an exception should be thrown. A method must not return a value that can hold both valid return data and an error code; see Guideline 52, “Avoid in-band error indicators,” for more details.
Alternatively, an object can provide a state-testing method [Bloch 2008] that checks whether the object is in a consistent state. This approach is useful only in cases where the object’s state cannot be modified by external threads. This prevents a time-of-check, time-of-use (TOCTOU) race condition between invocation of the object’s state-testing method and the call to a method that depends on the object’s state. During this interval, the object’s state could change unexpectedly or even maliciously.
Method return values and/or error codes must accurately specify the object’s state at an appropriate level of abstraction. Clients must be able to rely on the value for performing critical actions.
Noncompliant Code Example
The updateNode() method in this noncompliant code example modifies a node if it can find it in a linked list and does nothing if the node is not found.
public void updateNode(int id, int newValue) {
Node current = root;
while (current != null) {
if (current.getId() == id) {
current.setValue(newValue);
break;
}
current = current.next;
}
}
This method fails to indicate whether it modified any node. Consequently, a caller cannot determine that the method succeeded or failed silently.
Compliant Solution (Boolean)
This compliant solution returns the result of the operation as true if it modified a node and false if it did not.
public boolean updateNode(int id, int newValue) {
Node current = root;
while (current != null) {
if (current.getId() == id) {
current.setValue(newValue);
return true; // Node successfully updated
}
current = current.next;
}
return false;
}
Compliant Solution (Exception)
This compliant solution returns the modified Node when one is found and throws a NodeNotFoundException when the node is not available in the list.
public Node updateNode(int id, int newValue)
throws NodeNotFoundException {
Node current = root;
while (current != null) {
if (current.getId() == id) {
current.setValue(newValue);
return current;
}
current = current.next;
}
throw new NodeNotFoundException();
}
Using exceptions to indicate failure can be a good design choice, but throwing exceptions is not always appropriate. In general, a method should throw an exception only when it is expected to succeed but an unrecoverable situation occurs or when it expects a method higher up in the call hierarchy to initiate recovery.
Compliant Solution (Null Return Value)
This compliant solution returns the updated Node so that the developer can simply check for a null return value if the operation fails.
public Node updateNode(int id, int newValue) {
Node current = root;
while (current != null) {
if (current.getId() == id) {
current.setValue(newValue);
return current;
}
current = current.next;
}
return null;
}
A return value that might be null is an in-band error indicator, which is discussed more thoroughly in Guideline 52, “Avoid in-band error indicators.” This design is permitted but is considered inferior to other designs, such as those shown in the other compliant solutions in this guideline.
Applicability
Failure to provide appropriate feedback through a combination of return values, error codes, and exceptions can lead to inconsistent object state and unexpected program behavior.
Bibliography
[Bloch 2008]
Item 59. Avoid unnecessary use of checked exceptions
[Long 2012]
EXP00-J. Do not ignore values returned by methods
[Ware 2008]
Writing Secure Java Code
27. Identify files using multiple file attributes
Many file-related security vulnerabilities result from a program accessing an unintended file object. This often happens because file names are only loosely bound to underlying file objects. File names provide no information regarding the nature of the file object itself. Furthermore, the binding of a file name to a file object is reevaluated each time the file name is used in an operation. This reevaluation can introduce a time-of-check, time-of-use (TOCTOU) race condition into an application. Objects of type java.io.File and of type java.nio.file.Path are bound to underlying file objects by the operating system only when the file is accessed.
The java.io.File constructors and the java.io.File methods renameTo() and delete() rely solely on file names for file identification. The same holds for the java.nio.file.Path.get() methods for creating Path objects and the move() and delete() methods of java.nio.file.Files. Use all of these methods with caution.
Fortunately, files can often be identified by other attributes in addition to the file name—for example, by comparing file creation times or modification times. Information about a file that has been created and closed can be stored and then used to validate the identity of the file if it must be reopened. Comparing multiple attributes of the file increases the likelihood that the reopened file is the same file that was previously opened.
File identification is less crucial for applications that maintain their files in secure directories where they can be accessed only by the owner of the file and (possibly) by a system administrator (see The CERT® Oracle® Secure Coding Standard for Java™ [Long 2012], “FIO00-J. Do not operate on files in shared directories”).
Noncompliant Code Example
In this noncompliant code example, the file identified by the string filename is opened, processed, closed, and then reopened for reading:
public void processFile(String filename){
// Identify a file by its path
Path file1 = Paths.get(filename);
// Open the file for writing
try (BufferedWriter bw = new BufferedWriter(new
OutputStreamWriter(Files.newOutputStream(file1)))) {
// Write to file...
} catch (IOException e) {
// Handle error
}
// Close the file
/*
* A race condition here allows an attacker to switch
* out the file for another
*/
// Reopen the file for reading
Path file2 = Paths.get(filename);
try (BufferedReader br = new BufferedReader(new
InputStreamReader(Files.newInputStream(file2)))) {
String line;
while ((line = br.readLine()) != null) {
System.out.println(line);
}
} catch (IOException e) {
// Handle error
}
}
Because the binding between the file name and the underlying file object is reevaluated when the BufferedReader is created, this code cannot guarantee that the file opened for reading is the same file that was previously opened for writing. An attacker might have replaced the original file (with a symbolic link, for example) between the first call to close() and the subsequent creation of the BufferedReader.
Noncompliant Code Example (Files.isSameFile())
In this noncompliant code example, the programmer attempts to ensure that the file opened for reading is the same as the file previously opened for writing by calling the method Files.isSameFile():
public void processFile(String filename){
// Identify a file by its path
Path file1 = Paths.get(filename);
// Open the file for writing
try (BufferedWriter bw = new BufferedWriter(new
OutputStreamWriter(Files.newOutputStream(file1)))) {
// Write to file
} catch (IOException e) {
// Handle error
}
// ...
// Reopen the file for reading
Path file2 = Paths.get(filename);
if (!Files.isSameFile(file1, file2)) {
// File was tampered with, handle error
}
try (BufferedReader br = new BufferedReader(new
InputStreamReader(Files.newInputStream(file2)))) {
String line;
while ((line = br.readLine()) != null) {
System.out.println(line);
}
} catch (IOException e) {
// Handle error
}
}
Unfortunately, the Java API lacks any guarantee that the isSameFile() method actually checks whether the files are the same file. The Java 7 API for isSameFile() [API 2013] says:
If both Path objects are equal then this method returns true without checking if the file exists.
That is, isSameFile() may simply check that the paths to the two files are the same and cannot detect if the file at that path had been replaced by a different file between the two open operations.
Compliant Solution (Multiple Attributes)
This compliant solution checks the creation and last-modified times of the files to increase the likelihood that the file opened for reading is the same file that was written:
public void processFile(String filename) throws IOException{
// Identify a file by its path
Path file1 = Paths.get(filename);
BasicFileAttributes attr1 =
Files.readAttributes(file1, BasicFileAttributes.class);
FileTime creation1 = attr1.creationTime();
FileTime modified1 = attr1.lastModifiedTime();
// Open the file for writing
try (BufferedWriter bw = new BufferedWriter(new
OutputStreamWriter(Files.newOutputStream(file1)))) {
// Write to file...
} catch (IOException e) {
// Handle error
}
// Reopen the file for reading
Path file2 = Paths.get(filename);
BasicFileAttributes attr2 =
Files.readAttributes(file2, BasicFileAttributes.class);
FileTime creation2 = attr2.creationTime();
FileTime modified2 = attr2.lastModifiedTime();
if ( (!creation1.equals(creation2)) ||
(!modified1.equals(modified2)) ) {
// File was tampered with, handle error
}
try (BufferedReader br = new BufferedReader(new
InputStreamReader(Files.newInputStream(file2)))){
String line;
while ((line = br.readLine()) != null) {
System.out.println(line);
}
} catch (IOException e) {
// Handle error
}
}
Although this solution is reasonably secure, a determined attacker could create a symbolic link with the same creation and last-modified times as the original file. Also, a TOCTOU race condition occurs between the time the file’s attributes are first read and the time the file is first opened. Likewise, another TOCTOU condition occurs the second time the attributes are read and the file is reopened.
Compliant Solution (POSIX fileKey Attribute)
In environments that support the fileKey attribute, a more reliable approach is to check that the fileKey attributes of the two files are the same. The fileKey attribute is an object that “uniquely identifies the file” [API 2013], as shown in this compliant solution:
public void processFile(String filename) throws IOException{
// Identify a file by its path
Path file1 = Paths.get(filename);
BasicFileAttributes attr1 =
Files.readAttributes(file1, BasicFileAttributes.class);
Object key1 = attr1.fileKey();
// Open the file for writing
try (BufferedWriter bw =
new BufferedWriter(
new OutputStreamWriter(Files.newOutputStream(file1)))) {
// Write to file
} catch (IOException e) {
// Handle error
}
// Reopen the file for reading
Path file2 = Paths.get(filename);
BasicFileAttributes attr2 =
Files.readAttributes(file2, BasicFileAttributes.class);
Object key2 = attr2.fileKey();
if ( !key1.equals(key2) ) {
System.out.println("File tampered with");
// File was tampered with, handle error
}
try (BufferedReader br =
new BufferedReader(
new InputStreamReader(Files.newInputStream(file2)))) {
String line;
while ((line = br.readLine()) != null) {
System.out.println(line);
}
} catch (IOException e) {
// Handle error
}
}
This approach will not work on all platforms. For example, on Windows 7 Enterprise Edition, all fileKey attributes are null.
The file key returned by the fileKey() method is guaranteed to be unique only if the file system and files remain static. A file system may reuse an identifier, for example, after a file is deleted. Like the previous compliant solution, there is a TOCTOU race window between the time the file’s attributes are first read and the time the file is first opened. Another TOCTOU condition occurs the second time the attributes are read and the file is reopened.
Compliant Solution (RandomAccessFile)
A better approach is to avoid reopening files. The following compliant solution demonstrates use of a RandomAccessFile, which can be opened for both reading and writing. Because the file is only closed automatically by the try-with-resources statement, no race condition can occur. Note that this compliant solution and others use the readLine() method for illustrative purposes, but see “MSC05-J. Do not exhaust heap space” [Long 2012] for more information about possible weaknesses of this method.
public void processFile(String filename) throws IOException{
// Identify a file by its path
try ( RandomAccessFile file = new
RandomAccessFile(filename, "rw")) {
// Write to file...
// Go back to beginning and read contents
file.seek(0);
String line;
while ((line = file.readLine()) != null) {
System.out.println(line);
}
}
}
Noncompliant Code Example (File Size)
This noncompliant code example tries to ensure that the file it opens contains exactly 1024 bytes:
static long goodSize = 1024;
public void doSomethingWithFile(String filename) {
long size = new File(filename).length();
if (size != goodSize) {
System.out.println("File has wrong size!");
return;
}
try (BufferedReader br = new BufferedReader(new
InputStreamReader(new FileInputStream(filename)))) {
// ... Work with file
} catch (IOException e) {
// Handle error
}
}
This code is subject to a TOCTOU race condition between when the file size is checked and when the file is opened. If an attacker replaces a 1024-byte file with another file during this race window, they can cause this program to open any file, defeating the check.
Compliant Solution (File Size)
This compliant solution uses the FileChannel.size() method to obtain the file size. Because this method is applied to the FileInputStream only after the file has been opened, this solution eliminates the race window.
static long goodSize = 1024;
public void doSomethingWithFile(String filename) {
try (FileInputStream in = new FileInputStream(filename);
BufferedReader br = new BufferedReader(
new InputStreamReader(in))) {
long size = in.getChannel().size();
if (size != goodSize) {
System.out.println("File has wrong size!");
return;
}
String line;
while ((line = br.readLine()) != null) {
System.out.println(line);
}
} catch (IOException e) {
// Handle error
}
}
Applicability
Attackers frequently exploit file-related vulnerabilities to cause programs to access an unintended file. Proper file identification is necessary to prevent exploitation.
Bibliography
[API 2013]
Class java.io.File
Interface java.nio.file.Path
Class java.nio.file.Files
Interface java.nio.file.attribute.BasicFileAttributes
[Long 2012]
FIO00-J. Do not operate on files in shared directories
28. Do not attach significance to the ordinal associated with an enum
Java language enumeration types have an ordinal() method that returns the numerical position of each enumeration constant in its class declaration.
According to the Java API, Class Enum<E extends Enum<E>> [API 2013], public final int ordinal()
returns the ordinal of the enumeration constant (its position in its enum declaration, where the initial constant is assigned an ordinal of zero). Most programmers will have no use for this method. It is designed for use by sophisticated enum-based data structures, such as EnumSet andEnumMap.
The Java Language Specification (JLS), §8.9, “Enums” [JLS 2013], does not specify the use of ordinal() in programs. However, attaching external significance to the ordinal() value of an enum constant is error prone and should be avoided for defensive programming.
Noncompliant Code Example
This noncompliant code example declares enum Hydrocarbon and uses its ordinal() method to provide the result of the getNumberOfCarbons() method:
enum Hydrocarbon {
METHANE, ETHANE, PROPANE, BUTANE, PENTANE,
HEXANE, HEPTANE, OCTANE, NONANE, DECANE;
public int getNumberOfCarbons() {
return ordinal() + 1;
}
}
Although this noncompliant code example behaves as expected, its maintenance is likely to be problematic. If the enum constants were reordered, the getNumberOfCarbons() method would return incorrect values. Furthermore, adding an additional BENZENE constant to the model would break the invariant assumed by the getNumberOfCarbons() method, because benzene has six carbons, but the ordinal value six is already taken by hexane.
Compliant Solution
In this compliant solution, enum constants are explicitly associated with the corresponding integer values for the number of carbon atoms they contain:
enum Hydrocarbon {
METHANE(1), ETHANE(2), PROPANE(3), BUTANE(4), PENTANE(5),
HEXANE(6), BENZENE(6), HEPTANE(7), OCTANE(8), NONANE(9),
DECANE(10);
private final int numberOfCarbons;
Hydrocarbon(int carbons) { this.numberOfCarbons = carbons; }
public int getNumberOfCarbons() {
return numberOfCarbons;
}
}
The getNumberOfCarbons() method no longer uses the ordinal() to discover the number of carbon atoms for each value. Different enum constants may be associated with the same value, as shown for HEXANE and BENZENE. Furthermore, this solution lacks any dependence on the order of the enumeration; the getNumberOfCarbons() method would continue to work correctly even if the enumeration were reordered.
Applicability
It is acceptable to use the ordinals associated with an enumerated type when the order of the enumeration constants is standard and extra constants cannot be added. For example, the use of ordinals is permitted with the following enumerated type:
public enum Day { SUNDAY, MONDAY, TUESDAY, WEDNESDAY,
THURSDAY, FRIDAY, SATURDAY }
In general, the use of ordinals to derive integer values reduces the program’s maintainability and can lead to errors in the program.
Bibliography
[API 2013]
Class Enum<E extends Enum<E>>
[Bloch 2008]
Item 31, “Use Instance Fields Instead of Ordinals”
[JLS 2013]
§8.9, “Enums”
29. Be aware of numeric promotion behavior
Numeric promotions are used to convert the operands of a numeric operator to a common type so that an operation can be performed. When using arithmetic operators with mixed operand sizes, narrower operands are promoted to the type of the wider operand.
Promotion Rules
The JLS, §5.6, “Numeric Promotions” [JLS 2013], describes numeric promotion as the following:
1. If any of the operands is of a reference type, unboxing conversion is performed.
2. If either operand is of type double, the other is converted to double.
3. Otherwise, if either operand is of type float, the other is converted to float.
4. Otherwise, if either operand is of type long, the other is converted to long.
5. Otherwise, both operands are converted to type int.
Widening conversions, resulting from integer promotions, preserve the overall magnitude of the number. However, promotions in which the operands are converted from an int to a float or from a long to a double can cause a loss of precision. (See The CERT® Oracle® Secure Coding Standard for Java™ [Long 2012], “NUM13-J. Avoid loss of precision when converting primitive integers to floating-point,” for more details.)
These conversions can occur when using multiplicative operators (%, *, /), additive operators (+, -), comparison operators (<, >, <=, >=), equality operators (==, !=), and integer bitwise operators (&, |, ^).
Examples
In the following example, a is promoted to a double before the + operator is applied:
int a = some_value;
double b = some_other_value;
double c = a + b;
In the following program fragment, b is first converted to int so that the + operator can be applied to operands of the same type:
int a = some_value;
char b = some_character;
if ((a + b) > 1.1f) {
// Do something
}
The result of (a+b) is then converted to a float, and the comparison operator is finally applied.
Compound Operators
Type coercion may occur when compound expressions are used with mixed operand types. Examples of compound assignment operators are +=, -=, *=, /=, &=, ^=, %=, <<=, >>=, >>>=, and |=.
According to the JLS §15.26.2, “Compound Assignment Operators” [JLS 2013],
A compound assignment expression of the form E1 op= E2 is equivalent to E1 = (T)((E1) op (E2)), where T is the type of E1, except that E1 is evaluated only once.
That is, the compound assignment expression implicitly casts the resulting computation to the type of the left-hand operand.
When the operands are different types, multiple conversions can occur. For example, when E1 is an int, and E2 is either a long, a float, or a double, then E1 is widened from type int to the type of E2 (before the “op”), followed by a narrowing conversion from the type of E2 back to type int(after the “op,” but before the assignment).
Noncompliant Code Example (Multiplication)
In this noncompliant code example, a variable of type int (big) is multiplied by a value of type float (one).
int big = 1999999999;
float one = 1.0f;
// Binary operation, loses precision because of implicit cast
System.out.println(big * one);
In this case, numeric promotions require that big be promoted to the type float before the multiplication occurs, resulting in loss of precision. (See “NUM13-J. Avoid loss of precision when converting primitive integers to floating-point” [Long 2012].) This code outputs 2.0E9 rather than 1.999999999E9.
Compliant Solution (Multiplication)
This compliant solution uses the double type, instead of float, as a safer means of handling the widening primitive conversion resulting from integer promotion:
int big = 1999999999;
double one = 1.0d; // Double instead of float
System.out.println(big * one);
This solution produces the expected output of 1.999999999E9, which is the value obtained when an int is assigned (implicitly cast) to a double.
See also Guideline 60, “Convert integers to floating-point for floating-point operations,” for more information about mixing integer and floating-point arithmetic.
Noncompliant Code Example (Left Shift)
This noncompliant code example shows integer promotion resulting from the use of the bitwise OR operator.
byte[] b = new byte[4];
int result = 0;
for (int i = 0; i < 4; i++) {
result = (result << 8) | b[i];
}
Each byte array element is sign-extended to 32 bits before it is used as an operand. If it originally contained the value 0xff, it would contain 0xffffffff [FindBugs 2008]. This causes result to contain a value other than the concatenation of the four array elements.
Compliant Solution (Left Shift)
This compliant solution masks off the upper 24 bits of the byte array element to achieve the intended result:
byte[] b = new byte[4];
int result = 0;
for (int i = 0; i < 4; i++) {
result = (result << 8) | (b[i] & 0xff);
}
Noncompliant Code Example (Compound Addition and Assignment)
This noncompliant code example performs a compound assignment operation.
int x = 2147483642; // 0x7ffffffa
x += 1.0f; // x contains 2147483647 (0x7fffffff)
// after the computation
The compound operation involves an int value that contains too many significant bits to fit in the 23-bit mantissa of a Java float, causing the widening conversion from int to float to lose precision. The resulting value is frequently unexpected.
Compliant Solution (Compound Addition and Assignment)
For defensive programming purposes, avoid using any of the compound assignment operators on variables of type byte, short, or char. Also, refrain from using a wider operand on the right-hand side. In this compliant solution, all operands are of the Java type double.
double x = 2147483642; // 0x7ffffffa
x += 1.0; // x contains 2147483643.0 (0x7ffffffb.0) as expected
Noncompliant Code Example (Compound Bit Shift and Assignment)
This noncompliant code example uses a compound right-shift operator for shifting the value of i by one bit.
short i = -1;
i >>>= 1;
Unfortunately, the value of i remains the same. The value of i is first promoted to an int. This is a widening primitive conversion, so no data is lost. As a short, -1 is represented as 0xffff. The conversion to int results in the value 0xffffffff, which is right-shifted by 1 bit to yield0x7fffffff. To store the value back into the short variable i, Java performs an implicit narrowing conversion, discarding the 16 higher-order bits. The final result is again 0xffff, or -1.
Compliant Solution (Compound Bit Shift and Assignment)
This compliant solution applies the compound assignment operator to an int, which does not require widening and subsequent narrowing. Consequently, i gets the value 0x7fffffff.
int i = -1;
i >>>= 1;
Applicability
Failing to consider integer promotions when dealing with floating-point and integer operands can result in loss of precision.
Bibliography
[Bloch 2005]
Puzzle 9, “Tweedledum”
Puzzle 31, “Ghost of Looper”
[Findbugs 2008]
“BIT: Bitwise OR of Signed Byte Value”
[JLS 2013]
§4.2.2, “Integer Operations”
§5.6, “Numeric Promotions”
§15.26.2, “Compound Assignment Operators”
[Long 2012]
NUM13-J. Avoid loss of precision when converting primitive integers to floating-point
30. Enable compile-time type checking of variable arity parameter types
A variable arity (aka varargs) method is a method that can take a variable number of arguments. The method must contain at least one fixed argument. When processing a variable arity method call, the Java compiler checks the types of all arguments, and all of the variable actual arguments must match the variable formal argument type. However, compile-time type checking is ineffective when Object or generic parameter types are used [Bloch 2008]. The presence of initial parameters of specific types is irrelevant; the compiler will remain unable to check Object or generic variable parameter types. Enable strong compile-time type checking of variable arity methods by using the most specific type possible for the method parameter.
Noncompliant Code Example (Object)
This noncompliant code example sums a set of numbers using a variable arity method that uses Object as the variable arity type. Consequently, this method accepts an arbitrary mix of parameters of any object type. Legitimate uses of such declarations are rare (but see the “Applicability” section of this guideline).
double sum(Object... args) {
double result = 0.0;
for (Object arg : args) {
if (arg instanceof Byte) {
result += ((Byte) arg).byteValue();
} else if (arg instanceof Short) {
result += ((Short) arg).shortValue();
} else if (arg instanceof Integer) {
result += ((Integer) arg).intValue();
} else if (arg instanceof Long) {
result += ((Long) arg).longValue();
} else if (arg instanceof Float) {
result += ((Float) arg).floatValue();
} else if (arg instanceof Double) {
result += ((Double) arg).doubleValue();
} else {
throw new ClassCastException();
}
}
return result;
}
Compliant Solution (Number)
This compliant solution defines the same method, but uses the Number type. This abstract class is general enough to encompass all numeric types, yet specific enough to exclude nonnumeric types.
double sum(Number... args) {
// ...
}
Noncompliant Code Example (Generic Type)
This noncompliant code example declares the same variable arity method using a generic type parameter. It accepts a variable number of parameters that are all of the same object type; however, it may be any object type. Again, legitimate uses of such declarations are rare.
<T> double sum(T... args) {
// ...
}
Compliant Solution (Generic Type)
This compliant solution defines the same generic method using the Number type.
<T extends Number> double sum(T... args) {
// ...
}
Be as specific as possible when declaring parameter types; avoid Object and imprecise generic types in variable arity methods. Retrofitting old methods containing final array parameters with generically typed variable arity parameters is not always a good idea. For example, given a method that does not accept an argument of a particular type, it could be possible to override the compile-time checking—through the use of generic variable arity parameters—so that the method would compile cleanly rather than correctly, causing a runtime error [Bloch 2008].
Also, note that autoboxing prevents strong compile-time type checking of primitive types and their corresponding wrapper classes. For example, this compliant solution produces the following warning, but works as expected:
Java.java:10: warning: [unchecked] Possible heap pollution from
parameterized vararg type T
<T extends Number> double sum(T... args) {
This particular compiler warning can be safely ignored.
Applicability
Injudicious use of variable arity parameter types prevents strong compile-time type checking, creates ambiguity, and diminishes code readability.
Variable arity signatures using Object and imprecise generic types are acceptable when the body of the method lacks both casts and autoboxing, and it also compiles without error. Consider the following example, which operates correctly for all object types and type-checks successfully:
<T> Collection<T> assembleCollection(T... args) {
return new HashSet<T>(Arrays.asList(args));
}
In some circumstances, it is necessary to use a variable arity parameter of type Object. A good example is the method java.util.Formatter.format(String format, Object... args), which can format objects of any type.
Automated detection is straightforward.
Bibliography
[Bloch 2008]
Item 42, “Use Varargs Judiciously”
[Steinberg 2008]
Using the Varargs Language Feature
[Oracle 2011b]
Varargs
31. Do not apply public final to constants whose value might change in later releases
The final keyword can be used to specify constant values (that is, values that cannot change during program execution). However, constants that can change over the lifetime of a program should not be declared public final. The JLS [JLS 2013] allows implementations to insert the value of any public final field inline in any compilation unit that reads the field. Consequently, if the declaring class is edited so that the new version gives a different value for the field, compilation units that read the public final field could still see the old value until they are recompiled. This problem may occur, for example, when a third-party library is updated to the latest version, but the referencing code is not recompiled.
A related error can arise when a programmer declares a static final reference to a mutable object; see Guideline 73, “Never confuse the immutability of a reference with that of the referenced object,” for additional information.
Noncompliant Code Example
In this noncompliant code example, class Foo in Foo.java declares a field whose value represents the version of the software:
class Foo {
public static final int VERSION = 1;
// ...
}
The field is subsequently accessed by class Bar from a separate compilation unit (Bar.java):
class Bar {
public static void main(String[] args) {
System.out.println("You are using version " + Foo.VERSION);
}
}
When compiled and run, the software correctly prints
You are using version 1
But if a developer were to change the value of VERSION to 2 by modifying Foo.java and subsequently recompile Foo.java, while failing to recompile Bar.java, the software would incorrectly print
You are using version 1
Although recompiling Bar.java solves this problem, a better solution is available.
Compliant Solution
According to §13.4.9, “final Fields and Constants,” of the JLS [JLS 2013],
Other than for true mathematical constants, we recommend that source code make very sparing use of class variables that are declared static and final. If the read-only nature of final is required, a better choice is to declare a private static variable and a suitable accessor method to get its value.
In this compliant solution, the version field in Foo.java is declared private static and accessed by the getVersion() method:
class Foo {
private static int version = 1;
public static final int getVersion() {
return version;
}
// ...
}
The Bar class in Bar.java is modified to invoke the getVersion() accessor method to retrieve the version field from Foo.java:
class Bar {
public static void main(String[] args) {
System.out.println(
"You are using version " + Foo.getVersion()
);
}
}
In this solution, the private version value cannot be copied into the Bar class when it is compiled, consequently preventing the bug. Note that this transformation imposes little or no performance penalty because most just-in-time (JIT) code generators can inline the getVersion() method at runtime.
Applicability
Declaring a value that changes over the lifetime of the software as final may lead to unexpected results.
According to §9.3, “Field (Constant) Declarations,” of the JLS [JLS 2013], “Every field declaration in the body of an interface is implicitly public, static, and final. It is permitted to redundantly specify any or all of these modifiers for such fields.” Therefore, this guideline does not apply to fields defined in interfaces. Clearly, if the value of a field in an interface changes, every class that implements or uses the interface must be recompiled. See Guideline 35, “Carefully design interfaces before releasing them,” for more information.
Constants declared using the enum type are permitted to violate this guideline.
Constants whose value never changes throughout the entire lifetime of the software may be declared as final. For example, the JLS recommends that mathematical constants be declared final.
Bibliography
[JLS 2013]
§4.12.4, “final Variables”
§8.3.1.1, “static Fields”
§9.3, “Field (Constant) Declarations”
§13.4.9, “final Fields and Constants”
32. Avoid cyclic dependencies between packages
Both The Elements of Java™ Style [Allen 2000] and the JPL Java Coding Standard [Havelund 2009] require that the dependency structure of a package must never contain cycles; that is, it must be representable as a directed acyclic graph (DAG).
Eliminating cycles between packages has several advantages.
Testing and maintainability. Cyclic dependencies magnify the repercussions of changes or patches to source code. Reducing the repercussions of changes simplifies testing and improves maintainability. Inability to perform adequate testing because of cyclic dependencies is a frequent source of security vulnerabilities.
Reusability. Cyclic dependencies between packages require that the packages be released and upgraded in lockstep. This requirement reduces reusability.
Releases and builds. Avoiding cycles also helps to steer the development toward an environment that fosters modularization.
Deployment. Avoiding cyclic dependencies between packages reduces coupling between packages. Reduced coupling reduces the frequency of runtime errors such as ClassNotFoundError. This, in turn, simplifies deployment.
Noncompliant Code Example
This noncompliant code example contains packages named account and user that consist of the classes AccountHolder, User, and UserDetails, respectively. The class UserDetails extends from AccountHolder because a user is a kind of account holder. The class AccountHolder depends on a non-static utility method defined in the User class. Likewise, UserDetails depends on AccountHolder by extending it.
package account;
import user.User;
public class AccountHolder {
private User user;
public void setUser(User newUser) {user = newUser;}
synchronized void depositFunds(String username,
double amount) {
// Use a utility method of User to check whether
// username exists
if (user.exists(username)) {
// Deposit the amount
}
}
protected double getBalance(String accountNumber) {
// Return the account balance
return 1.0;
}
}
package user;
import account.AccountHolder;
public class UserDetails extends AccountHolder {
public synchronized
double getUserBalance(String accountNumber) {
// Use a method of AccountHolder to get the account balance
return getBalance(accountNumber);
}
}
public class User {
public boolean exists(String username) {
// Check whether user exists
return true; // Exists
}
}
Compliant Solution
The tight coupling between the classes in the two packages can be weakened by introducing an interface called BankApplication in a third package, bank. The cyclic package dependency is eliminated by ensuring that the AccountHolder does not depend on User, but instead relies on the interface by importing the bank package (and not by implementing the interface).
In this compliant solution, such functionality is achieved by adding a parameter of the interface type BankApplication to the depositFunds() method. This solution gives the AccountHolder a solid contract to bank on. Additionally, UserDetails implements the interface and provides concrete implementations of the methods while at the same time inheriting the other methods from AccountHolder.
package bank;
public interface BankApplication {
void depositFunds(BankApplication ba, String username,
double amount);
double getBalance(String accountNumber);
double getUserBalance(String accountNumber);
boolean exists(String username);
}
package account;
import bank.BankApplication; // Import from a third package
class AccountHolder {
private BankApplication ba;
public void setBankApplication(BankApplication newBA) {
ba = newBA;
}
public synchronized void depositFunds(BankApplication ba,
String username, double amount) {
// Use a utility method of UserDetails to
// check whether username exists
if (ba.exists(username)) {
// Deposit the amount
}
}
public double getBalance(String accountNumber) {
// Return the account balance
return 1.0;
}
}
package user;
import account.AccountHolder; // One-way dependency
import bank.BankApplication; // Import from a third package
public class UserDetails extends AccountHolder
implements BankApplication {
public synchronized double getUserBalance(
String accountNumber) {
// Use a method of AccountHolder to get the account balance
return getBalance(accountNumber);
}
public boolean exists(String username) {
// Check whether user exists
return true;
}
}
The interface BankApplication appears to contain superfluous methods such as depositFunds() and getBalance(). These methods are present so that if the subclass overrides them, the superclass retains the capability of internally invoking the subclass’s methods polymorphically (for example, calling ba.getBalance() with an overridden implementation of the method in UserDetails). One consequence of this solution is that methods declared in the interface are required to be public in the classes that define them.
Applicability
Cyclic dependencies between packages can result in fragile builds. A security vulnerability in a package can easily percolate to other packages.
Bibliography
[Allen 2000]
The Elements of Java™ Style
[Havelund 2009]
JPL Coding Standard, Version 1.1
[Knoernschild 2002]
Chapter 1, “OO Principles and Patterns”
33. Prefer user-defined exceptions over more general exception types
Because an exception is caught by its type, it is better to define exceptions for specific purposes than to use general exception types for multiple purposes. Throwing general exception types makes code hard to understand and maintain, and defeats much of the advantage of the Java exception-handling mechanism.
Noncompliant Code Example
This noncompliant code example attempts to distinguish between different exceptional behaviors by looking at the exception’s message:
If doSomething() throws an exception or error whose type is a subclass of Throwable, the switch statement allows selection of a specific case to execute. For example, if the exception message is “file not found,” the appropriate action is taken in the exception-handling code.
try {
doSomething();
} catch (Throwable e) {
String msg = e.getMessage();
switch (msg) {
case "file not found":
// Handle error
break;
case "connection timeout":
// Handle error
break;
case "security violation":
// Handle error
break;
default: throw e;
}
}
However, any change to the exception message literals involved will break the code. For example, suppose this code is executed:
throw new Exception("cannot find file");
This exception should be handled by the first case clause, but it will instead be rethrown because the string does not match any case clause.
Furthermore, exceptions may be thrown without a message.
This noncompliant code example falls under ERR08-EX0 of The CERT® Oracle®Secure Coding Standard for Java™ [Long 2012], “ERR08-J. Do not catch NullPointerException or any of its ancestors,” because it catches general exceptions but rethrows them.
Compliant Solution
This compliant solution uses specific exception types and defines new special purpose exception types where required.
public class TimeoutException extends Exception {
TimeoutException () {
super();
}
TimeoutException (String msg) {
super(msg);
}
}
// ...
try {
doSomething();
} catch (FileNotFoundException e) {
// Handle error
} catch (TimeoutException te) {
// Handle error
} catch (SecurityException se) {
// Handle error
}
Applicability
Exceptions are used to handle exceptional conditions. If an exception is not caught, the program will be terminated. An exception that is incorrectly caught or is caught at the wrong level of recovery will often cause incorrect behavior.
Bibliography
[JLS 2013]
Chapter 11, “Exceptions”
[Long 2012]
ERR08-J. Do not catch NullPointerException or any of its ancestors
34. Try to gracefully recover from system errors
According to the JLS, §11.1.1, “The Kinds of Exceptions” [JLS 2013],
The unchecked exceptions classes are the class RuntimeException and its subclasses, and the class Error and its subclasses. All other exception classes are checked exception classes.
Unchecked exception classes are not subject to compile-time checking because it is tedious to account for all exceptional conditions and because recovery is often difficult or impossible. However, even when recovery is impossible, the Java Virtual Machine (JVM) allows a graceful exit and a chance to at least log the error. This is made possible by using a try-catch block that catches Throwable. Also, when code must avoid leaking potentially sensitive information, catching Throwable is permitted. In all other cases, catching Throwable is not recommended because it makes handling specific exceptions difficult. Where cleanup operations such as releasing system resources can be performed, code should use a finally block to release the resources or a try-with-resources statement.
Catching Throwable is disallowed in general by The CERT® Oracle® Secure Coding Standard for Java™ [Long 2012], “ERR08-J. Do not catch NullPointer-Exception or any of its ancestors,” but it is permitted when filtering exception traces by the exception ERR08-EX0 in that rule.
Noncompliant Code Example
This noncompliant code example generates a StackOverflowError as a result of infinite recursion. It exhausts the available stack space and may result in denial of service.
public class StackOverflow {
public static void main(String[] args) {
infiniteRun();
// ...
}
private static void infiniteRun() {
infiniteRun();
}
}
Compliant Solution
This compliant solution shows a try-catch block that can be used to capture java.lang.Error or java.lang.Throwable. A log entry can be made at this point, followed by attempts to free key system resources in the finally block.
public class StackOverflow {
public static void main(String[] args) {
try {
infiniteRun();
} catch (Throwable t) {
// Forward to handler
} finally {
// Free cache, release resources
}
// ...
}
private static void infiniteRun() {
infiniteRun();
}
}
Note that the Forward to handler code must operate correctly in constrained memory conditions because the stack or heap may be nearly exhausted. In such a scenario, one useful technique is for the program to initially reserve memory specifically to be used by an out-of-memory exception handler.
Note that this solution catches Throwable in an attempt to handle the error; it falls under exception ERR08-EX2 in “ERR08-J. Do not catch NullPointerException or any of its ancestors” [Long 2012].
Applicability
Allowing a system error to abruptly terminate a Java program may result in a denial-of-service vulnerability.
In the event of actually running out of memory, it is likely that some program data will be in an inconsistent state. Consequently, it might be best to restart the process. If an attempt is made to carry on, reducing the number of threads may be an effective workaround. This measure can help in such scenarios because threads often leak memory, and their continued existence can increase the memory footprint of the program.
The methods Thread.setUncaughtExceptionHandler() and ThreadGroup.uncaughtException() can be used to help deal with an OutOfMemoryError in threads.
Bibliography
[JLS 2013]
§11.2, “Compile-Time Checking of Exceptions”
[Kalinovsky 2004]
Chapter 16, “Intercepting Control Flow: Intercepting System Errors”
[Long 2012]
ERR08-J. Do not catch NullPointerException or any of its ancestors
35. Carefully design interfaces before releasing them
Interfaces are used to group all the methods that a class promises to publicly expose. The implementing classes are obliged to provide concrete implementations for all of these methods. Interfaces are a necessary ingredient of most public APIs; once released, flaws can be hard to fix without breaking any code that implements the older version. The repercussions include the following.
Interface changes resulting from fixes can severely impair the contracts of the implementing classes. For example, a fix introduced in a later version may be accompanied by modifications to an unrelated interface that must now be implemented by the client. The client may be prevented from implementing the fix because the new interface may impose additional implementation burden on it.
Implementers can provide default or skeletal implementations of interface methods for their clients to extend; however, such code can adversely affect the behavior of the subclasses. Conversely, when such default implementations are absent, the subclasses must provide dummy implementations. Such implementations foster an environment where comments such as “ignore this code, does nothing” occur incessantly. Such code may never even get tested.
If there is a security flaw in a public API (see, for example, the discussion of ThreadGroup methods in The CERT® Oracle® Secure Coding Standard for Java™ [Long 2012], “THI01-J. Do not invoke ThreadGroup methods”), it will persist throughout the lifetime of the API, affecting the security of any application or library that uses it. Even after the security flaw is mitigated, applications and libraries may continue using the insecure version until they are also updated.
Noncompliant Code Example
In this noncompliant code example, an interface User is frozen with two methods: authenticate() and subscribe(). Sometime later, the providers release a free service that does not rely on authentication.
public interface User {
boolean authenticate(String username, char[] password);
void subscribe(int noOfDays);
// Introduced after the class is publicly released
void freeService();
}
The addition of the freeService() method, unfortunately, breaks all the client code that implements the interface. Moreover, the implementers who wish to use only freeService() have to face the onus of also providing the other two methods, which pollute the API, for reasons discussed earlier.
Noncompliant Code Example
An alternative idea is to prefer abstract classes for dealing with constant evolution, but that comes at the cost of flexibility that interfaces offer (a class may implement multiple interfaces, but extend only one class). One notable pattern is for the provider to distribute an abstract skeletal class that implements the evolving interface. The skeletal class can selectively implement a few methods, and force the extending classes to provide concrete implementations of the others. If a new method is added to the interface, the skeletal class can provide a non-abstract default implementation that the extending class can optionally override. This noncompliant code example shows such a skeletal class.
public interface User {
boolean authenticate(String username, char[] password);
void subscribe(int noOfDays);
void freeService(); // Introduced after API is
// publicly released
}
abstract class SkeletalUser implements User {
public abstract boolean authenticate(String username,
char[] password);
public abstract void subscribe(int noOfDays);
public void freeService() {
// Added later, provide implementation and re-release class
}
}
class Client extends SkeletalUser {
// Implements authenticate() and subscribe(), not freeService()
}
Although useful, this pattern may be insecure because a provider who is unaware of the extending class’s code may choose an implementation that introduces security weaknesses in the client API.
Compliant Solution (Modularize)
A better design strategy is to anticipate the future evolution of the service. The core functionality should be implemented in the User interface; in this case, only the premium service may be required to extend from it. To make use of the new free service, an existing class may then choose to implement the new interface FreeUser, or it may just completely ignore it.
public interface User {
boolean authenticate(String username, char[] password);
}
public interface PremiumUser extends User {
void subscribe(int noOfDays);
}
public interface FreeUser {
void freeService();
}
Compliant Solution (Make New Method Unusable)
Another compliant solution is to throw an exception from within the new freeService() method defined in the implementing subclass.
class Client implements User {
public void freeService() {
throw new AbstractMethodError();
}
}
Compliant Solution (Delegate Implementation to Subclasses)
Although allowable, a less flexible compliant solution is to delegate the implementation of the method to subclasses of the client’s core interface-implementing class.
abstract class Client implements User {
public abstract void freeService();
// Delegate implementation of new method to subclasses
// Other concrete implementations
}
Applicability
Failing to publish stable, flaw-free interfaces can break the contracts of the implementing classes, pollute the client API, and possibly introduce security weaknesses in the implementing classes.
Bibliography
[Bloch 2008]
Item 18, “Prefer Interfaces to Abstract Classes”
[Long 2012]
THI01-J. Do not invoke ThreadGroup methods
36. Write garbage collection–friendly code
Java’s garbage-collection feature provides significant benefits over non-garbage-collected languages. The garbage collector (GC) is designed to automatically reclaim unreachable memory, and to avoid memory leaks. Although the GC is quite adept at performing this task, a malicious attacker can nevertheless launch a denial-of-service (DoS) attack against the GC, such as by inducing abnormal heap memory allocation or abnormally prolonged object retention. For example, some versions of the GC could need to halt all executing threads to keep up with incoming allocation requests that trigger increased heap management activity. System throughput rapidly diminishes in this scenario.
Real-time systems, in particular, are vulnerable to a more subtle slow-heap-exhaustion DoS attack, perpetrated by stealing CPU cycles. An attacker can perform memory allocations in a way that increases the consumption of resources (such as CPU, battery power, and memory) without triggering an OutOfMemoryError. Writing garbage collection–friendly code restricts many attack avenues.
Use Short-Lived Immutable Objects
Beginning with JDK 1.2, the generational GC has reduced memory allocation costs, in many cases to levels lower than in C or C++. Generational garbage collection reduces garbage collection costs by grouping objects into generations. The younger generation consists of short-lived objects. The GC performs a minor collection on the younger generation when it fills up with dead objects [Oracle 2010a]. Improved garbage collection algorithms have reduced the cost of garbage collection so that it is proportional to the number of live objects in the younger generation, rather than to the number of objects allocated since the last garbage collection.
Note that objects in the younger generation that persist for longer durations are tenured and are moved to the tenured generation. Few younger-generation objects continue to live through to the next garbage-collection cycle. The rest become ready to be collected in the impending collection cycle [Oracle 2010a].
With generational GCs, use of short-lived immutable objects is generally more efficient than use of long-lived mutable objects, such as object pools. Avoiding object pools improves the GC’s efficiency. Object pools bring additional costs and risks: they can create synchronization problems and can require explicit management of deallocations, possibly creating problems with dangling pointers. Further, determining the correct amount of memory to reserve for an object pool can be difficult, especially for mission-critical code. Use of long-lived mutable objects remains appropriate when allocation of objects is particularly expensive (for example, when performing multiple joins across databases). Similarly, object pools are an appropriate design choice when the objects represent scarce resources, such as thread pools and database connections.
Avoid Large Objects
The allocation of large objects is expensive, in part because the cost to initialize their fields is proportional to their size. Additionally, frequent allocation of large objects of different sizes can cause fragmentation issues or compacting collect operations.
Do Not Explicitly Invoke the Garbage Collector
The GC can be explicitly invoked by calling the System.gc() method. Even though the documentation says that it “runs the garbage collector,” there is no guarantee as to when or whether the GC will actually run. In fact, the call merely suggests that the GC should subsequently execute; the JVM is free to ignore this suggestion.
Irresponsible use of this feature can severely degrade system performance by triggering garbage collection at inopportune moments, rather than waiting until ripe periods when it is safe to garbage-collect without significant interruption of the program’s execution.
In the Java Hotspot VM (default since JDK 1.2), System.gc() forces an explicit garbage collection. Such calls can be buried deep within libraries, so they may be difficult to trace. To ignore the call in such cases, use the flag -XX:+DisableExplicitGC. To avoid long pauses while performing a full garbage collection, a less demanding concurrent cycle may be invoked by specifying the flag -XX:ExplicitGCInvokedConcurrent.
Applicability
Misusing garbage-collection utilities can cause severe performance degradation, which can be exploited to cause a DoS attack. The Apache Geronimo and Tomcat vulnerability GERONIMO-4574, reported in March 2009, resulted from PolicyContext handler data objects being set in a thread and never released, causing these data objects to remain in memory longer than necessary.
When an application goes through several phases, such as an initialization and a ready phase, it could require heap compaction between phases. The System.gc() method may be invoked in such cases, provided a suitable uneventful period occurs between phases.
Bibliography
[API 2013]
Class System
[Bloch 2008]
Item 6, “Eliminate Obsolete Object References”
[Coomes 2007]
“Garbage Collection Concepts and Programming Tips”
[Goetz 2004]
Java Theory and Practice: Garbage Collection and Performance
[Lo 2005]
“Security Issues in Garbage Collection”
[Long 2012]
OBJ05-J. Defensively copy private mutable class members before returning their references
OBJ06-J. Defensively copy mutable inputs and mutable internal components
[Oracle 2010a]
Java SE 6 HotSpot™ Virtual Machine Garbage Collection Tuning