The Java garbage collector is called to free unreferenced but as-yet unreleased memory. However, the garbage collector cannot free nonmemory resources such as open file descriptors and database connections. Consequently, failing to release such resources can lead to resource exhaustion attacks. In addition, programs can experience resource starvation while waiting for a finalizer to release resources such as Lock or Semaphore objects. This can occur because Java lacks any temporal guarantee of when finalizers execute other than "sometime before program termination." Finally, output streams may cache object references; such cached objects are not garbage-collected until after the output stream is closed. Consequently, output streams should be closed promptly after use.

A program may leak resources when it relies on finalizers to release system resources or when there is confusion over which part of the program is responsible for releasing system resources. In a busy system, the delay before the finalizer is called for an object provides a window of vulnerability during which an attacker could induce a denial-of-service (DoS) attack. Consequently, resources other than raw memory must be explicitly freed in nonfinalizer methods because of the unsuitability of using finalizers. See MET12-J. Do not use finalizers for additional reasons to avoid the use of finalizers.

Note that on Windows systems, attempts to delete open files fail silently (see FIO03-J. Remove temporary files before termination for more information).

Noncompliant Code Example (File Handle)

This noncompliant code example opens a file and uses it but fails to explicitly close the file:

public int processFile(String fileName)
                       throws IOException, FileNotFoundException {
  FileInputStream stream = new FileInputStream(fileName);
  BufferedReader bufRead =
      new BufferedReader(new InputStreamReader(stream));
  String line;
  while ((line = bufRead.readLine()) != null) {
    sendLine(line);
  }
  return 1;
}

Compliant Solution

This compliant solution releases all acquired resources, regardless of any exceptions that might occur. Even though dereferencing bufRead might result in an exception, the FileInputStream object is closed as required.

try {
  final FileInputStream stream = new FileInputStream(fileName);
  try {
    final BufferedReader bufRead =
        new BufferedReader(new InputStreamReader(stream));

    String line;
    while ((line = bufRead.readLine()) != null) {
      sendLine(line);
    }
  } finally {
    if (stream != null) {
      try {
        stream.close();
      } catch (IOException e) {
        // Forward to handler
      }
    }
  }
} catch (IOException e) {
  // Forward to handler
}

Compliant Solution (try-with-resources)

This compliant solution uses the try-with-resources statement, introduced in Java SE 7, to release all acquired resources regardless of any exceptions that might occur:

try (FileInputStream stream = new FileInputStream(fileName);
     BufferedReader bufRead =
         new BufferedReader(new InputStreamReader(stream))) {

  String line;
  while ((line = bufRead.readLine()) != null) {
    sendLine(line);
  }
} catch (IOException e) {
  // Forward to handler
}

The try-with-resources construct sends any IOException to the catch clause, where it is forwarded to an exception handler. Exceptions generated during the allocation of resources (that is, the creation of the FileInputStream or BufferedReader), as well as any IOException thrown during execution of the while loop and any IOException generated by closing bufRead or stream, are included.

Noncompliant Code Example (SQL Connection)

The problem of resource pool exhaustion is exacerbated in the case of database connections. Many database servers allow only a fixed number of connections, depending on configuration and licensing. Consequently, failure to release database connections can result in rapid exhaustion of available connections. This noncompliant code example fails to close the connection when an error occurs during execution of the SQL statement or during processing of the results:

public void getResults(String sqlQuery) {
  try {
    Connection conn = getConnection();
    Statement stmt = conn.createStatement();
    ResultSet rs = stmt.executeQuery(sqlQuery);
    processResults(rs);
    stmt.close(); conn.close();
  } catch (SQLException e) { /* Forward to handler */ }
}

Noncompliant Code Example

This noncompliant code example attempts to address exhaustion of database connections by adding cleanup code in a finally block. However, rs, stmt, or conn could be null, causing the code in the finally block to throw a NullPointerException.

Statement stmt = null;
ResultSet rs = null;
Connection conn = getConnection();
try {
  stmt = conn.createStatement();
  rs = stmt.executeQuery(sqlQuery);
  processResults(rs);
} catch(SQLException e) {
  // Forward to handler
} finally {
  rs.close();
  stmt.close(); conn.close();
}

Noncompliant Code Example

In this noncompliant code example, the call to rs.close() or the call to stmt.close() might throw a SQLException. Consequently, conn.close() is never called, which violates ERR05-J. Do not let checked exceptions escape from a finally block.

Statement stmt = null;
ResultSet rs = null;
Connection conn = getConnection();
try {
  stmt = conn.createStatement();
  rs = stmt.executeQuery(sqlQuery);
  processResults(rs);
} catch (SQLException e) {
  // Forward to handler
} finally {
  if (rs != null) {
    rs.close();
  }
  if (stmt != null) {
    stmt.close();
  } if (conn !=null) {
       conn.close();
    }
}

Compliant Solution

This compliant solution ensures that resources are released as required:

Statement stmt = null;
ResultSet rs = null;
Connection conn = getConnection();
try {
  stmt = conn.createStatement();
  rs = stmt.executeQuery(sqlQuery);
  processResults(rs);
} catch (SQLException e) {
  // Forward to handler
} finally {
  try {
    if (rs != null) {rs.close();}
  } catch (SQLException e) {
    // Forward to handler
  } finally {
    try {
      if (stmt != null) {stmt.close();}
    } catch (SQLException e) {
      // Forward to handler
    } finally {
      try {
        if (conn != null) {conn.close();}
      } catch (SQLException e) {
        // Forward to handler
      }
    }
  }
}

Compliant Solution (try-with-resources)

This compliant solution uses the try-with-resources construct, introduced in Java SE 7, to ensure that resources are released as required:

try (Connection conn = getConnection();
     Statement stmt = conn.createStatement();
     ResultSet rs = stmt.executeQuery(sqlQuery)) {
  processResults(rs);
} catch (SQLException e) {
  // Forward to handler
}

The try-with-resources construct sends any SQLException to the catch clause, where it is forwarded to an exception handler. Exceptions generated during the allocation of resources (that is, the creation of the Connection, Statement, or ResultSet), as well as any SQLException thrown by processResults() and any SQLException generated by closing rs, stmt, or conn are included.

Risk Assessment

Failure to explicitly release nonmemory system resources when they are no longer needed can result in resource exhaustion.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

FIO04-J

Low

Probable

Medium

P4

L3

Automated Detection

Although sound automated detection of this vulnerability is not feasible in the general case, many interesting cases can be soundly detected.

Some static analysis tools can detect cases in which there is leak of a socket resource or leak of a stream representing a file or other system resources.

Tool
Version
Checker
Description
CodeSonar
8.1p0

JAVA.ALLOC.LEAK.NOTCLOSED
JAVA.ALLOC.LEAK.NOTSTORED

Closeable Not Closed (Java)
Closeable Not Stored (Java)

Coverity7.5

ITERATOR
JDBC_CONNECTION
RESOURCE_LEAK

Implemented
Parasoft Jtest
2023.1
CERT.FIO04.LEAKS
CERT.FIO04.CIO
CERT.FIO04.CCR
Ensure resources are deallocated
Close input and output resources in "finally" blocks
Close all "java.io.Closeable" resources in a "finally" block
SonarQube
9.9
S2095Implemented

Related Guidelines

SEI CERT C Coding Standard

FIO22-C. Close files before spawning processes

SEI CERT C++ Coding Standard

FIO51-CPP. Close files when they are no longer needed

MITRE CWE

CWE-404, Improper Resource Shutdown or Release
CWE-405, Asymmetric Resource Consumption (Amplification)
CWE-459, Incomplete Cleanup
CWE-770, Allocation of Resources without Limits or Throttling

Android Implementation Details

The compliant solution (try-with-resources) is not yet supported at API level 18 (Android 4.3).

Bibliography

[API 2014]

Class Object

[Goetz 2006b]


[J2SE 2011]

The try-with-resources Statement



25 Comments

    • Each code sample should go under a Non-Compliant Code Example section, or a Compliant Solution section. Don't group multiple bad code samples in one section.
    • Your 3rd code section, which closes resources in a 'finally' clause is not valid Java, because the fianlly clause references variables declared in the try clause. You need to declare them outside the try clause for the finally clause to recognize them.
    • Provide a compliant solution for closing the file, which 'fixes' the code in the first noncompliant code.
  1. It is possible to write the compliant code more concisely.

    FileInputStream rawIn = new FileInputStream(fileName);;
    try {
      BufferedReader in = new BufferedReader(rawIn);
      for (;;) {
        String line = in.readLine(); // Note, unlimited size.
        if (line == null) {
          break;
        }
        sendLine(line);
      }
    } finally {
      rawIn.close();
    }
    1. Grazie, I see your point.

  2. I suspect the severity might be medium, in the case that malicious code could access an open file descriptor or database connection and have access to privileged information.

    (This has nothing to do with the grade or assignment, as the assignment is done.)

    1.  In that case, it would a be totally different vulnerability then the one described above. I'm unclear on how an attacker could hijack said unreleased resources - by reflection, perhaps? I'll see if I can get a concrete example of this, do you know of something I can use?

       I would argue that this is more of a DoS attack, since it is highly probable that an attacker can forge multiple calls to a specific function, especially in the case of web based applications, leading to rapid exhaustion of non-memory resources.

       I guess one scenario could be if an attacker manages to exhaust say database connections to the user database, forcing the app to fall back on less secure authentication, maybe an insecure default password file or something. Though that scenario sounds a little more feasible, I'm not sure if it ranks a mention on the page.

      1. It's a DoS problem. In particular it's a problem if the resource can be leaked without allocating significant (Java heap) memory. If the GC doesn't need to run, the finalizers wont be found and eventually will run out. With resource acquisition failing legitimate clients will stop working and therefore probably cause significatly slower allocation of memory, delaying clearing of the problem.

  3. I just thought of a more concise way to write compliant example one:

    try {
        stmt = conn.createStatement();
        rs = stmt.executeQuery(sqlQuery);
        processResults(rs);
    } catch(SQLException e) { }
      finally {
         try {
            rs.close();
         } finally (
            try {
                stmt.close();
            }
            finally {
               conn.close();
            }
          }
    }
    

    In this case, even if rs/stmt are null and an exception is thrown, the rest of the commands are still called. Though it strikes me as bad programming to be so dismissive of generating exceptions easily avoided.

    1. Of course this is very artificial. Real code doesn't go through this for every SQL query. If you did, then I think you'd write a method to encapsulate the sequence.

      In any case, I would write it as:

      Connection connection = getConnection();
      try {
          Statement statement = connection.createStatement();
          try {
              ResultSet results = statement.executeQuery(query);
              try {
                  processResults(rs);
              }  finally {
                  results.close();
              }
          } finally (
              statement.close();
          }
      } finally {
          connection.close();
      }
  4. The intro to this needs to be much clearer in describing what exactly this guideline requires. I think it is confused with specific examples, the descriptions of which should be included in the corresponding noncompliant code examples.

  5. The JDK7 solution is a bit different. It catches exceptions from resource release. "try-with-resource" with catch operates the opposite way around to try with catch with finally. It's unfortunate that the three uses of the try keyword can be mangled into the same structure. A try block with both catch and finally probably has something wrong with it.

    1. I've amended the text for both Java 7 compliant solutions to indicate that the catch clause catches everything, as you suggest. (Also fleshed out other compliant solutions, which weren't fully correct).

      I'm not sure what you mean by 'operates the opposite way', unless you are talking about order of execution. I always assumed order of execution is the same as order of appearance, except that any control-flow statement (return, throw, break, etc) doesn't get executed until all catch and finally clauses are done. If my assumption is wrong, then we probably have fodder for a rule that explains what execution order really is.

      1. What I mean by 'operates the opposite way'.

        Consider this code.

        try (InputStream in = new FileInputStream(file)) {
            use(in);
        } catch (IOException exc) {
            throw wrap(exc);
        }
        

        It is not equivalent to:

        InputStream in = null;
        try (
            in = new FileInputStream(file);
            use(in);
        } catch (IOException exc) {
            throw wrap(exc);
        } finally {
            if (in !?= null) in.close();
        }
        

        Or:

        final InputStream in = new FileInputStream(file);
        try (
            use(in);
        } catch (IOException exc) {
            throw wrap(exc);
        } finally {
            in.close();
        }
        

        And this is illegal:

        InputStream in = null;
        try (
            in = new FileInputStream(file);
            use(in);
        } finally {
            if (in !?= null) in.close();
        } catch (IOException exc) {
            throw wrap(exc);
        }
        

        For (rough) equivalence, you need two try blocks (which would be the preferred way of writing it in Java SE 6, although usually the exception handling would be done in a different method).

        try {
            final InputStream in = new FileInputStream(file);
            try {
                use(in);
            } finally {
                in.close();
            }
        } catch (IOException exc) {
            throw wrap(exc);
        }
        
        1. OK, I see that your 5th code example is the cloesst analogue to how try-with-resources actually works. I modified one of our CS's to use that pattern (embedded try blocks).

          AFAICT there is no functional difference between your 5th code example and your 2nd. The 2nd example is equally correct; it is just a bit bulkier (as the variables are set to null outside the try block). Is there any other reason the 5th code samle is preferred over than the 2nd?

          (BTW all your code samples violate ERR05-J. Do not let checked exceptions escape from a finally block, because the close() can throw, but that's an easy fix.)

  6. Two comments:
    1. In the title, "Release resource..." would be more generic than "Close resource..."
    2. First NCCE: Why "File Handle"? It doesn't look like a Windows specific code example.

    1. 1. Agreed, changed.
      2. Dunno, changed to 'File'.

  7. David,

    In this test case,  I would like to mention one more point.This is related to URLClassLoader. Java 7 has introduced a new method , close()  to close the loader as soon as its operation is over. So this operation closes all opened files and makes eligible for garbage collection. Sample code snippet:

    URL url = new URL("file:test.jar");
           URLClassLoader loader = new URLClassLoader (new URL[] {url});
           Class clazz = Class.forName ("Test", true, loader);
           Runnable test = (Runnable) clazz.newInstance();
           test.runSomeMethod();
    
           // preferred way is , close the loader once operation is over
           loader.close ();


    1. Unni:

      Interesting. I see that URLClassLoader (unlike its parents) implements the Closable interface. This interface requires the close() method, and must be implemented by any class to be used in a try-with-resources clause.

      The URLClassLoader.close() method says:

      In the case of jar: and file: URLs, it also closes any files that were opened by it. If another thread is loading a class when the close method is invoked, then the result of that load is undefined.

      I take this to mean that it closes just the file or socket indicated by the URL, and not any files opened by the class itself.

  8. Hi,

    Just a remark: Shouldn't InputStreamReader(stream) have Charset explicitly specified to be compliant with other Secure Coding rules for proper encoding?
    I mean: InputStreamReader(fis, StandardCharsets.UTF_8)

    Adam

    1. I suspect you are addressing rule STR02-J. Specify an appropriate locale when comparing locale-dependent data rather than this one. Whether to specify an explicit encoding depends on the source of the file being read...see that rule for details. Any further comments should be sent to the Comments section in that rule.

      1.     final BufferedReader bufRead =
                new BufferedReader(new InputStreamReader(stream));

        ...

            String line;
            while ((line = bufRead.readLine()) != null) {

        yields eventually new String from raw bytes (InputStream) and it can be seen a specific variation of STR04-J and STR02-J (default charset issue).

        This way "Compliant Solution" from FIO04-J promotes inadvertently (as side effect) other poor practice pointed out in STR02-J and STR04-J - omitting specifying an explicit charset while translating bytes into chars/Strings.

        Compliant Solution from STR04-J says:
        This compliant solution explicitly specifies the character encoding used to create the string (in this example, UTF-16LE) as the second argument to the String constructor."

        Perhaps, we should apply the same recommendation to InputStreamReader, shouldn't we? Just food for thought.

        Adam

  9. Statement stmt = null;
    ResultSet rs = null;
    Connection conn = getConnection();
    try {
      stmt = conn.createStatement();
      rs = stmt.executeQuery(sqlQuery);
      processResults(rs);
    } catch (SQLException e) {
      // Forward to handler
    } finally {
      try {
        if (rs != null) {rs.close();}
      } catch (SQLException e) {
        // Forward to handler
      }
        try {
          if (stmt != null) {stmt.close();}
        } catch (SQLException e) {
          // Forward to handler
        }
      }
    My coding is as above, still I am getting sonar issue as close the statement. Please advice as I am closing all the objects.
    1. @prashant this is not the appropriate forum for that report. If you truly feel this is a false positive, you should report it at the SonarQube Google Group (altho I don't see where you're closing conn).

  10. Hi,

    Below is my code where I have used StringBuilder with Size parameter and same in BufferedReader.

    But still, Fortify-SCA says its a DOS issue on readline().

    Is there any way to solve this, please help. 


    try (
    InputStream in = new FileInputStream(path);
    InputStreamReader inputStreamReader = new InputStreamReader(in);
    BufferedReader responseStreamReader = new BufferedReader(inputStreamReader,4096);
    )
    {
    String line = "";
    StringBuilder stringBuilder = new StringBuilder(4096);
    while ((line = responseStreamReader.readLine()) != null) {
    stringBuilder.append(line);
    }
    jsonData = stringBuilder.toString();
    }
    catch(IOException e){
    e.printStackTrace();
    }



    1. Technically, this is not a resource exhaustion problem, so this is an off-topic question. But the specific answer is that Fortify is right. The readline() function can exhaust memory if the file has lots of characters but no newline (iow an arbitrarily long line). So to avoid  a DOS issue, you must not use readLine, but instead use a function that limits the number of bytes read.