Java coding guidelines: 75 recommendations for reliable and secure programs (2014)
Chapter 4. Program Understandability
Program understandability is the ease with which the program can be understood—that is, the ability to determine what a program does and how it works by reading its source code and accompanying documentation [Grubb 2003]. Understandable code is easier to maintain because software maintainers are less likely to introduce defects into code that is clear and comprehensible. Understandability helps in manual analysis of source code because it allows the auditor to more easily spot defects and vulnerabilities.
Some guidelines in this chapter are stylistic in nature; they will help a Java programmer to write clearer, more readable code. Failure to follow these guidelines could result in obscure code and design defects.
50. Be careful using visually misleading identifiers and literals
Use visually distinct identifiers that are unlikely to be misread during development and review of code. Depending on the fonts used, certain characters are visually similar or even identical and can be misinterpreted. Consider the examples in Table 4–1.
Table 4–1. Misleading characters
The Java Language Specification (JLS) mandates that program source code be written using the Unicode character encoding [Unicode 2013]. Some distinct Unicode characters share identical glyph representation when displayed in many common fonts. For example, the Greek and Coptic characters (Unicode Range 0370–03FF) are frequently indistinguishable from the Greek-character subset of the Mathematical Alphanumeric Symbols (Unicode Range 1D400–1D7FF).
Avoid defining identifiers that include Unicode characters with overloaded glyphs. One straightforward approach is to use only ASCII or Latin-1 characters in identifiers. Note that the ASCII character set is a subset of Unicode.
Do not use multiple identifiers that vary only by one or more visually similar characters. Also, make the initial portions of long identifiers distinct to aid recognition.
According to the JLS, §3.10.1, “Integer Literals” [JLS 2013],
An integer literal is of type long if it is suffixed with an ASCII letter L or l (ell); otherwise, it is of type int. The suffix L is preferred because the letter l (ell) is often hard to distinguish from the digit 1 (one).
Consequently, use L, not l, to clarify programmer intent when indicating that an integer literal is of type long.
Integer literals with leading zeros, in actuality, denote octal values not decimal values. According to §3.10.1, “Integer Literals,” of the JLS [JLS 2013],
An octal numeral consists of an ASCII digit 0 followed by one or more of the ASCII digits 0 through 7 interspersed with underscores, and can represent a positive, zero, or negative integer.
This misinterpretation may result in programming errors and is more likely to occur while declaring multiple constants and trying to enhance the formatting with zero padding.
Noncompliant Code Example
This noncompliant code example has two variables, stem and stern, within the same scope that can be easily confused and accidentally interchanged:
int stem; // Position near the front of the boat
/* ... */
int stern; // Position near the back of the boat
Compliant Solution
This compliant solution eliminates the confusion by assigning visually distinct identifiers to the variables:
int bow; // Position near the front of the boat
/* ... */
int stern; // Position near the back of the boat
Noncompliant Code Example
This noncompliant code example prints the result of adding an int and a long value, even though it may appear that two integers (11111) are being added:
public class Visual {
public static void main(String[] args) {
System.out.println(11111 + 1111l);
}
}
Compliant Solution
This compliant solution uses an uppercase L (long) instead of lowercase l to disambiguate the visual appearance of the second integer. Its behavior is the same as that of the noncompliant code example, but the programmer’s intent is clear:
public class Visual {
public static void main(String[] args) {
System.out.println(11111 + 1111L);
}
}
Noncompliant Code Example
This noncompliant code example mixes decimal values and octal values while storing them in an array:
int[] array = new int[3];
void exampleFunction() {
array[0] = 2719;
array[1] = 4435;
array[2] = 0042;
// ...
}
It appears that the third element in array is intended to hold the decimal value 42. However, the decimal value 34 (corresponding to the octal value 42) gets assigned.
Compliant Solution
When integer literals are intended to represent a decimal value, avoid padding with leading zeros. Use another technique instead, such as padding with whitespace, to preserve digit alignment.
int[] array = new int[3];
void exampleFunction() {
array[0] = 2719;
array[1] = 4435;
array[2] = 42;
// ...
}
Applicability
Failing to use visually distinct identifiers could result in the use of the wrong identifier and lead to unexpected program behavior.
Heuristic detection of identifiers with visually similar names is straightforward. Confusing a lowercase letter l with a digit 1 when indicating that an integer denotation is a long value can result in incorrect computations. Automated detection is trivial.
Mixing decimal and octal values can result in improper initialization or assignment.
Detection of integer literals that have a leading zero is trivial. However, determining whether the programmer intended to use an octal literal or a decimal literal is infeasible. Accordingly, sound automated detection is also infeasible. Heuristic checks may be useful.
Bibliography
[Bloch 2005]
Puzzle 4, “It’s Elementary”
[JLS 2013]
§3.10.1, “Integer Literals”
[Seacord 2009]
DCL02-C. Use visually distinct identifiers
[Unicode 2013]
51. Avoid ambiguous overloading of variable arity methods
The variable arity (varargs) feature was introduced in JDK v1.5.0 to support methods that accept a variable number of arguments.
According to the Java SE 6 Documentation [Oracle 2011b],
As an API designer, you should use [variable arity methods] sparingly, only when the benefit is truly compelling. Generally speaking, you should not overload a varargs method, or it will be difficult for programmers to figure out which overloading gets called.
Noncompliant Code Example
In this noncompliant code example, overloading variable arity methods makes it unclear which definition of displayBooleans() is invoked:
class Varargs {
private static void displayBooleans(boolean... bool) {
System.out.print("Number of arguments: "
+ bool.length + ", Contents: ");
for (boolean b : bool) {
System.out.print("[" + b + "]");
}
}
private static void displayBooleans(boolean bool1,
boolean bool2) {
System.out.println("Overloaded method invoked");
}
public static void main(String[] args) {
displayBooleans(true, false);
}
}
When run, this program outputs
Overloaded method invoked
because the nonvariable arity definition is more specific and consequently a better fit for the provided arguments. However, this complexity is best avoided.
Compliant Solution
To avoid overloading variable arity methods, use distinct method names to ensure that the intended method is invoked, as shown in this compliant solution:
class Varargs {
private static void displayManyBooleans(boolean... bool) {
System.out.print("Number of arguments: "
+ bool.length + ", Contents: ");
for (boolean b : bool) {
System.out.print("[" + b + "]");
}
}
private static void displayTwoBooleans(boolean bool1,
boolean bool2) {
System.out.println("Overloaded method invoked");
System.out.println("Contents: ["
+ bool1 + "], [" + bool2 + "]");
}
public static void main(String[] args) {
displayManyBooleans(true, false);
}
}
Applicability
Injudicious use of overloaded variable arity methods may create ambiguity and diminish code readability.
It may be desirable to violate this rule for performance reasons. One such reason would be to avoid the cost of creating an array instance and initializing it on every invocation of a method [Bloch 2008].
public void foo() { }
public void foo(int a1) { }
public void foo(int a1, int a2, int... rest) { }
When overloading variable arity methods, it is important to avoid any ambiguity regarding which method should be invoked. The preceding code sample avoids the possibility of incorrect method selection by using unambiguous method signatures.
Automated detection is straightforward.
Bibliography
[Bloch 2008]
Item 42, “Use Varargs Judiciously”
[Steinberg 2008]
Using the Varargs Language Feature
[Oracle 2011b]
Varargs
52. Avoid in-band error indicators
An in-band error indicator is a value returned by a method that indicates either a legitimate return value or an illegitimate value that denotes an error. Some common examples of in-band error indicators include
A valid object or a null reference.
An integer indicating a positive value, or −1 to indicate that an error occurred.
An array of valid objects or a null reference indicating the absence of valid objects. (This topic is further addressed in Guideline 41, “Return an empty array or collection instead of a null value for methods that return an array or collection.”)
In-band error indicators require the caller to check for the error; however, this checking is often overlooked. Failure to check for such error conditions not only violates The CERT® Oracle® Secure Coding Standard for Java™ [Long 2012], “EXP00-J. Do not ignore values returned by methods,” but also has the unfortunate effect of propagating invalid values that may subsequently be treated as valid in later computations.
Avoid the use of in-band error indicators. They are much less common in Java’s core library than in some other programming languages; nevertheless, they are used in the read(byte[] b, int off, int len) and read(char[] cbuf, int off, int len) families of methods in java.io.
In Java, the best way to indicate an exceptional situation is by throwing an exception rather than by returning an error code. Exceptions are propagated across scopes and cannot be ignored as easily as error codes can. When using exceptions, the error-detection and error-handling code is kept separate from the main flow of control.
Noncompliant Code Example
This noncompliant code example attempts to read into an array of characters and to add an extra character into the buffer immediately after the characters that are read.
static final int MAX = 21;
static final int MAX_READ = MAX - 1;
static final char TERMINATOR = '\\';
int read;
char [] chBuff = new char[MAX];
BufferedReader buffRdr;
// Set up buffRdr
read = buffRdr.read(chBuff, 0, MAX_READ);
chBuff[read] = TERMINATOR;
However, if the input buffer is initially at end-of-file, the read() method will return −1, and the attempt to place the terminator character will throw an ArrayIndexOutOfBoundsException.
Compliant Solution (Wrapping)
This compliant solution defines a readSafe() method that wraps the original read() method and throws an exception if end-of-file is detected:
public static int readSafe(BufferedReader buffer, char[] cbuf,
int off, int len) throws IOException {
int read = buffer.read(cbuf, off, len);
if (read == -1) {
throw new EOFException();
} else {
return read;
}
}
// ...
BufferedReader buffRdr;
// Set up buffRdr
try {
read = readSafe(buffRdr, chBuff, 0, MAX_READ);
chBuff[read] = TERMINATOR;
} catch (EOFException eof) {
chBuff[0] = TERMINATOR;
}
Applicability
Using in-band error indicators may result in programmers either failing to check status codes or using incorrect return values, leading to unexpected behavior.
Given the comparatively rare occurrence of in-band error indicators in Java, it may be possible to compile a list of all standard library methods that use them and to automatically detect their use. However, detecting the safe use of in-band error indicators is not feasible in the general case.
Returning an object that might be null on failure or a valid object on success is a common example of an in-band error indicator. Although better method designs are often available, returning an object that may be null can be acceptable under some circumstances. See Guideline 26, “Always provide feedback about the resulting value of a method,” for an example.
Bibliography
[API 2013]
Class Reader
[JLS 2013]
Chapter 11, “Exceptions”
[Long 2012]
EXP00-J. Do not ignore values returned by methods
53. Do not perform assignments in conditional expressions
Using the assignment operator in conditional expressions frequently indicates programmer error and can result in unexpected behavior. The assignment operator should not be used in the following contexts:
if (controlling expression)
while (controlling expression)
do ... while (controlling expression)
for (second operand)
switch (controlling expression)
?: (first operand)
&& (either operand)
|| (either operand)
?: (second or third operands) where the ternary expression is used in any of these contexts
Noncompliant Code Example
In this noncompliant code example, the controlling expression in the if statement is an assignment expression.
public void f(boolean a, boolean b) {
if (a = b) {
/* ... */
}
}
Although the programmer’s intent could have been to assign b to a and test the value of the result, this usage frequently occurs when the programmer mistakenly uses the assignment operator = rather than the equality operator ==.
Compliant Solution
The conditional block shown in this compliant solution only executes when a is equal to b.
public void f(boolean a, boolean b) {
if (a == b) {
/* ... */
}
}
Unintended assignment of b to a cannot occur.
Compliant Solution
When the assignment is intentional, this compliant solution clarifies the programmer’s intent:
public void f(boolean a, boolean b) {
if ((a = b) == true) {
/* ... */
}
}
Compliant Solution
It is clearer to express the logic as an explicit assignment followed by the if condition:
public void f(boolean a, boolean b) {
a = b;
if (a) {
/* ... */
}
}
Noncompliant Code Example
In this noncompliant code example, an assignment expression appears as an operand of the && operator:
public void f(boolean a, boolean b, boolean flag) {
while ( (a = b) && flag ) {
/* ... */
}
}
Because && is not a comparison operator, assignment is an illegal operand. Again, this is frequently a case of the programmer mistakenly using the assignment operator = instead of the equals operator ==.
Compliant Solution
When the assignment of b to a is unintentional, this conditional block is now executed only when a is equal to b and flag is true:
public void f(boolean a, boolean b, boolean flag) {
while ( (a == b) && flag ) {
/* ... */
}
}
Applicability
The use of the assignment operator in controlling conditional expressions frequently indicates programmer error and can result in unexpected behavior.
As an exception to this guideline, it is permitted to use the assignment operator in conditional expressions when the assignment is not the controlling expression (that is, the assignment is a subexpression), as shown in the following compliant solution.
public void assignNocontrol(BufferedReader reader)
throws IOException {
String line;
while ((line = reader.readLine()) != null) {
// ... Work with line
}
}
Bibliography
[Hatton 1995]
§2.7.2, “Errors of Omission and Addition”
54. Use braces for the body of an if, for, or while statement
Use opening and closing braces for if, for, and while statements even when the body contains only a single statement. Braces improve the uniformity and readability of code.
More important, it is easy to forget to add braces when inserting additional statements into a body containing only a single statement, because the conventional program indentation gives strong (but misleading) guidance to the structure.
Noncompliant Code Example
This noncompliant code example authenticates a user with an if statement that lacks braces.
int login;
if (invalid_login())
login = 0;
else
login = 1;
This program behaves as expected. However, a maintainer might subsequently add a debug statement or other logic but forget to add opening and closing braces:
int login;
if (invalid_login())
login = 0;
else
// Debug line added below
System.out.println("Login is valid\n");
// The next line is always executed
login = 1;
The code’s indentation disguises the functionality of the program, potentially leading to a security vulnerability.
Compliant Solution
This compliant solution uses opening and closing braces even though the if and else bodies of the if statement are single statements:
int login;
if (invalid_login()) {
login = 0;
} else {
login = 1;
}
Noncompliant Code Example
This noncompliant code example nests an if statement within another if statement, without braces around the if and else bodies:
int privileges;
if (invalid_login())
if (allow_guests())
privileges = GUEST;
else
privileges = ADMINISTRATOR;
The indentation might lead the programmer to believe users are granted administrator privileges only when their login is valid. However, the else statement actually binds to the inner if statement:
int privileges;
if (invalid_login())
if (allow_guests())
privileges = GUEST;
else
privileges = ADMINISTRATOR;
Consequently, this defect allows unauthorized users to obtain administrator privileges.
Compliant Solution
This compliant solution uses braces to eliminate the ambiguity, consequently ensuring that privileges are correctly assigned:
int privileges;
if (invalid_login()) {
if (allow_guests()) {
privileges = GUEST;
}
} else {
privileges = ADMINISTRATOR;
}
Applicability
Failure to enclose the bodies of if, for, or while statements in braces makes code error prone and increases maintenance costs.
Bibliography
[GNU 2013]
§5.3, “Clean Use of C Constructs”
55. Do not place a semicolon immediately following an if, for, or while condition
Do not use a semicolon after an if, for, or while condition because it typically indicates programmer error and can result in unexpected behavior.
Noncompliant Code Example
In this noncompliant code example, a semicolon is used immediately following an if condition.
if (a == b); {
/* ... */
}
The statements in the apparent body of the if statement are always evaluated, regardless of the result of the condition expression.
Compliant Solution
This compliant solution eliminates the semicolon and ensures that the body of the if statement is executed only when the condition expression is true:
if (a == b) {
/* ... */
}
Applicability
Placing a semicolon immediately following an if, for, or while condition may result in unexpected behavior.
Bibliography
[Hatton 1995]
§2.7.2, “Errors of Omission and Addition”
56. Finish every set of statements associated with a case label with a break statement
A switch block comprises several case labels and an optional but highly recommended default label. Statements that follow each case label must end with a break statement, which is responsible for transferring the control to the end of the switch block. When omitted, the statements in the subsequent case label are executed. Because the break statement is optional, omitting it produces no compiler warnings. When this behavior is unintentional, it can cause unexpected control flow.
Noncompliant Code Example
In this noncompliant code example, the case where the card is 11 lacks a break statement. As a result, execution continues with the statements for card = 12.
int card = 11;
switch (card) {
/* ... */
case 11:
System.out.println("Jack");
case 12:
System.out.println("Queen");
break;
case 13:
System.out.println("King");
break;
default:
System.out.println("Invalid Card");
break;
}
Compliant Solution
This compliant solution terminates each case with a break statement.
int card = 11;
switch (card) {
/* ... */
case 11:
System.out.println("Jack");
break;
case 12:
System.out.println("Queen");
break;
case 13:
System.out.println("King");
break;
default:
System.out.println("Invalid Card");
break;
}
Applicability
Failure to include break statements can cause unexpected control flow.
The break statement at the end of the final case in a switch statement may be omitted. By convention, this is the default label. The break statement serves to transfer control to the end of the switch block. Fall-through behavior also causes control to arrive at the end of the switch block. Consequently, control transfers to the statements following the switch block without regard to the presence or absence of the break statement. Nevertheless, the final case in a switch statement should end with a break statement in accordance with good programming style [Allen 2000].
Exceptionally, when multiple cases require execution of identical code, break statements may be omitted from all cases except the last one. Similarly, when processing for one case is a proper prefix of processing for one or more other cases, the break statement may be omitted from the prefix case. This should be clearly indicated with a comment. For example:
int card = 11;
int value;
// Cases 11,12,13 fall through to the same case
switch (card) {
// Processing for this case requires a prefix
// of the actions for the following three
case 10:
do_something(card);
// Intentional fall-through
// These three cases are treated identically
case 11: // Break not required
case 12: // Break not required
case 13:
value = 10;
break; // Break required
default:
// Handle error condition
}
Also, when a case ends with a return or throw statement or nonreturning method such as System.exit(), the break statement may be omitted.
Bibliography
[JLS 2013]
§14.11, “The switch Statement”
57. Avoid inadvertent wrapping of loop counters
Unless coded properly, a while or for loop may execute forever, or until the counter wraps around and reaches its final value. (See The CERT® Oracle® Secure Coding Standard for Java™ [Long 2012], “NUM00-J. Detect or prevent integer overflow.”) This problem may result from incrementing or decrementing a loop counter by more than one and then testing for equality to a specified value to terminate the loop. In this case, it is possible that the loop counter will leapfrog the specified value and execute either forever, or until the counter wraps around and reaches its final value. This problem may also be caused by naïve testing against limits; for example, looping while a counter is less than or equal to Integer.MAX_VALUE or greater than or equal to Integer.MIN_VALUE.
Noncompliant Code Example
This noncompliant code example appears to iterate five times.
for (i = 1; i != 10; i += 2) {
// ...
}
However, the loop never terminates. Successive values of i are 1, 3, 5, 7, 9, 11, and so on; the comparison with 10 never evaluates to true. The value reaches the maximum representable positive number (Integer.MAX_VALUE), then wraps to the second lowest negative number (Integer.MIN_VALUE + 1). It then works its way up to −1, then 1, and proceeds as described earlier.
Noncompliant Code Example
This noncompliant code example terminates but performs many more iterations than expected:
for (i = 1; i != 10; i += 5) {
// ...
}
Successive values of i are 1, 6, and 11, skipping 10. The value of i then wraps from near the maximum positive value to near the lowest negative value and works its way up toward 0. It then assumes 2, 7, and 12, skipping 10 again. After the value wraps from the high positive to the low negative side three more times, it finally reaches 0, 5, and 10, terminating the loop.
Compliant Solution
One solution is to simply ensure the loop termination condition is reached before the counter inadvertently wraps.
for (i = 1; i == 11; i += 2) {
// ...
}
This solution can be fragile when one or more of the conditions affecting the iteration must be changed. A better solution is to use a numerical comparison operator (that is, <, <=, >, or >=) to terminate the loop.
for (i = 1; i <= 10; i += 2) {
// ...
}
This latter solution can be more robust in the event of changes to the iteration conditions. However, this approach should never replace careful consideration regarding the intended and actual number of iterations.
Noncompliant Code Example
A loop expression that tests whether a counter is less than or equal to Integer.MAX_VALUE or greater or equal to Integer.MIN_VALUE will never terminate because the expression will always evaluate to true. For example, the following loop will never terminate because i can never be greater thanInteger.MAX_VALUE:
for (i = 1; i <= Integer.MAX_VALUE; i++) {
// ...
}
Compliant Solution
The loop in this compliant solution terminates when i is equal to Integer.MAX_VALUE:
for (i = 1; i < Integer.MAX_VALUE; i++) {
// ...
}
If the loop is meant to iterate for every value of i greater than 0, including Integer.MAX_VALUE, it can be implemented as follows:
i = 0;
do {
i++
// ...
} while (i != Integer.MAX_VALUE);
Noncompliant Code Example
This noncompliant code example initializes the loop counter i to 0 and then increments it by two on each iteration, basically enumerating all the even, positive values. The loop is expected to terminate when i is greater than Integer.MAX_VALUE − 1, an even value. In this case, the loop fails to terminate because the counter wraps around before becoming greater than Integer.MAX_VALUE − 1.
for (i = 0; i <= Integer.MAX_VALUE - 1; i += 2) {
// ...
}
Compliant Solution
The loop in this compliant solution terminates when the counter i is greater than Integer.MAX_VALUE minus the step value as the loop-terminating condition.
for (i = 0; i <= Integer.MAX_VALUE - 2; i += 2) {
// ...
}
Applicability
Incorrect termination of loops may result in infinite loops, poor performance, incorrect results, and other problems. If any of the conditions used to terminate a loop can be influenced by an attacker, these errors can be exploited to cause a denial of service or other attack.
Bibliography
[JLS 2013]
§15.20.1, “Numerical Comparison Operators <, <=, >, and >=”
[Long 2012]
NUM00-J. Detect or prevent integer overflow
58. Use parentheses for precedence of operation
Programmers frequently make errors regarding the precedence of operators because of the unintuitive low precedence levels of &, |, ^, <<, and >>. Avoid mistakes regarding precedence through the suitable use of parentheses, which also improves code readability. The precedence of operations by the order of the subclauses is listed in the Java™ Tutorials [Tutorials 2013].
Although it advises against depending on parentheses for specifying evaluation order, The CERT® Oracle® Secure Coding Standard for Java™ [Long 2012], “EXP05-J. Do not write more than once to the same variable within an expression,” applies only to expressions that contain side effects.
Noncompliant Code Example
The intent of the expression in this noncompliant code example is to add the variable OFFSET to the result of the bitwise logical AND between x and MASK.
public static final int MASK = 1337;
public static final int OFFSET = -1337;
public static int computeCode(int x) {
return x & MASK + OFFSET;
}
According to the operator precedence guidelines, the expression is parsed as
x & (MASK + OFFSET)
This expression is evaluated as follows, resulting in the value 0:
x & (1337 - 1337)
Compliant Solution
This compliant solution uses parentheses to ensure that the expression is evaluated as intended:
public static final int MASK = 1337;
public static final int OFFSET = -1337;
public static int computeCode(int x) {
return (x & MASK) + OFFSET;
}
Noncompliant Code Example
In this noncompliant code example, the intent is to append either "0" or "1" to the string "value=".
public class PrintValue {
public static void main(String[] args) {
String s = null;
// Prints "1"
System.out.println("value=" + s == null ? 0 : 1);
}
}
However, the precedence rules result in the expression to be printed being parsed as ("value=" + s) == null ? 0 : 1.
Compliant Solution
This compliant solution uses parentheses to ensure that the expression evaluates as intended.
public class PrintValue {
public static void main(String[] args) {
String s = null;
// Prints "value=0" as expected
System.out.println("value=" + (s == null ? 0 : 1));
}
}
Applicability
Mistakes concerning precedence rules can cause an expression to be evaluated in an unintended way, which can result in unexpected and abnormal program behavior.
Parentheses may be omitted from mathematical expressions that follow the algebraic precedence rules. For example, consider the following expression:
x + y * z
By mathematical convention, multiplication is performed before addition; parentheses are redundant in this case:
x + (y * z)
Detection of all expressions using low-precedence operators without parentheses is straightforward. Determining the correctness of such uses is infeasible in the general case, although heuristic warnings could be useful.
Bibliography
[ESA 2005]
Rule 65, Use parentheses to explicitly indicate the order of execution of numerical operators
[Long 2012]
EXP05-J. Do not write more than once to the same variable within an expression
[Tutorials 2013]
Expressions, Statements, and Blocks
59. Do not make assumptions about file creation
Although creating a file is usually accomplished with a single method call, this single action raises multiple security-related questions. What should be done if the file cannot be created? What should be done if the file already exists? What should be the file’s initial attributes, such as permissions?
Java provides several generations of file-handling facilities. The original input/output facilities, which included basic file handling, are in the package java.io. More comprehensive facilities were included in JDK 1.4 with the New I/O package java.nio (see New I/O APIs [Oracle 2010b]). Still more comprehensive facilities were included in JDK 1.7 with the New I/O 2 package java.nio.file. Both packages introduced a number of methods to support finer-grained control over file creation.
The CERT® Oracle® Secure Coding Standard for Java™ [Long 2012], “FIO01-J. Create files with appropriate access permissions,” explains how to specify the permissions of a newly created file.
Noncompliant Code Example
This noncompliant code example tries to open a file for writing:
public void createFile(String filename)
throws FileNotFoundException{
OutputStream out = new FileOutputStream(filename);
// Work with file
}
If the file exists before being opened, its former contents will be overwritten with the contents provided by the program.
Noncompliant Code Example (TOCTOU)
This noncompliant code example tries to avoid altering an existing file by creating an empty file using java.io.File.createNewFile(). If a file with the given name already exists, then createNewFile() will return false without destroying the named file’s contents.
public void createFile(String filename)
throws FileNotFoundException{
OutputStream out = new FileOutputStream(filename, true);
if (!new File(filename).createNewFile()) {
// File cannot be created...handle error
} else {
out = new FileOutputStream(filename);
// Work with file
}
}
Unfortunately, this solution is subject to a TOCTOU (time-of-check, time-of-use) race condition. It is possible for an attacker to modify the file system after the empty file is created but before the file is opened, such that the file that is opened is distinct from the file that was created.
Compliant Solution (Files)
This compliant solution uses the java.nio.file.Files.newOutputStream() method to atomically create the file, and throw an exception if the file already exists:
public void createFile(String filename)
throws FileNotFoundException{
try (OutputStream out = new BufferedOutputStream(
Files.newOutputStream(Paths.get(filename),
StandardOpenOption.CREATE_NEW))) {
// Work with out
} catch (IOException x) {
// File not writable...Handle error
}
}
Applicability
The ability to determine whether an existing file has been opened or a new file has been created provides greater assurance that only the intended file is opened or overwritten and that other files remain unaffected.
Bibliography
[API 2013]
Class java.io.File
Class java.nio.file.Files
[Long 2012]
FIO01-J. Create files with appropriate access permissions
[Oracle 2010b]
New I/O APIs
60. Convert integers to floating-point for floating-point operations
Incautious use of integer arithmetic to calculate a value for assignment to a floating-point variable can result in loss of information. For example, integer arithmetic always produces integral results, discarding information about any possible fractional remainder. Furthermore, there can be loss of precision when converting integers to floating-point values. 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 additional information. Correct programming of expressions that combine integer and floating-point values requires careful consideration.
Operations that could suffer from integer overflow or loss of a fractional remainder should be performed on floating-point values, rather than integral values.
Noncompliant Code Example
In this noncompliant code example, the division and multiplication operations are performed on integral values; the results of these operations are then converted to floating-point.
short a = 533;
int b = 6789;
long c = 4664382371590123456L;
float d = a / 7; // d is 76.0 (truncated)
double e = b / 30; // e is 226.0 (truncated)
double f = c * 2; // f is -9.1179793305293046E18
// because of integer overflow
The results of the integral operations are truncated to the nearest integer and can also overflow. As a result, the floating-point variables d, e, and f are initialized incorrectly because the truncation and overflow take place before the conversion to floating-point.
Note that the calculation for c also violates “NUM00-J. Detect or prevent integer overflow” [Long 2012].
Compliant Solution (Floating-Point Literal)
The following compliant solution performs the multiplication and division operations on floating-point values, avoiding both the truncation and the overflow seen in the noncompliant code example. In every operation, at least one of the operands is of a floating-point type, forcing floating-point multiplication and division, and avoiding truncation and overflow:
short a = 533;
int b = 6789;
long c = 4664382371590123456L;
float d = a / 7.0f; // d is 76.14286
double e = b / 30.; // e is 226.3
double f = (double)c * 2; // f is 9.328764743180247E18
Another compliant solution is to eliminate the truncation and overflow errors by storing the integers in the floating-point variables before performing the arithmetic operations:
short a = 533;
int b = 6789;
long c = 4664382371590123456L;
float d = a;
double e = b;
double f = c;
d /= 7; // d is 76.14286
e /= 30; // e is 226.3
f *= 2; // f is 9.328764743180247E18
As in the previous compliant solution, this practice ensures that at least one of the operands of each operation is a floating-point number. Consequently, the operations are performed on floating-point values.
In both compliant solutions, the original value of c cannot be represented exactly as a double. The representation of type double has only 48 mantissa bits, but a precise representation of the value of c would require 56 mantissa bits. Consequently, the value of c is rounded to the nearest value that can be represented by type double, and the computed value of f (9.328764743180247E18) differs from the exact mathematical result (9328564743180246912). This loss of precision is one of the many reasons correct programming of expressions that mix integer and floating-point operations or values requires careful consideration. See “NUM13-J. Avoid loss of precision when converting primitive integers to floating-point” [Long 2012], for more information about integer-to-floating-point conversion. Even with this loss of precision, however, the computed value of fis far more accurate than that produced in the noncompliant code example.
Noncompliant Code Example
This noncompliant code example attempts to compute the whole number greater than the ratio of two integers. The result of the computation is 1.0 rather than the intended 2.0.
int a = 60070;
int b = 57750;
double value = Math.ceil(a/b);
As a consequence of Java’s numeric promotion rules, the division operation performed is an integer division whose result is truncated to 1. This result is then promoted to double before being passed to the Math.ceil() method.
Compliant Solution
This compliant solution casts one of the operands to double before the division is performed.
int a = 60070;
int b = 57750;
double value = Math.ceil(a/((double) b));
As a result of the cast, the other operand is automatically promoted to double. The division operation becomes a double divide, and value is assigned the correct result of 2.0. As in previous compliant solutions, this practice ensures that at least one of the operands of each operation is a floating-point number.
Applicability
Improper conversions between integers and floating-point values can yield unexpected results, especially from precision loss. In some cases, these unexpected results can involve overflow or other exceptional conditions.
It is acceptable to perform operations using a mix of integer and floating-point values when deliberately exploiting the properties of integer arithmetic before conversion to floating-point. For example, use of integer arithmetic eliminates the need to use the floor() method. Any such codemust be clearly documented to help future maintainers understand that this behavior is intentional.
Bibliography
[JLS 2013]
§5.1.2, “Widening Primitive Conversion”
[Long 2012]
NUM13-J. Avoid loss of precision when converting primitive integers to floating-point
NUM00-J. Detect or prevent integer overflow
61. Ensure that the clone() method calls super.clone()
Cloning a subclass of a nonfinal class which defines a clone() method that fails to call super.clone() will produce an object of the wrong class.
The Java API [API 2013] for the clone() method says:
By convention, the returned object should be obtained by calling super.clone. If a class and all of its superclasses (except Object) obey this convention, it will be the case that x.clone().getClass() == x.getClass().
Noncompliant Code Example
In this noncompliant code example, the clone() method in the class Base fails to call super.clone():
class Base implements Cloneable {
public Object clone() throws CloneNotSupportedException {
return new Base();
}
protected void doLogic() {
System.out.println("Superclass doLogic");
}
}
class Derived extends Base {
public Object clone() throws CloneNotSupportedException {
return super.clone();
}
protected void doLogic() {
System.out.println("Subclass doLogic");
}
public static void main(String[] args) {
Derived dev = new Derived();
try {
Base devClone = (Base)dev.clone(); // Has type Base
// instead of Derived
devClone.doLogic(); // Prints "Superclass doLogic"
// instead of "Subclass doLogic"
} catch (CloneNotSupportedException e) { /* ... */ }
}
}
Consequently, the object devClone ends up being of type Base instead of Derived, and the doLogic() method is incorrectly applied.
Compliant Solution
This compliant solution correctly calls super.clone() in the Base class’s clone() method.
class Base implements Cloneable {
public Object clone() throws CloneNotSupportedException {
return super.clone();
}
protected void doLogic() {
System.out.println("Superclass doLogic");
}
}
class Derived extends Base {
public Object clone() throws CloneNotSupportedException {
return super.clone();
}
protected void doLogic() {
System.out.println("Subclass doLogic");
}
public static void main(String[] args) {
Derived dev = new Derived();
try {
// Has type Derived, as expected
Base devClone = (Base)dev.clone();
devClone.doLogic(); // Prints "Subclass doLogic"
// as expected
} catch (CloneNotSupportedException e) { /* ... */ }
}
}
Applicability
Failing to call super.clone() may cause a cloned object to have the wrong type.
Bibliography
[API 2013]
Class Object
62. Use comments consistently and in a readable fashion
Mixing the use of traditional or block comments (starting with /* and ending with */) and end-of-line comments (from // to the end of the line) can lead to misleading and confusing code, which may result in errors.
Noncompliant Code Example
The following noncompliant code examples show mixed comments that may be misunderstood.
// */ /* Comment, not syntax error */
f = g/**//h; /* Equivalent to f = g / h; */
/*//*/ l(); /* Equivalent to l(); */
m = n//**/o
+ p; /* Equivalent to m = n + p; */
a = b //*divisor:*/c
+ d; /* Equivalent to a = b + d; */
Compliant Solution
Use a consistent style of commenting.
// Nice simple comment
int i; // Counter
Noncompliant Code Example
There are other misuses of comments that should be avoided. This noncompliant code example uses the character sequence /* to begin a comment, but neglects to use the delimiter */ to end the comment. Consequently, the call to the security-critical method is not executed. A reviewer examining this page could incorrectly assume that the code is executed.
/* Comment with end comment marker unintentionally omitted
security_critical_method();
/* Some other comment */
Using an editor that provides syntax highlighting or that formats the code to identify issues such as missing end comment delimiters can help detect accidental omissions.
Because missing end delimiters are error prone and often viewed as a mistake, this approach is not recommended for commenting out code.
Compliant Solution
This compliant solution demonstrates the recommended way to mark code as “dead.” It also takes advantage of the compiler’s ability to remove unreachable (dead) code. The code inside the if block must be syntactically correct. If other parts of the program later change in a way that would cause syntax errors, the unexecuted code must be brought up to date to correct the problem. Then, if it is needed again in the future, the programmer need only remove the surrounding if statement and the NOTREACHED comment.
The NOTREACHED comment could tell some compilers and static analysis tools not to complain about this unreachable code. It also serves as documentation.
if (false) { /* Use of critical security method no
* longer necessary, for now */
/* NOTREACHED */
security_critical_method();
/* Some other comment */
}
This is an example of an exceptional situation described in Guideline 63, “Detect and remove superfluous code and values.”
Applicability
Confusion over which instructions are executed and which are not can result in serious programming errors and vulnerabilities, including denial of service, abnormal program termination, and data integrity violation. This problem is mitigated by the use of interactive development environments (IDEs) and editors that use fonts, colors, or other mechanisms to differentiate between comments and code. However, the problem can still manifest, for example, when reviewing source code printed on a black-and-white printer.
Nested block comments and inconsistent use of comments can be detected by suitable static analysis tools.
Bibliography
[JLS 2013]
§3.7, “Comments”
63. Detect and remove superfluous code and values
Superfluous code and values may occur in the form of dead code, code that has no effect, and unused values in program logic.
Code that is never executed is known as dead code. Typically, the presence of dead code indicates that a logic error has occurred as a result of changes to a program or to the program’s environment. Dead code is often optimized out of a program during compilation. However, to improve readability and ensure that logic errors are resolved, dead code should be identified, understood, and removed.
Code that is executed but fails to perform any action, or that has an unintended effect, most likely results from a coding error and can cause unexpected behavior. Statements or expressions that have no effect should be identified and removed from code. Most modern compilers can warn about code that has no effect.
The presence of unused values in code may indicate significant logic errors. To prevent such errors, unused values should be identified and removed from code.
Noncompliant Code Example (Dead Code)
This noncompliant code example demonstrates how dead code can be introduced into a program [Fortify 2013]:
public int func(boolean condition) {
int x = 0;
if (condition) {
x = foo();
/* Process x */
return x;
}
/* ... */
if (x != 0) {
/* This code is never executed */
}
return x;
}
The condition in the second if statement, (x != 0), will never evaluate to true because the only path where x can be assigned a nonzero value ends with a return statement.
Compliant Solution
Remediation of dead code requires the programmer to determine not only why the code is never executed, but also whether the code should have been executed, and then to resolve that situation appropriately. This compliant solution assumes that the dead code should have executed, and consequently the body of the first conditional statement no longer ends with a return.
int func(boolean condition) {
int x = 0;
if (condition) {
x = foo();
/* Process x */
}
/* ... */
if (x != 0) {
/* This code is now executed */
}
return 0;
}
Noncompliant Code Example (Dead Code)
In this example, the length() function is used to limit the number of times the function string_loop() iterates. The condition of the if statement inside the loop evaluates to true when the current index is the length of str. However, because i is always strictly less than str.length(), that can never happen.
public int string_loop(String str) {
for (int i=0; i < str.length(); i++) {
/* ... */
if (i == str.length()) {
/* This code is never executed */
}
}
return 0;
}
Compliant Solution
Proper remediation of the dead code depends on the intent of the programmer. Assuming the intent is to do something special with the last character in str, the conditional statement is adjusted to check whether i refers to the index of the last character in str.
public int string_loop(String str) {
for (int i=0; i < str.length(); i++) {
/* ... */
if (i == str.length()-1) {
/* This code is now executed */
}
}
return 0;
}
Noncompliant Code Example (Code with No Effect)
In this noncompliant code example, the comparison of s to t has no effect:
String s;
String t;
// ...
s.equals(t);
This error is probably the result of the programmer intending to do something with the comparison, but failing to complete the code.
Compliant Solution
In this compliant solution, the result of the comparison is printed out:
String s;
String t;
// ...
if (s.equals(t)) {
System.out.println("Strings equal");
} else {
System.out.println("Strings unequal");
}
Noncompliant Code Example (Unused Values)
In this example, p2 is assigned the value returned by bar(), but that value is never used:
int p1 = foo();
int p2 = bar();
if (baz()) {
return p1;
} else {
p2 = p1;
}
return p2;
Compliant Solution
This example can be corrected in many different ways depending on the intent of the programmer. In this compliant solution, p2 is found to be extraneous. The calls to bar() and baz() could also be removed if they do not produce any side effects.
int p1 = foo();
bar(); /* Removable if bar() lacks side effects */
baz(); /* Removable if baz() lacks side effects */
return p1;
Applicability
The presence of dead code may indicate logic errors that can lead to unintended program behavior. The ways in which dead code can be introduced into a program and the effort required to remove it can be complex. As a result, resolving dead code can be an in-depth process requiring significant analysis.
In exceptional situations, dead code may make software resilient to future changes. An example is the presence of a default case in a switch statement even though all possible switch labels are specified (see Guideline 64, “Strive for logical completeness,” for an illustration of this example).
It is also permissible to temporarily retain dead code that may be needed later. Such cases should be clearly indicated with an appropriate comment.
The presence of code that has no effect can indicate logic errors that may result in unexpected behavior and vulnerabilities. Unused values in code may indicate significant logic errors.
Code and values that have no effect can be detected by suitable static analysis.
Bibliography
[Fortify 2013]
Code Quality: Dead Code
[Coverity 2007]
Coverity Prevent™ User’s Manual (3.3.0)
64. Strive for logical completeness
Software vulnerabilities can result when a programmer fails to consider all possible data states.
Noncompliant Code Example (if Chain)
This noncompliant code example fails to test for conditions in which a is neither b nor c. This may be the correct behavior in this case, but failure to account for all the values of a can result in logic errors if a unexpectedly assumes a different value.
if (a == b) {
/* ... */
}
else if (a == c) {
/* ... */
}
Compliant Solution (if Chain)
This compliant solution explicitly checks for the unexpected condition and handles it appropriately:
if (a == b) {
/* ... */
}
else if (a == c) {
/* ... */
}
else {
/* Handle error condition */
}
Noncompliant Code Example (switch)
Even though x is supposed to represent a bit (0 or 1) in this noncompliant code example, some previous error may have allowed x to assume a different value. Detecting and dealing with that inconsistent state sooner rather than later makes the error easier to find.
switch (x) {
case 0: foo(); break;
case 1: bar(); break;
}
Compliant Solution (switch)
This compliant solution provides the default label to handle all possible values of type int:
switch (x) {
case 0: foo(); break;
case 1: bar(); break;
default: /* Handle error */ break;
}
Noncompliant Code Example (Zune 30)
This noncompliant code example is adapted from C code that appeared in the Zune 30 media player, causing many players to lock up on December 30, 2008, at midnight PST. It contains incomplete logic that causes a denial of service when converting dates.
final static int ORIGIN_YEAR = 1980;
/* Number of days since January 1, 1980 */
public void convertDays(long days){
int year = ORIGIN_YEAR;
/* ... */
while (days > 365) {
if (IsLeapYear(year)) {
if (days > 366) {
days -= 366;
year += 1;
}
} else {
days -= 365;
year += 1;
}
}
}
The original ConvertDays() C function in the real-time clock (RTC) routines for the MC13783 PMIC RTC takes the number of days since January 1, 1980, and computes the correct year and number of days since January 1 of the correct year.
The flaw in the code occurs when days has the value 366 because the loop never terminates. This bug manifested itself on the 366th day of 2008, which was the first leap year in which this code was active.
Compliant Solution (Zune 30)
This proposed rewrite is provided by “A Lesson on Infinite Loops” by Bryant Zadegan [Zadegan 2009]. The loop is guaranteed to exit, as days decreases for each iteration of the loop, unless the while condition fails, in which case the loop terminates.
final static int ORIGIN_YEAR = 1980;
/* Number of days since January 1, 1980 */
public void convertDays(long days){
int year = ORIGIN_YEAR;
/* ... */
int daysThisYear = (IsLeapYear(year) ? 366 : 365);
while (days > daysThisYear) {
days -= daysThisYear;
year += 1;
daysThisYear = (IsLeapYear(year) ? 366 : 365);
}
}
This compliant solution is for illustrative purposes and may differ from the solution implemented by Microsoft.
Applicability
Failing to take into account all possibilities within a logic statement can lead to a corrupted running state, potentially resulting in unintentional information disclosure or abnormal program termination.
Bibliography
[Hatton 1995]
§2.7.2, “Errors of Omission and Addition”
[Viega 2005]
§5.2.17, “Failure to Account for Default Case in Switch”
[Zadegan 2009]
A Lesson on Infinite Loops
65. Avoid ambiguous or confusing uses of overloading
Method and constructor overloading allows declaration of methods or constructors with the same name but with different parameter lists. The compiler inspects each call to an overloaded method or constructor and uses the declared types of the method parameters to decide which method to invoke. In some cases, however, confusion may arise because of the presence of relatively new language features such as autoboxing and generics.
Furthermore, methods or constructors with the same parameter types that differ only in their declaration order are typically not flagged by Java compilers. Errors can result when a developer fails to consult the documentation at each use of a method or constructor. A related pitfall is to associate different semantics with each of the overloaded methods or constructors. Defining different semantics sometimes necessitates different orderings of the same method parameters, creating a vicious circle. Consider, for example, an overloaded getDistance() method in which one overloaded method returns the distance traveled from the source while another (with reordered parameters) returns the remaining distance to the destination. Implementers may fail to realize the difference unless they consult the documentation at each use.
Noncompliant Code Example (Constructor)
Constructors cannot be overridden and can only be overloaded. This noncompliant code example shows the class Con with three overloaded constructors:
class Con {
public Con(int i, String s) {
// Initialization Sequence #1
}
public Con(String s, int i) {
// Initialization Sequence #2
}
public Con(Integer i, String s) {
// Initialization Sequence #3
}
}
Failure to exercise caution while passing arguments to these constructors can create confusion because calls to these constructors contain the same number of similarly typed actual parameters. Overloading must also be avoided when the overloaded constructors or methods provide distinct semantics for formal parameters of the same types, differing solely in their declaration order.
Compliant Solution (Constructor)
This compliant solution avoids overloading by declaring public static factory methods that have distinct names in place of the public class constructors:
public static Con createCon1(int i, String s) {
/* Initialization Sequence #1 */
}
public static Con createCon2(String s, int i) {
/* Initialization Sequence #2 */
}
public static Con createCon3(Integer i, String s) {
/* Initialization Sequence #3 */
}
Noncompliant Code Example (Method)
In this noncompliant code example, the OverLoader class holds a HashMap instance and has overloaded getData() methods. One getData() method chooses the record to return on the basis of its key value in the map; the other chooses on the basis of the actual mapped value.
class OverLoader extends HashMap<Integer,Integer> {
HashMap<Integer,Integer> hm;
public OverLoader() {
hm = new HashMap<Integer, Integer>();
// SSN records
hm.put(1, 111990000);
hm.put(2, 222990000);
hm.put(3, 333990000);
}
public String getData(Integer i) { // Overloading sequence #1
String s = get(i).toString(); // Get a particular record
return (s.substring(0, 3) + "-" + s.substring(3, 5) + "-" +
s.substring(5, 9));
}
public Integer getData(int i) { // Overloading sequence #2
return hm.get(i); // Get record at position 'i'
}
// Checks whether the ssn exists
@Override public Integer get(Object data) {
// SecurityManagerCheck()
for (Map.Entry<Integer, Integer> entry : hm.entrySet()) {
if (entry.getValue().equals(data)) {
return entry.getValue(); // Exists
}
}
return null;
}
public static void main(String[] args) {
OverLoader bo = new OverLoader();
// Get record at index '3'
System.out.println(bo.getData(3));
// Get record containing data '111990000'
System.out.println(bo.getData((Integer)111990000));
}
}
For purposes of overload resolution, the signatures of the getData() methods differ only in the static type of their formal parameters. The OverLoader class inherits from java.util.HashMap and overrides its get() method to provide the checking functionality. This implementation can be extremely confusing to the client who expects both getData() methods to behave in a similar fashion and not depend on whether an index of the record or the value to be retrieved is specified.
Although the client programmer might eventually deduce such behavior, other cases, such as with the List interface, may go unnoticed, as Joshua Bloch [Bloch 2008] describes:
The List<E> interface has two overloadings of the remove method: remove(E) and remove(int). Prior to release 1.5 when it was “generified,” the List interface had a remove(Object) method in place of remove(E), and the corresponding parameter types, Object and int, were radically different. But in the presence of generics and autoboxing, the two parameter types are no longer radically different.
Consequently, a programmer may fail to realize that the wrong element has been removed from the list.
A further problem is that in the presence of autoboxing, adding a new overloaded method definition can break previously working client code. This can happen when a new overloaded method with a more specific type is added to an API whose methods used less specific types in earlier versions. For example, if an earlier version of the OverLoader class provided only the getData(Integer) method, the client could correctly invoke this method by passing a parameter of type int; the result would be selected on the basis of its value because the int parameter would be autoboxed to Integer. Subsequently, when the getData(int) method is added, the compiler resolves all calls whose parameter is of type int to invoke the new getData(int) method, thereby changing their semantics and potentially breaking previously correct code. The compiler is entirely correct in such cases; the actual problem is an incompatible change to the API.
Compliant Solution (Method)
Naming the two related methods differently eliminates both the overloading and the confusion:
public Integer getDataByIndex(int i) {
// No longer overloaded
}
public String getDataByValue(Integer i) {
// No longer overloaded
}
Applicability
Ambiguous or confusing uses of overloading can lead to unexpected results.
Bibliography
[API 2013]
Interface Collection<E>
[Bloch 2008]
Item 41, “Use Overloading Judiciously”