From d41a5e3a25f7585c74551aa7e80716e9ff845cbd Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Thu, 7 Aug 2025 13:25:32 +0200 Subject: [PATCH 1/3] Java: Added basic test cases for `java/jvm-exit` --- .../CallsToSystemExit.expected | 8 ++++ .../CallsToSystemExit/CallsToSystemExit.qlref | 2 + .../CallsToSystemExit/ExampleRuntimeExit.java | 37 +++++++++++++++++++ .../CallsToSystemExit/ExampleRuntimeHalt.java | 25 +++++++++++++ .../CallsToSystemExit/ExampleSystemExit.java | 37 +++++++++++++++++++ 5 files changed, 109 insertions(+) create mode 100644 java/ql/test/query-tests/CallsToSystemExit/CallsToSystemExit.expected create mode 100644 java/ql/test/query-tests/CallsToSystemExit/CallsToSystemExit.qlref create mode 100644 java/ql/test/query-tests/CallsToSystemExit/ExampleRuntimeExit.java create mode 100644 java/ql/test/query-tests/CallsToSystemExit/ExampleRuntimeHalt.java create mode 100644 java/ql/test/query-tests/CallsToSystemExit/ExampleSystemExit.java diff --git a/java/ql/test/query-tests/CallsToSystemExit/CallsToSystemExit.expected b/java/ql/test/query-tests/CallsToSystemExit/CallsToSystemExit.expected new file mode 100644 index 000000000000..fa06d66fbc12 --- /dev/null +++ b/java/ql/test/query-tests/CallsToSystemExit/CallsToSystemExit.expected @@ -0,0 +1,8 @@ +| ExampleRuntimeExit.java:22:17:22:44 | exit(...) | Avoid calls to Runtime.exit() as this makes code harder to reuse. | +| ExampleRuntimeExit.java:25:17:25:44 | exit(...) | Avoid calls to Runtime.exit() as this makes code harder to reuse. | +| ExampleRuntimeExit.java:35:9:35:43 | exit(...) | Avoid calls to Runtime.exit() as this makes code harder to reuse. | +| ExampleRuntimeHalt.java:18:17:18:44 | halt(...) | Avoid calls to Runtime.halt() as this makes code harder to reuse. | +| ExampleRuntimeHalt.java:21:17:21:44 | halt(...) | Avoid calls to Runtime.halt() as this makes code harder to reuse. | +| ExampleSystemExit.java:22:17:22:30 | exit(...) | Avoid calls to System.exit() as this makes code harder to reuse. | +| ExampleSystemExit.java:25:17:25:30 | exit(...) | Avoid calls to System.exit() as this makes code harder to reuse. | +| ExampleSystemExit.java:35:9:35:29 | exit(...) | Avoid calls to System.exit() as this makes code harder to reuse. | diff --git a/java/ql/test/query-tests/CallsToSystemExit/CallsToSystemExit.qlref b/java/ql/test/query-tests/CallsToSystemExit/CallsToSystemExit.qlref new file mode 100644 index 000000000000..4561fcfcfd04 --- /dev/null +++ b/java/ql/test/query-tests/CallsToSystemExit/CallsToSystemExit.qlref @@ -0,0 +1,2 @@ +query: Violations of Best Practice/Undesirable Calls/CallsToSystemExit.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/java/ql/test/query-tests/CallsToSystemExit/ExampleRuntimeExit.java b/java/ql/test/query-tests/CallsToSystemExit/ExampleRuntimeExit.java new file mode 100644 index 000000000000..78603ea2bd9a --- /dev/null +++ b/java/ql/test/query-tests/CallsToSystemExit/ExampleRuntimeExit.java @@ -0,0 +1,37 @@ +import java.io.FileOutputStream; +import java.io.IOException; + +public class ExampleRuntimeExit { + + public static void main(String[] args) { + Action action = new Action(); + try { + action.run(); + } catch (Exception e) { + printUsageAndExit(e.getMessage(), 1); + } + Runtime.getRuntime().exit(0); // COMPLIANT + } + + static class Action { + public void run() { + try { + FileOutputStream fos = new FileOutputStream("output.txt"); + fos.write("Hello, World!".getBytes()); + fos.close(); + Runtime.getRuntime().exit(0); // $ Alert + } catch (IOException e) { + e.printStackTrace(); + Runtime.getRuntime().exit(1); // $ Alert + } catch (Exception e) { + // re-throw the exception + throw e; + } + } + } + + protected static void printUsageAndExit(final String message, final int exitCode) { + System.err.println("Usage: : " + message); + Runtime.getRuntime().exit(exitCode); // $ SPURIOUS: Alert + } +} diff --git a/java/ql/test/query-tests/CallsToSystemExit/ExampleRuntimeHalt.java b/java/ql/test/query-tests/CallsToSystemExit/ExampleRuntimeHalt.java new file mode 100644 index 000000000000..b1d4be04f201 --- /dev/null +++ b/java/ql/test/query-tests/CallsToSystemExit/ExampleRuntimeHalt.java @@ -0,0 +1,25 @@ +import java.io.FileOutputStream; +import java.io.IOException; + +public class ExampleRuntimeHalt { + + public static void main(String[] args) { + Action action = new Action(); + action.run(); + Runtime.getRuntime().halt(0); // COMPLIANT + } + + static class Action { + public void run() { + try { + FileOutputStream fos = new FileOutputStream("output.txt"); + fos.write("Hello, World!".getBytes()); + fos.close(); + Runtime.getRuntime().halt(0); // $ Alert + } catch (IOException e) { + e.printStackTrace(); + Runtime.getRuntime().halt(1); // $ Alert + } + } + } +} diff --git a/java/ql/test/query-tests/CallsToSystemExit/ExampleSystemExit.java b/java/ql/test/query-tests/CallsToSystemExit/ExampleSystemExit.java new file mode 100644 index 000000000000..fbb383550b66 --- /dev/null +++ b/java/ql/test/query-tests/CallsToSystemExit/ExampleSystemExit.java @@ -0,0 +1,37 @@ +import java.io.FileOutputStream; +import java.io.IOException; + +public class ExampleSystemExit { + + public static void main(String[] args) { + Action action = new Action(); + try { + action.run(); + } catch (Exception e) { + printUsageAndExit(e.getMessage(), 1); + } + System.exit(0); // COMPLIANT + } + + static class Action { + public void run() { + try { + FileOutputStream fos = new FileOutputStream("output.txt"); + fos.write("Hello, World!".getBytes()); + fos.close(); + System.exit(0); // $ Alert + } catch (IOException e) { + e.printStackTrace(); + System.exit(1); // $ Alert + } catch (Exception e) { + // re-throw the exception + throw e; + } + } + } + + protected static void printUsageAndExit(final String message, final int exitCode) { + System.err.println("Usage: : " + message); + System.exit(exitCode); // $ SPURIOUS: Alert + } +} From 4df613ce37251ef60dc3d82760366520ecf2be8a Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Thu, 7 Aug 2025 15:39:15 +0200 Subject: [PATCH 2/3] Java: Improved `java/jvm-exit` query to remove FP's. --- .../Undesirable Calls/CallsToSystemExit.ql | 78 +++++++++++++++---- .../CallsToSystemExit.expected | 14 ++-- .../CallsToSystemExit/ExampleRuntimeExit.java | 2 +- .../CallsToSystemExit/ExampleSystemExit.java | 2 +- 4 files changed, 72 insertions(+), 24 deletions(-) diff --git a/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.ql b/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.ql index 93d7911694cf..3760edf5663e 100644 --- a/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.ql +++ b/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.ql @@ -13,17 +13,67 @@ import java -from Method m, MethodCall sysexitCall, Method sysexit, Class system -where - sysexitCall = m.getACallSite(sysexit) and - (sysexit.hasName("exit") or sysexit.hasName("halt")) and - sysexit.getDeclaringType() = system and - ( - system.hasQualifiedName("java.lang", "System") or - system.hasQualifiedName("java.lang", "Runtime") - ) and - m.fromSource() and - not m instanceof MainMethod -select sysexitCall, - "Avoid calls to " + sysexit.getDeclaringType().getName() + "." + sysexit.getName() + - "() as this makes code harder to reuse." +/** + * A `Method` which, when called, causes the JVM to exit or halt. + * Explicitly includes these methods from the java standard library: + * - `java.lang.System.exit` + * - `java.lang.Runtime.halt` + * - `java.lang.Runtime.exit` + */ +class ExitOrHaltMethod extends Method { + ExitOrHaltMethod() { + exists(Class system | + this.getDeclaringType() = system and + ( + this.hasName("exit") and + ( + system.hasQualifiedName("java.lang", "System") or + system.hasQualifiedName("java.lang", "Runtime") + ) + or + this.hasName("halt") and + system.hasQualifiedName("java.lang", "Runtime") + ) + ) + } +} + +/** A `MethodCall` to an `ExitOrHaltMethod`, which causes the JVM to exit abruptly. */ +class ExitOrHaltMethodCall extends MethodCall { + ExitOrHaltMethodCall() { + exists(ExitOrHaltMethod exitMethod | this.getMethod() = exitMethod | + exists(SourceMethodNotMainOrTest srcMethod | this = srcMethod.getACallSite(exitMethod)) + ) + } +} + +/** + * Represents an intentional `MethodCall` to a system or runtime "exit" method, such as for + * functions which exist for the purpose of exiting the program. Assumes that a an exit method + * call within a method is intentional if the exit code is passed from a parameter of the + * enclosing method. + */ +class IntentionalExitMethodCall extends ExitOrHaltMethodCall { + IntentionalExitMethodCall() { + this.getMethod().hasName("exit") and + this.getAnArgument() = this.getEnclosingCallable().getAParameter().getAnAccess() + } +} + +/** + * A `Method` that is defined in source code and is not a `MainMethod` or a `LikelyTestMethod`. + */ +class SourceMethodNotMainOrTest extends Method { + SourceMethodNotMainOrTest() { + this.fromSource() and + not this instanceof MainMethod and + not this instanceof LikelyTestMethod and + not this.getEnclosingCallable() instanceof LikelyTestMethod + } +} + +from ExitOrHaltMethodCall mc +where not mc instanceof IntentionalExitMethodCall +select mc, + "Avoid calls to " + mc.getMethod().getDeclaringType().getName() + "." + mc.getMethod().getName() + + "() as this prevents runtime cleanup and makes code harder to reuse." diff --git a/java/ql/test/query-tests/CallsToSystemExit/CallsToSystemExit.expected b/java/ql/test/query-tests/CallsToSystemExit/CallsToSystemExit.expected index fa06d66fbc12..cad6d0097c7b 100644 --- a/java/ql/test/query-tests/CallsToSystemExit/CallsToSystemExit.expected +++ b/java/ql/test/query-tests/CallsToSystemExit/CallsToSystemExit.expected @@ -1,8 +1,6 @@ -| ExampleRuntimeExit.java:22:17:22:44 | exit(...) | Avoid calls to Runtime.exit() as this makes code harder to reuse. | -| ExampleRuntimeExit.java:25:17:25:44 | exit(...) | Avoid calls to Runtime.exit() as this makes code harder to reuse. | -| ExampleRuntimeExit.java:35:9:35:43 | exit(...) | Avoid calls to Runtime.exit() as this makes code harder to reuse. | -| ExampleRuntimeHalt.java:18:17:18:44 | halt(...) | Avoid calls to Runtime.halt() as this makes code harder to reuse. | -| ExampleRuntimeHalt.java:21:17:21:44 | halt(...) | Avoid calls to Runtime.halt() as this makes code harder to reuse. | -| ExampleSystemExit.java:22:17:22:30 | exit(...) | Avoid calls to System.exit() as this makes code harder to reuse. | -| ExampleSystemExit.java:25:17:25:30 | exit(...) | Avoid calls to System.exit() as this makes code harder to reuse. | -| ExampleSystemExit.java:35:9:35:29 | exit(...) | Avoid calls to System.exit() as this makes code harder to reuse. | +| ExampleRuntimeExit.java:22:17:22:44 | exit(...) | Avoid calls to Runtime.exit() as this prevents runtime cleanup and makes code harder to reuse. | +| ExampleRuntimeExit.java:25:17:25:44 | exit(...) | Avoid calls to Runtime.exit() as this prevents runtime cleanup and makes code harder to reuse. | +| ExampleRuntimeHalt.java:18:17:18:44 | halt(...) | Avoid calls to Runtime.halt() as this prevents runtime cleanup and makes code harder to reuse. | +| ExampleRuntimeHalt.java:21:17:21:44 | halt(...) | Avoid calls to Runtime.halt() as this prevents runtime cleanup and makes code harder to reuse. | +| ExampleSystemExit.java:22:17:22:30 | exit(...) | Avoid calls to System.exit() as this prevents runtime cleanup and makes code harder to reuse. | +| ExampleSystemExit.java:25:17:25:30 | exit(...) | Avoid calls to System.exit() as this prevents runtime cleanup and makes code harder to reuse. | diff --git a/java/ql/test/query-tests/CallsToSystemExit/ExampleRuntimeExit.java b/java/ql/test/query-tests/CallsToSystemExit/ExampleRuntimeExit.java index 78603ea2bd9a..13fd53b11419 100644 --- a/java/ql/test/query-tests/CallsToSystemExit/ExampleRuntimeExit.java +++ b/java/ql/test/query-tests/CallsToSystemExit/ExampleRuntimeExit.java @@ -32,6 +32,6 @@ public void run() { protected static void printUsageAndExit(final String message, final int exitCode) { System.err.println("Usage: : " + message); - Runtime.getRuntime().exit(exitCode); // $ SPURIOUS: Alert + Runtime.getRuntime().exit(exitCode); // COMPLIANT } } diff --git a/java/ql/test/query-tests/CallsToSystemExit/ExampleSystemExit.java b/java/ql/test/query-tests/CallsToSystemExit/ExampleSystemExit.java index fbb383550b66..dece6e689ba1 100644 --- a/java/ql/test/query-tests/CallsToSystemExit/ExampleSystemExit.java +++ b/java/ql/test/query-tests/CallsToSystemExit/ExampleSystemExit.java @@ -32,6 +32,6 @@ public void run() { protected static void printUsageAndExit(final String message, final int exitCode) { System.err.println("Usage: : " + message); - System.exit(exitCode); // $ SPURIOUS: Alert + System.exit(exitCode); // COMPLIANT } } From f6aad965049f7b6b6e1a9bb50e9a4692b59a6263 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Fri, 8 Aug 2025 09:54:27 +0200 Subject: [PATCH 3/3] Java: Update docs and promote to quality `java/jvm-exit` --- .../java-code-quality-extended.qls.expected | 1 + .../query-suite/not_included_in_qls.expected | 1 - .../Undesirable Calls/CallsToSystemExit.java | 29 ++++++++++++++++--- .../Undesirable Calls/CallsToSystemExit.qhelp | 27 ++++++++++------- .../Undesirable Calls/CallsToSystemExit.ql | 27 +++++++---------- 5 files changed, 53 insertions(+), 32 deletions(-) diff --git a/java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected b/java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected index 7a1a986b2aa1..8e79288d9402 100644 --- a/java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected +++ b/java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected @@ -81,6 +81,7 @@ ql/java/ql/src/Violations of Best Practice/Naming Conventions/SameNameAsSuper.ql ql/java/ql/src/Violations of Best Practice/Records/IgnoredSerializationMembersOfRecordClass.ql ql/java/ql/src/Violations of Best Practice/SpecialCharactersInLiterals/NonExplicitControlAndWhitespaceCharsInLiterals.ql ql/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToStringToString.ql +ql/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.ql ql/java/ql/src/Violations of Best Practice/Undesirable Calls/DefaultToString.ql ql/java/ql/src/Violations of Best Practice/Undesirable Calls/DoNotCallFinalize.ql ql/java/ql/src/Violations of Best Practice/Undesirable Calls/PrintLnArray.ql diff --git a/java/ql/integration-tests/java/query-suite/not_included_in_qls.expected b/java/ql/integration-tests/java/query-suite/not_included_in_qls.expected index 1f58e51ad800..a0bc27b8051a 100644 --- a/java/ql/integration-tests/java/query-suite/not_included_in_qls.expected +++ b/java/ql/integration-tests/java/query-suite/not_included_in_qls.expected @@ -187,7 +187,6 @@ ql/java/ql/src/Violations of Best Practice/Magic Constants/MagicNumbersUseConsta ql/java/ql/src/Violations of Best Practice/Magic Constants/MagicStringsUseConstant.ql ql/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingOverridesNames.ql ql/java/ql/src/Violations of Best Practice/Naming Conventions/LocalShadowsField.ql -ql/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.ql ql/java/ql/src/Violations of Best Practice/Undesirable Calls/GarbageCollection.ql ql/java/ql/src/Violations of Best Practice/legacy/AutoBoxing.ql ql/java/ql/src/Violations of Best Practice/legacy/FinallyMayNotComplete.ql diff --git a/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.java b/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.java index da7277aa25c3..50e0fb1cbda0 100644 --- a/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.java +++ b/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.java @@ -4,7 +4,7 @@ boolean write(String[] s) { try { output.write(s.getBytes()); } catch (IOException e) { - System.exit(1); + System.exit(1); // BAD: Should handle or propagate error instead of exiting } return true; } @@ -16,9 +16,30 @@ public void run() { // ... // Perform tasks ... // ... - System.exit(0); + System.exit(0); // BAD: Should return status or throw exception } public static void main(String[] args) { - new Action(args).run(); + new Action().run(); } -} \ No newline at end of file +} + +// Good example: Proper error handling +class BetterAction { + public int run() throws Exception { + // ... + // Perform tasks ... + // ... + return 0; // Return status instead of calling System.exit + } + + public static void main(String[] args) { + try { + BetterAction action = new BetterAction(); + int exitCode = action.run(); + System.exit(exitCode); // GOOD: Exit from main method + } catch (Exception e) { + System.err.println("Error: " + e.getMessage()); + System.exit(1); // GOOD: Exit from main method on error + } + } +} diff --git a/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.qhelp b/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.qhelp index e4d4fa7a7f0e..6992a734607c 100644 --- a/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.qhelp +++ b/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.qhelp @@ -13,17 +13,20 @@ program state from being written to disk consistently.

It is sometimes considered acceptable to call System.exit from a program's main method in order to indicate the overall exit status -of the program. Such calls are an exception to this rule.

+of the program. The main method should be the primary place +where exit conditions are handled, as it represents the natural termination point +of the application. Such calls are an exception to this rule.

-

It is usually preferable to use a different mechanism for reporting -failure conditions. Consider returning a special value (perhaps -null) that users of the current method check for and -recover from appropriately. Alternatively, throw a suitable exception, which -unwinds the stack and allows properly written code to clean up after itself, -while leaving other threads undisturbed.

+

Instead of calling System.exit from non-main methods, prefer to propagate +errors upward to the main method where they can be handled appropriately. +Consider returning a special value (perhaps null) that users of the current +method check for and recover from appropriately. Alternatively, throw a suitable exception, +which unwinds the stack and allows properly written code to clean up after itself, +while leaving other threads undisturbed. The main method can then catch +these exceptions and decide whether to exit the program and with what exit code.

 @@ -38,12 +41,14 @@ upwards and be handled by a method that knows how to recover.

Problem 2 is more subtle. In this example, there is just one entry point to the program (the main method), which constructs an Action and performs it. Action.run calls -System.exit to indicate successful completion. Consider, -however, how this code might be integrated in an application server that -constructs Action instances and calls +System.exit to indicate successful completion. Instead, the +run method should return a status code or throw an exception +on failure, allowing the main method to decide whether to exit +and with what exit code. Consider how this code might be integrated in an +application server that constructs Action instances and calls run on them without going through main. The fact that run terminates the JVM instead of returning its -exit code as an integer makes that use-case impossible.

+exit code makes that use-case impossible.

diff --git a/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.ql b/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.ql index 3760edf5663e..c17141122d1e 100644 --- a/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.ql +++ b/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.ql @@ -4,10 +4,11 @@ * reuse and prevent important cleanup steps from running. * @kind problem * @problem.severity warning - * @precision low + * @precision medium * @id java/jvm-exit - * @tags reliability - * maintainability + * @tags quality + * reliability + * correctness * external/cwe/cwe-382 */ @@ -22,18 +23,12 @@ import java */ class ExitOrHaltMethod extends Method { ExitOrHaltMethod() { - exists(Class system | - this.getDeclaringType() = system and - ( - this.hasName("exit") and - ( - system.hasQualifiedName("java.lang", "System") or - system.hasQualifiedName("java.lang", "Runtime") - ) - or - this.hasName("halt") and - system.hasQualifiedName("java.lang", "Runtime") - ) + exists(Class system | this.getDeclaringType() = system | + this.hasName("exit") and + system.hasQualifiedName("java.lang", ["System", "Runtime"]) + or + this.hasName("halt") and + system.hasQualifiedName("java.lang", "Runtime") ) } } @@ -48,7 +43,7 @@ class ExitOrHaltMethodCall extends MethodCall { } /** - * Represents an intentional `MethodCall` to a system or runtime "exit" method, such as for + * An intentional `MethodCall` to a system or runtime "exit" method, such as for * functions which exist for the purpose of exiting the program. Assumes that a an exit method * call within a method is intentional if the exit code is passed from a parameter of the * enclosing method.