From d9bcc40dc3ded7181d218088e08adca00fc558a2 Mon Sep 17 00:00:00 2001 From: Dorian Burihabwa Date: Thu, 12 Jun 2025 16:53:00 +0200 Subject: [PATCH 1/3] JAVASE-7 Remove the NullDereferenceCheck from the plugin --- .../sonar/java/se/plugin/JavaSECheckList.java | 1 - .../org/sonar/java/se/plugin/RulesList.java | 1 - .../sonar/l10n/java/rules/javase/S2259.html | 144 ------------------ .../sonar/l10n/java/rules/javase/S2259.json | 34 ----- .../java/rules/javase/Sonar_way_profile.json | 1 - .../java/se/plugin/JavaSECheckListTest.java | 2 +- .../se/plugin/JavaSECheckRegistrarTest.java | 2 +- .../se/plugin/JavaSEProfileRegistrarTest.java | 2 +- 8 files changed, 3 insertions(+), 184 deletions(-) delete mode 100644 java-symbolic-execution/java-symbolic-execution-plugin/src/main/resources/org/sonar/l10n/java/rules/javase/S2259.html delete mode 100644 java-symbolic-execution/java-symbolic-execution-plugin/src/main/resources/org/sonar/l10n/java/rules/javase/S2259.json diff --git a/java-symbolic-execution/java-symbolic-execution-plugin/src/main/java/org/sonar/java/se/plugin/JavaSECheckList.java b/java-symbolic-execution/java-symbolic-execution-plugin/src/main/java/org/sonar/java/se/plugin/JavaSECheckList.java index 165fad9c6..284ee1c9d 100644 --- a/java-symbolic-execution/java-symbolic-execution-plugin/src/main/java/org/sonar/java/se/plugin/JavaSECheckList.java +++ b/java-symbolic-execution/java-symbolic-execution-plugin/src/main/java/org/sonar/java/se/plugin/JavaSECheckList.java @@ -51,7 +51,6 @@ private JavaSECheckList(){ public static List> getChecks() { return List.of( // SEChecks ordered by ExplodedGraphWalker need - NullDereferenceCheck.class, DivisionByZeroCheck.class, UnclosedResourcesCheck.class, LocksNotUnlockedCheck.class, diff --git a/java-symbolic-execution/java-symbolic-execution-plugin/src/main/java/org/sonar/java/se/plugin/RulesList.java b/java-symbolic-execution/java-symbolic-execution-plugin/src/main/java/org/sonar/java/se/plugin/RulesList.java index 26608a629..7ee8f3541 100644 --- a/java-symbolic-execution/java-symbolic-execution-plugin/src/main/java/org/sonar/java/se/plugin/RulesList.java +++ b/java-symbolic-execution/java-symbolic-execution-plugin/src/main/java/org/sonar/java/se/plugin/RulesList.java @@ -29,7 +29,6 @@ public static List getSonarWayRuleKeys() { "S2095", "S2189", "S2222", - "S2259", "S2583", "S2589", "S2637", diff --git a/java-symbolic-execution/java-symbolic-execution-plugin/src/main/resources/org/sonar/l10n/java/rules/javase/S2259.html b/java-symbolic-execution/java-symbolic-execution-plugin/src/main/resources/org/sonar/l10n/java/rules/javase/S2259.html deleted file mode 100644 index 25a93c7f4..000000000 --- a/java-symbolic-execution/java-symbolic-execution-plugin/src/main/resources/org/sonar/l10n/java/rules/javase/S2259.html +++ /dev/null @@ -1,144 +0,0 @@ -

Why is this an issue?

-

A reference to null should never be dereferenced/accessed. Doing so will cause a NullPointerException to be thrown. At -best, such an exception will cause abrupt program termination. At worst, it could expose debugging information that would be useful to an attacker, or -it could allow an attacker to bypass security measures.

-

Note that when they are present, this rule takes advantage of nullability annotations, like @CheckForNull or @Nonnull, -defined in JSR-305 to understand which values can be null or not. @Nonnull will be -ignored if used on the parameter of the equals method, which by contract should always work with null.

-

How to fix it

-

Code examples

-

Noncompliant code example

-

The variable myObject is equal to null, meaning it has no value:

-
-public void method() {
-  Object myObject = null;
-  System.out.println(myObject.toString()); // Noncompliant: myObject is null
-}
-
-

The parameter input might be null as suggested by the if condition:

-
-public void method(Object input)
-{
-  if (input == null)
-  {
-    // ...
-  }
-  System.out.println(input.toString()); // Noncompliant
-}
-
-

The unboxing triggered in the return statement will throw a NullPointerException:

-
-public boolean method() {
-  Boolean boxed = null;
-  return boxed; // Noncompliant
-}
-
-

Both conn and stmt might be null in case an exception was thrown in the try{} block:

-
-Connection conn = null;
-Statement stmt = null;
-try {
-  conn = DriverManager.getConnection(DB_URL,USER,PASS);
-  stmt = conn.createStatement();
-  // ...
-} catch(Exception e) {
-  e.printStackTrace();
-} finally {
-  stmt.close();  // Noncompliant
-  conn.close();  // Noncompliant
-}
-
-

As getName() is annotated with @CheckForNull, there is a risk of NullPointerException here:

-
-@CheckForNull
-String getName() {...}
-
-public boolean isNameEmpty() {
-  return getName().length() == 0; // Noncompliant
-}
-
-

As merge(…​) parameter is annotated with @Nonnull, passing an identified potential null value (thanks to @CheckForNull) -is not safe:

-
-private void merge(@Nonnull Color firstColor, @Nonnull Color secondColor) {...}
-
-public void append(@CheckForNull Color color) {
-  merge(currentColor, color);  // Noncompliant: color should be null-checked because merge(...) doesn't accept nullable parameters
-}
-
-

Compliant solution

-

Ensuring the variable myObject has a value resolves the issue:

-
-public void method() {
-  Object myObject = new Object();
-  System.out.println(myObject.toString()); // Compliant: myObject is not null
-}
-
-

Preventing the non-compliant code to be executed by returning early:

-
-public void method(Object input)
-{
-  if (input == null)
-  {
-    return;
-  }
-  System.out.println(input.toString()); // Compliant: if 'input' is null, this is unreachable
-}
-
-

Ensuring that no unboxing of null value can happen resolves the issue

-
-public boolean method() {
-  Boolean boxed = true;
-  return boxed; // Compliant
-}
-
-

Ensuring that both conn and stmt are not null resolves the issue:

-
-Connection conn = null;
-Statement stmt = null;
-try {
-  conn = DriverManager.getConnection(DB_URL,USER,PASS);
-  stmt = conn.createStatement();
-  // ...
-} catch(Exception e) {
-  e.printStackTrace();
-} finally {
-  if (stmt != null) {
-    stmt.close();  // Compliant
-  }
-  if (conn != null) {
-    conn.close();  // Compliant
-  }
-}
-
-

Checking the returned value of getName() resolves the issue:

-
-@CheckForNull
-String getName() {...}
-
-public boolean isNameEmpty() {
-  String name = getName();
-  if (name != null) {
-    return name.length() == 0; // Compliant
-  } else {
-    // ...
-  }
-}
-
-

Ensuring that the provided color is not null resolves the issue:

-
-private void merge(@Nonnull Color firstColor, @Nonnull Color secondColor) {...}
-
-public void append(@CheckForNull Color color) {
-  if (color != null) {
-    merge(currentColor, color);  // Compliant
-  }
-}
-
-

Resources

- - diff --git a/java-symbolic-execution/java-symbolic-execution-plugin/src/main/resources/org/sonar/l10n/java/rules/javase/S2259.json b/java-symbolic-execution/java-symbolic-execution-plugin/src/main/resources/org/sonar/l10n/java/rules/javase/S2259.json deleted file mode 100644 index 996c08831..000000000 --- a/java-symbolic-execution/java-symbolic-execution-plugin/src/main/resources/org/sonar/l10n/java/rules/javase/S2259.json +++ /dev/null @@ -1,34 +0,0 @@ -{ - "title": "Null pointers should not be dereferenced", - "type": "BUG", - "code": { - "impacts": { - "RELIABILITY": "MEDIUM" - }, - "attribute": "LOGICAL" - }, - "status": "ready", - "remediation": { - "func": "Constant\/Issue", - "constantCost": "10min" - }, - "tags": [ - "cwe", - "cert", - "symbolic-execution" - ], - "defaultSeverity": "Major", - "ruleSpecification": "RSPEC-2259", - "sqKey": "S2259", - "scope": "Main", - "securityStandards": { - "CERT": [ - "EXP34-C.", - "EXP01-J." - ], - "CWE": [ - 476 - ] - }, - "quickfix": "unknown" -} diff --git a/java-symbolic-execution/java-symbolic-execution-plugin/src/main/resources/org/sonar/l10n/java/rules/javase/Sonar_way_profile.json b/java-symbolic-execution/java-symbolic-execution-plugin/src/main/resources/org/sonar/l10n/java/rules/javase/Sonar_way_profile.json index bf7388964..6c131982b 100644 --- a/java-symbolic-execution/java-symbolic-execution-plugin/src/main/resources/org/sonar/l10n/java/rules/javase/Sonar_way_profile.json +++ b/java-symbolic-execution/java-symbolic-execution-plugin/src/main/resources/org/sonar/l10n/java/rules/javase/Sonar_way_profile.json @@ -4,7 +4,6 @@ "S2095", "S2189", "S2222", - "S2259", "S2583", "S2589", "S2637", diff --git a/java-symbolic-execution/java-symbolic-execution-plugin/src/test/java/org/sonar/java/se/plugin/JavaSECheckListTest.java b/java-symbolic-execution/java-symbolic-execution-plugin/src/test/java/org/sonar/java/se/plugin/JavaSECheckListTest.java index 0c9cdc40b..7175571bf 100644 --- a/java-symbolic-execution/java-symbolic-execution-plugin/src/test/java/org/sonar/java/se/plugin/JavaSECheckListTest.java +++ b/java-symbolic-execution/java-symbolic-execution-plugin/src/test/java/org/sonar/java/se/plugin/JavaSECheckListTest.java @@ -24,7 +24,7 @@ class JavaSECheckListTest { @Test void getChecks() { - assertThat(JavaSECheckList.getChecks()).isNotNull().hasSize(23); + assertThat(JavaSECheckList.getChecks()).isNotNull().hasSize(22); } } diff --git a/java-symbolic-execution/java-symbolic-execution-plugin/src/test/java/org/sonar/java/se/plugin/JavaSECheckRegistrarTest.java b/java-symbolic-execution/java-symbolic-execution-plugin/src/test/java/org/sonar/java/se/plugin/JavaSECheckRegistrarTest.java index 725c1bca0..29be9e8ae 100644 --- a/java-symbolic-execution/java-symbolic-execution-plugin/src/test/java/org/sonar/java/se/plugin/JavaSECheckRegistrarTest.java +++ b/java-symbolic-execution/java-symbolic-execution-plugin/src/test/java/org/sonar/java/se/plugin/JavaSECheckRegistrarTest.java @@ -85,7 +85,7 @@ void rules_definition() { assertThat(repository.name()).isEqualTo("Sonar"); assertThat(repository.language()).isEqualTo("java"); List rules = repository.rules(); - assertThat(rules).hasSize(23); + assertThat(rules).hasSize(22); var activeByDefault = rules.stream() .filter(k -> !rulesNotActiveByDefault.contains(k.key())) diff --git a/java-symbolic-execution/java-symbolic-execution-plugin/src/test/java/org/sonar/java/se/plugin/JavaSEProfileRegistrarTest.java b/java-symbolic-execution/java-symbolic-execution-plugin/src/test/java/org/sonar/java/se/plugin/JavaSEProfileRegistrarTest.java index 4bcc00e54..6b27d0bd0 100644 --- a/java-symbolic-execution/java-symbolic-execution-plugin/src/test/java/org/sonar/java/se/plugin/JavaSEProfileRegistrarTest.java +++ b/java-symbolic-execution/java-symbolic-execution-plugin/src/test/java/org/sonar/java/se/plugin/JavaSEProfileRegistrarTest.java @@ -28,7 +28,7 @@ void constructor() { JavaSEProfileRegistrar registrar = new JavaSEProfileRegistrar(); TestProfileRegistrarContext context = new TestProfileRegistrarContext(); registrar.register(context); - assertThat(context.defaultQualityProfileRules).hasSize(21); // 2 are not in the default profile + assertThat(context.defaultQualityProfileRules).hasSize(20); // 2 are not in the default profile } } From dce3bbd31a3a1c13cd4be1a1b80a92ed052c46e6 Mon Sep 17 00:00:00 2001 From: Dorian Burihabwa Date: Thu, 12 Jun 2025 17:06:25 +0200 Subject: [PATCH 2/3] JAVASE-7 Remove S2259 from ITs --- its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java b/its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java index 623565518..3eb8aeddb 100644 --- a/its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java +++ b/its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java @@ -61,7 +61,7 @@ public class JavaRulingTest { private static final Logger LOG = LoggerFactory.getLogger(JavaRulingTest.class); private static final ImmutableSet SUBSET_OF_ENABLED_RULES = ImmutableSet.of( - "S2095", "S2189", "S2222", "S2259", "S2583", "S2589", "S2637", "S2689", "S2755", "S3065", + "S2095", "S2189", "S2222", "S2583", "S2589", "S2637", "S2689", "S2755", "S3065", "S3516", "S3518", "S3546", "S3655", "S3824", "S3958", "S3959", "S4165", "S4449", "S6373", "S6374", "S6376", "S6377"); @ClassRule From 49936f25b4cb0ef7438ff6970804070d649f1bba Mon Sep 17 00:00:00 2001 From: Dorian Burihabwa Date: Thu, 12 Jun 2025 17:15:53 +0200 Subject: [PATCH 3/3] JAVASE-7 Fix ITs again --- its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java | 2 +- .../src/main/java/org/sonar/java/se/plugin/JavaSECheckList.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java b/its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java index 3eb8aeddb..7d0828f01 100644 --- a/its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java +++ b/its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java @@ -96,7 +96,7 @@ public static void prepare() throws Exception { List extraNonDefaultRules = List.of("S3546", "S6374"); ProfileGenerator.generate(ORCHESTRATOR, "Sonar Way", ImmutableMap.of(), new HashSet<>(), SUBSET_OF_ENABLED_RULES, result, extraNonDefaultRules); - assertThat(result).hasSize(23); // ALL symbolic-execution rules + assertThat(result).hasSize(22); // ALL symbolic-execution rules Path allRulesFolder = Paths.get("src/test/resources"); effectiveDumpOldFolder = tmpDumpOldFolder.getRoot().toPath().toAbsolutePath(); diff --git a/java-symbolic-execution/java-symbolic-execution-plugin/src/main/java/org/sonar/java/se/plugin/JavaSECheckList.java b/java-symbolic-execution/java-symbolic-execution-plugin/src/main/java/org/sonar/java/se/plugin/JavaSECheckList.java index 284ee1c9d..f79c7b06e 100644 --- a/java-symbolic-execution/java-symbolic-execution-plugin/src/main/java/org/sonar/java/se/plugin/JavaSECheckList.java +++ b/java-symbolic-execution/java-symbolic-execution-plugin/src/main/java/org/sonar/java/se/plugin/JavaSECheckList.java @@ -29,7 +29,6 @@ import org.sonar.java.se.checks.MinMaxRangeCheck; import org.sonar.java.se.checks.NoWayOutLoopCheck; import org.sonar.java.se.checks.NonNullSetToNullCheck; -import org.sonar.java.se.checks.NullDereferenceCheck; import org.sonar.java.se.checks.ObjectOutputStreamCheck; import org.sonar.java.se.checks.OptionalGetBeforeIsPresentCheck; import org.sonar.java.se.checks.ParameterNullnessCheck;