Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Edited by NavBot (jp)

It is possible that an exception gets thrown in the finally block even though it escapes detection at compile time. This can prevent other clean-up statements from getting executed.

Noncompliant Code Example

The finally clause closes the reader object in this noncompliant example. However, it is incorrectly assumed that the statements within the finally block cannot throw exceptions. Notably, close() can throw an IOException which in turn prevents any subsequent clean-up lines from getting executed. This is not detected at compile time since close() throws the same exception type as read or write.

Code Block
bgColor#FFCCCC
import java.io.IOException;
import java.io.BufferedReader;
import java.io.FileReader;

public class Login {
  static void checkPassword(String password_file) throws IOException {
  
    StringBuffer fileData = new StringBuffer(1000);
    BufferedReader reader = new BufferedReader(new FileReader(password_file));

    try {
          int n;
          char[] passwd = new char[1024];
          while ((n = reader.read(passwd)) >= 0) {
            String readData = String.valueOf(passwd, 0, n);
            fileData.append(readData);
            passwd = new char[1024];
          }
          String realPassword = "javac<at:var at:name="f3b" />b3";
          System.out.println(fileData.toString());
        
          if (fileData.toString().equals(realPassword)) {
            System.out.println("Login successful");
          }
          else {
            System.out.println("Login failed");
          }
    } finally {
      reader.close();
      //other clean-up code 
    }
}

  public static void main(String[] args) throws IOException {
    String path = "c:\\password.txt";
    checkPassword(path);
  }
}

Compliant Solution

This compliant solution correctly places the close() statement in a try-catch block. As a result an IOException can be handled without letting it propagate any further.

Code Block
bgColor#ccccff
import java.io.IOException;
import java.io.BufferedReader;
import java.io.FileReader;

public class Login {
  static void checkPassword(String password_file) throws IOException {
  
    StringBuffer fileData = new StringBuffer(1000);
    BufferedReader reader = new BufferedReader(new FileReader(password_file));

    try {
          int n;
          char[] passwd = new char[1024];
          while ((n = reader.read(passwd)) >= 0) {
            String readData = String.valueOf(passwd, 0, n);
            fileData.append(readData);
            passwd = new char[1024];
          }
          String realPassword = "javac<at:var at:name="f3b" />b3";
          System.out.println(fileData.toString());
        
          if (fileData.toString().equals(realPassword)) {
            System.out.println("Login successful");
          }
          else {
            System.out.println("Login failed");
          }
    } finally {
      try {     //enclose in try-catch block
        reader.close();
        //other clean-up code 
      }catch (IOException ie) {ie.getMessage();}
    }
}

  public static void main(String[] args) throws IOException {
    String path = "c:\\password.txt";
    checkPassword(path);
  }
}

Compliant Solution 2

If the need to close a stream without throwing an exception occurs often, then an alternative solution to wrapping every call of close() in its own try-catch block is to write a method, as shown in this compliant solution.

Code Block
bgColor#ccccff
import java.io.IOException;
import java.io.BufferedReader;
import java.io.FileReader;
import java.io.Closeable;

public class Login {
  static void checkPassword(String password_file) throws IOException {
  
    StringBuffer fileData = new StringBuffer(1000);
    BufferedReader reader = new BufferedReader(new FileReader(password_file));

    try {
          int n;
          char[] passwd = new char[1024];
          while ((n = reader.read(passwd)) >= 0) {
            String readData = String.valueOf(passwd, 0, n);
            fileData.append(readData);
            passwd = new char[1024];
          }
          String realPassword = "javac<at:var at:name="f3b" />b3";
          System.out.println(fileData.toString());
        
          if (fileData.toString().equals(realPassword)) {
            System.out.println("Login successful");
          }
          else {
            System.out.println("Login failed");
          }
    } finally {
          closeIgnoringException(reader);
          //other clean-up code 
    }
}

  private static void closeIgnoringException(Closeable s) {
    if (s != null) {
      try {
        s.close();
      } catch (IOException ie) {
        // Ignore exception if close fails
      }
    }
  }

  public static void main(String[] args) throws IOException {
    String path = "c:\\password.txt";
    checkPassword(path);
  }
}

Risk Assessment

Failing to handle an exception in a finally block can lead to unexpected results.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

EXC31-J

low

unlikely

medium

P2

L3

Automated Detection

TODO

Related Vulnerabilities

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

References

Wiki Markup
\[[Bloch 05|AA. Java References#Bloch 05]\] Puzzle 41: Field and Stream
\[[Harold 99|AA. Java References#Harold 99]\]


EXC30-J. Do not exit abruptly from a finally block      10. Exceptional Behavior (EXC)      11. Miscellaneous (MSC)