EXC07-J. Restore prior object state on method failure

Lack of concern about leaving objects in an inconsistent state upon exceptional conditions may spring up a gaping hole of exploitable vulnerabilities in them. Techniques that are typically employed to avoid this scenario are:

• Input validation (for example, method parameters)
• Reordering the logic so that the code capable of resulting in the exceptional condition executes before the code that modifies the object
• Through the use of rollbacks upon intercepting a failure notification
• Performing required operations on a temporary copy and committing to the original object after their successful completion

Noncompliant Code Example

This noncompliant code example shows a Dimensions class that contains three internal attributes, the length, width and height of a rectangular box. The getVolumePackage() method is designed to return the total volume required to hold the box, after accounting for packing material which adds a further 2 units to the dimensions of each side. Non negative values are rejected in the subsequent input validation. Also, weight of the object is passed in as a parameter and cannot be more than 20 units. Consider the case where the weight is more than 20 units (21 units, here). This will cause an IllegalArgumentException which will be intercepted by the custom error reporter. While the logic restores the object's original state in the absence of this exception, it omits doing the same from within the catch block. This violates the object's invariants and when getVolumePackage() is called for the second time, it produces incorrect results.

class Dimensions {
  private int length;
  private int width;
  private int height;

  public Dimensions(int length, int width, int height) {
    this.length = length;
    this.width = width;
    this.height = height;
  }

  protected int getVolumePackage(int weight) {
    length += 2;
    width  += 2;
    height += 2;
    try {
      if(length <= 0 || width <= 0 || height <= 0 || weight <= 0 || weight >= 20)
        throw new IllegalArgumentException();
	
        int volume = length * width * height; // 12 * 12 * 12 = 1728
        length -=2; width -= 2; height -= 2; // revert back
        return volume;
      } catch(Throwable t) { 
        MyExceptionReporter mer = new MyExceptionReporter();
        mer.report(t); // sanitize 
        return -1; // non-positive error code
      }	
  }

  public static void main(String[] args) {
    Dimensions d = new Dimensions(10,10,10);
    System.out.println(d.getVolumePackage(21)); // prints -1 (error)
    System.out.println(d.getVolumePackage(19)); // prints 2744 instead of 1728
  }
}

Compliant Solution

To be compliant, restore prior object state on exceptional conditions.

// ...
} catch(Throwable t) { 
  MyExceptionReporter mer = new MyExceptionReporter();
  mer.report(t); // sanitize 
  length -=2; width -= 2; height -= 2; // revert back
  return -1;
}	

Compliant Solution

Another preferable way is to perform input validation before modifying the state of the object.

protected int getVolumePackage(int weight) {
  try {
    if(length <= 0 || width <= 0 || height <= 0 || weight <= 0 || weight > 20)
      throw new IllegalArgumentException(); // validate first
    
    length += 2;
    width  += 2;
    height += 2;

    int volume = length * width * height;
    length -=2; width -= 2; height -= 2;
    return volume;
  } catch(Throwable t) { MyExceptionReporter mer = new MyExceptionReporter();
    mer.report(t); // sanitize 
    return -1;
  }		
}

Risk Assessment

Failing to restore prior object state on method failure can leave the object in an inconsistent state.

Automated Detection

TODO

Related Vulnerabilities

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

References

[[Bloch 08]] Item 64: Strive for failure atomicity

EXC06-J. Be wary of code that can throw undeclared checked exceptions      11. Exceptional Behavior (EXC)      EXC30-J. Do not exit abruptly from a finally block