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 | Closeable Not Closed (Java) |
Coverity | 7.5 | ITERATOR | Implemented |
Parasoft Jtest | 2024.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 | S2095 | Implemented |
Related Guidelines
CWE-404, Improper Resource Shutdown or Release |
Android Implementation Details
The compliant solution (try
-with-resources) is not yet supported at API level 18 (Android 4.3).
Bibliography
[API 2014] | |
The |
25 Comments
David Svoboda
Thomas Hawtin
It is possible to write the compliant code more concisely.
Siddarth Adukia
Grazie, I see your point.
David Svoboda
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.)
Siddarth Adukia
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.
Thomas Hawtin
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.
Siddarth Adukia
I just thought of a more concise way to write compliant example one:
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.
Thomas Hawtin
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:
Robert Seacord (Manager)
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.
Thomas Hawtin
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.
David Svoboda
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.
Thomas Hawtin
What I mean by 'operates the opposite way'.
Consider this code.
It is not equivalent to:
Or:
And this is illegal:
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).
David Svoboda
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.)
Masaki Kubo
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.
David Svoboda
1. Agreed, changed.
2. Dunno, changed to 'File'.
Unni Vemanchery Mana
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:
David Svoboda
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:
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.
Adam
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
David Svoboda
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.
Adam
Maybe, I'm wrong, but I rather thought of STR02-J, STR04-J and general rule "When translating between char sequences and byte sequences, you can and usually should specify a charset explicitly" - as the compliant example here, i.e.
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
prashant
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.
G. Ann Campbell
@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
).prashant
Thanks G. Ann Campbell....
Abhishek Pandey
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.
David Svoboda
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.