EXP30-J. Do not depend on operator precedence while using expressions containing side-effects

According to the Java Language Specification [[JLS 05]] section 15.7 "Evaluation Order":

The Java programming language guarantees that the operands of operators appear to be evaluated in a specific evaluation order, namely, from left to right.

Section 15.7.3 "Evaluation Respects Parentheses and Precedence", on the other hand states:

Java programming language implementations must respect the order of evaluation as indicated explicitly by parentheses and implicitly by operator precedence.

This guideline demonstrates how conflicts can arise between the above two statements when expressions contain side-effects.

Noncompliant Code Example

This noncompliant example shows how side-effects in expressions can lead to unanticipated outcomes. The programmer intends to write some access control logic based on different threshold levels. Each user has a rating that must be above the threshold, in order to be granted appropriate access. As shown, a simple function is used to calculate the rating. The get() method has been used to provide a non-zero factor when the user is authorized and a zero value, when not.

In this case, the programmer expects that the rightmost subexpression would evaluate first because of the greater precedence of the operator '*' as compared to '+' or '>'. The parenthesis reinforce this belief. These ideas lead to the incorrect conclusion that the right hand side will evaluate to zero whenever the get() method returns zero. The test in the left hand subexpression should ideally reject the unprivileged user as the expected rating value is below the threshold of 10 (= 0, due to number=get()). Ironically, the program grants access to the unauthorized user. The reason is that evaluation of side-effect infested subexpressions follows the left to right ordering rule and should not be confused with the tenets of operator precedence, associativity and indicative parenthesis.

class BadPrecedence {
  public static void main(String[] args) {
    int number=17;
    int[] threshold = new int[20];
    threshold[0] = 10;
    number = (number > threshold[0]? 0:-2) + ((31 * ++number) * (number=get()));
    // ... 
    if(number == 0)
      System.out.println("Access granted");
    else
      System.out.println("Denied access"); // number = -2
  }
  public static int get() {
    int number=0;
    // assign number to non zero value if authorized else 0
    return number;
  }
}

Compliant Solution

While diligently following the left to right evaluation order, a programmer can expect this compliant code to evaluate to an expected final outcome depending on the value returned by the get() method.

number = ((31 * ++number) * (number=get())) + (number > threshold[0]? 0:-2);

Although this solution solves the problem, in general, it is advisable to avoid using expressions with more than one side-effect. It is also inadvisable to depend on the left-right ordering for evaluation of side-effects since operands are evaluated in place first, and then subject to laws of operator precedence and associativity.

Risk Assessment

Side-effects in expressions can be misleading due to their evaluation order. Failing to keep the order in mind can result in unexpected output.

Automated Detection

TODO

Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

References

[[JLS 05]] Section 15.7 "Evaluation Order" and 15.7.3 "Evaluation Respects Parentheses and Precedence"

EXP05-J. Be careful about the wrapper class and autoboxing      02. Expressions (EXP)      02. Expressions (EXP)