Return Inside Finally Block
Description
Return Inside Finally Block is a programming error where a return statement is placed inside a finally block. In Java and similar languages, a finally block is guaranteed to execute after its corresponding try block completes, whether normally or due to an exception. However, if the finally block contains a return statement, it will override any value returned from the try block and, critically, will completely discard any exception that was thrown in the try or catch blocks. The exception is silently swallowed, never reaching the caller, which leads to lost error information and incorrect program behavior.
Risk
Returning from finally blocks creates serious debugging and reliability issues. Exceptions indicating critical failures—security violations, data corruption, resource exhaustion—are silently discarded. Callers never learn that an error occurred, leading them to continue with corrupted state or invalid assumptions. Error logging and monitoring systems receive no notification of the failure. The program may produce incorrect results without any indication of the underlying problem. Code appears to succeed when it actually failed. This pattern makes bugs extremely difficult to diagnose since the original exception context is completely lost.
Solution
Never use return statements inside finally blocks. The finally block should contain only cleanup code—closing resources, releasing locks, or restoring state. If a value must be returned after cleanup, store it in a variable before the try block and return it after the finally block completes. Use try-with-resources for automatic resource management instead of manual cleanup in finally. If conditional returns are needed based on try/catch outcomes, structure the code so returns are outside the finally block.
Common Consequences
| Impact | Details |
|---|---|
| Other | Scope: Other Alter Execution Logic - Exceptions are silently discarded, changing the intended error handling flow and potentially hiding critical failures. |
| Integrity | Scope: Integrity Unexpected State - The program continues execution as if no error occurred, potentially operating on corrupted or invalid data. |
Example Code
Vulnerable Code
// Vulnerable: Return in finally discards exception
public class VulnerableReturnFinally {
// Vulnerable: Exception is silently swallowed
public int divideNumbers(int a, int b) {
try {
return a / b; // ArithmeticException if b == 0
} finally {
return -1; // This ALWAYS executes and returns -1
// The ArithmeticException is completely lost!
}
}
// Caller has no idea an error occurred
public void caller() {
int result = divideNumbers(10, 0);
// result is -1, but we don't know why
// We might think -1 is a valid computation result!
}
}
// Vulnerable: Security exception lost
public class VulnerableSecurityCheck {
public boolean authenticate(String username, String password) {
try {
if (password == null) {
throw new SecurityException("Password cannot be null");
}
return validateCredentials(username, password);
} finally {
// Vulnerable: Always returns true, hiding security exceptions!
return true;
}
}
// Attack vector: pass null password, still get authenticated!
public void exploit() {
boolean result = authenticate("admin", null);
// result is true despite SecurityException!
}
}
// Vulnerable: Resource cleanup with return
public class VulnerableResourceHandler {
public String readFile(String path) {
FileInputStream fis = null;
try {
fis = new FileInputStream(path);
// Read and process file
return processContent(fis); // May throw IOException
} catch (IOException e) {
throw new RuntimeException("File read failed", e);
} finally {
try {
if (fis != null) fis.close();
} catch (IOException e) {
// Vulnerable: Return in finally
return "Error closing file"; // Original exception lost!
}
}
}
}
// Vulnerable: Database transaction with return
public class VulnerableTransaction {
public boolean executeTransaction(String sql) {
Connection conn = null;
try {
conn = getConnection();
conn.setAutoCommit(false);
executeSQL(conn, sql); // May throw SQLException
conn.commit();
return true;
} catch (SQLException e) {
try {
if (conn != null) conn.rollback();
} catch (SQLException rollbackEx) {
// Log rollback failure
}
throw e; // Re-throw to inform caller
} finally {
try {
if (conn != null) conn.close();
} catch (SQLException closeEx) {
// Vulnerable: Discards any exception from try/catch
return false;
}
}
}
}
// Vulnerable: Complex control flow
public class VulnerableComplexFlow {
public Result processData(Data input) {
Result result = null;
try {
validate(input); // May throw ValidationException
result = transform(input); // May throw TransformException
save(result); // May throw PersistenceException
return result;
} finally {
// Vulnerable: Conditional return in finally
if (result == null) {
return Result.EMPTY; // Hides all exceptions!
}
}
}
}
// Vulnerable: Loop with return in finally
public class VulnerableLoop {
public int findValue(int[] array, int target) {
for (int i = 0; i < array.length; i++) {
try {
if (array[i] == target) {
return i; // Found it
}
if (array[i] < 0) {
throw new IllegalStateException("Negative value at " + i);
}
} finally {
// Vulnerable: Return in finally inside loop
if (i == array.length - 1) {
return -1; // Discards any exception and valid returns!
}
}
}
return -1;
}
}
Fixed Code
// Fixed: No return in finally
public class SafeReturnFinally {
// Fixed: Return only in try/catch, not in finally
public int divideNumbers(int a, int b) {
try {
return a / b;
} catch (ArithmeticException e) {
// Explicit handling of the error
throw new IllegalArgumentException("Cannot divide by zero", e);
}
// No finally needed if no cleanup required
}
// Alternative: Return default value explicitly in catch
public int divideNumbersWithDefault(int a, int b) {
try {
return a / b;
} catch (ArithmeticException e) {
// Explicit: caller knows -1 means error
return -1;
}
}
}
// Fixed: Proper exception propagation
public class SafeSecurityCheck {
public boolean authenticate(String username, String password) {
try {
if (password == null) {
throw new SecurityException("Password cannot be null");
}
return validateCredentials(username, password);
} finally {
// Only cleanup, no return
auditLog("Authentication attempt for: " + username);
}
// Exception propagates to caller
}
}
// Fixed: Resource cleanup without return in finally
public class SafeResourceHandler {
// Fixed: Use try-with-resources
public String readFile(String path) {
try (FileInputStream fis = new FileInputStream(path)) {
return processContent(fis);
} catch (IOException e) {
throw new RuntimeException("File read failed", e);
}
// Resources automatically closed, no finally needed
}
// Alternative: Manual cleanup without return
public String readFileManual(String path) {
FileInputStream fis = null;
String result;
try {
fis = new FileInputStream(path);
result = processContent(fis);
} catch (IOException e) {
throw new RuntimeException("File read failed", e);
} finally {
// Cleanup only, no return
if (fis != null) {
try {
fis.close();
} catch (IOException e) {
// Log but don't return or throw
logger.warn("Failed to close file", e);
}
}
}
return result; // Return outside finally
}
}
// Fixed: Transaction handling
public class SafeTransaction {
public boolean executeTransaction(String sql) {
Connection conn = null;
boolean success = false;
try {
conn = getConnection();
conn.setAutoCommit(false);
executeSQL(conn, sql);
conn.commit();
success = true;
} catch (SQLException e) {
if (conn != null) {
try {
conn.rollback();
} catch (SQLException rollbackEx) {
e.addSuppressed(rollbackEx);
}
}
throw new RuntimeException("Transaction failed", e);
} finally {
// Cleanup only, no return
if (conn != null) {
try {
conn.close();
} catch (SQLException closeEx) {
logger.warn("Failed to close connection", closeEx);
}
}
}
return success; // Return outside finally
}
// Better: Use try-with-resources for connection
public boolean executeTransactionModern(String sql) {
try (Connection conn = getConnection()) {
conn.setAutoCommit(false);
try {
executeSQL(conn, sql);
conn.commit();
return true;
} catch (SQLException e) {
conn.rollback();
throw e;
}
} catch (SQLException e) {
throw new RuntimeException("Transaction failed", e);
}
}
}
// Fixed: Complex control flow without return in finally
public class SafeComplexFlow {
public Result processData(Data input) {
Result result = null;
boolean success = false;
try {
validate(input);
result = transform(input);
save(result);
success = true;
} finally {
// Cleanup only
if (!success) {
cleanup();
}
}
// Handle null result outside finally
return result != null ? result : Result.EMPTY;
}
// Alternative: Let exceptions propagate naturally
public Result processDataSimple(Data input) {
validate(input); // Throws ValidationException
Result result = transform(input); // Throws TransformException
save(result); // Throws PersistenceException
return result;
// Caller handles exceptions appropriately
}
}
// Fixed: Loop without return in finally
public class SafeLoop {
public int findValue(int[] array, int target) {
for (int i = 0; i < array.length; i++) {
try {
if (array[i] == target) {
return i;
}
if (array[i] < 0) {
throw new IllegalStateException("Negative value at " + i);
}
} finally {
// Log only, no return
logger.debug("Checked index: " + i);
}
}
return -1; // Not found, returned outside try-finally
}
}
// Pattern: Store result in variable, return after finally
public class SafePattern {
public String processWithCleanup(String input) {
String result;
Resource resource = acquireResource();
try {
result = doProcess(resource, input);
} finally {
resource.release(); // Cleanup only
}
return result; // Return after finally completes
}
}
CVE Examples
No specific CVEs are commonly attributed to this CWE directly, though the pattern has contributed to silent failures in various applications.
References
- MITRE Corporation. "CWE-584: Return Inside Finally Block." https://cwe.mitre.org/data/definitions/584.html
- Java Language Specification. "The try statement."
- FindBugs. "RV: Method ignores return value (RV_RETURN_VALUE_IGNORED)."