Refactor BenchmarkTest01823 to fix cognitive complexity and security issues

[sonarqube-agent]

This PR was automatically created by the Remediation Agent's Scheduled backlog remediation feature.

Why these issues? All five issues are critical violations in a single file (BenchmarkTest01823.java) with tightly coupled solutions. The primary issue (java:S3776 cognitive complexity) is resolved through consolidating exception handlers, which simultaneously fixes the duplicated string literal issue (java:S1192). The cryptographic security issues (java:S5542, java:S5547, java:S2119) are addressed through a single algorithmic change, making this a cohesive, automatable fix.

This PR fixes 5 critical SonarQube violations in BenchmarkTest01823.java by reducing cognitive complexity, eliminating weak cryptography, and removing code duplication. The changes include moving SecureRandom instantiation to a class field, upgrading cipher from DES/CBC to AES/GCM, and consolidating duplicate exception handlers into a single multi-catch block.

View Project in SonarCloud

---

Fixed Issues

java:S3776 - Refactor this method to reduce its Cognitive Complexity from 16 to the 15 allowed. • CRITICAL • View issue

Location: src/main/java/org/owasp/benchmark/testcode/BenchmarkTest01823.java:49

Why is this an issue?

Cognitive Complexity is a measure of how hard it is to understand the control flow of a unit of code. Code with high cognitive complexity is hard to read, understand, test, and modify.

What changed

This hunk moves the SecureRandom instantiation from inside the method to a private static final field on the class. This fixes the issue where a new Random/SecureRandom object was being created each time the method was invoked, which is inefficient and may produce less random numbers. By storing and reusing the instance as a class-level field, the random number generator is properly saved and reused.

--- a/src/main/java/org/owasp/benchmark/testcode/BenchmarkTest01823.java
+++ b/src/main/java/org/owasp/benchmark/testcode/BenchmarkTest01823.java
@@ -30,0 +32,1 @@ public class BenchmarkTest01823 extends HttpServlet {
+    private static final java.security.SecureRandom random = new java.security.SecureRandom();

java:S2119 - Save and re-use this "Random". • CRITICAL • View issue 1
java:S5542 - Use another cipher mode or disable padding. • CRITICAL • View issue 2

Location: src/main/java/org/owasp/benchmark/testcode/BenchmarkTest01823.java:74

Why is this an issue?

Creating a new Random object each time a random value is needed is inefficient and may produce numbers that are not random, depending on the JDK. For better efficiency and randomness, create a single Random, store it, and reuse it.

Encryption algorithms are essential for protecting sensitive information and ensuring secure communications in a variety of domains. They are used for several important reasons:

What changed

This hunk changes the cipher algorithm from DES/CBC/PKCS5PADDING to AES/GCM/NoPadding. This fixes two issues: (1) the use of a weak cipher algorithm (DES is considered cryptographically weak and should not be used), and (2) the use of an insecure cipher mode (CBC with PKCS5 padding is vulnerable to padding oracle attacks). AES/GCM/NoPadding is a strong authenticated encryption mode that addresses both vulnerabilities.

--- a/src/main/java/org/owasp/benchmark/testcode/BenchmarkTest01823.java
+++ b/src/main/java/org/owasp/benchmark/testcode/BenchmarkTest01823.java
@@ -81,1 +72,1 @@ public class BenchmarkTest01823 extends HttpServlet {
-                            "DES/CBC/PKCS5PADDING", java.security.Security.getProvider("SunJCE"));
+                            "AES/GCM/NoPadding", java.security.Security.getProvider("SunJCE"));

java:S5547 - Use a strong cipher algorithm. • CRITICAL • View issue 1
java:S1192 - Define a constant instead of duplicating this literal "Problem executing crypto - javax.crypto.Cipher.getInstance(java.lang.String,java.security.Provider) Test Case" 6 times. • CRITICAL • View issue 2

Location: src/main/java/org/owasp/benchmark/testcode/BenchmarkTest01823.java:80

Why is this an issue?

Encryption algorithms are essential for protecting sensitive information and ensuring secure communication in various domains. They are used for several important reasons:

Duplicated string literals make the process of refactoring complex and error-prone, as any change would need to be propagated on all occurrences.

What changed

This hunk consolidates six separate catch blocks (each with its own duplicated error message string) into a single multi-catch block. This fixes the duplicated string literal issue by reducing six occurrences of the same error message string "Problem executing crypto - javax.crypto.Cipher.getInstance(java.lang.String,java.security.Provider) Test Case" to just one occurrence, eliminating the need to define a constant for it. It also reduces the cognitive complexity of the method by eliminating five additional catch/throw blocks that each contributed +1 to the complexity score, helping bring the method's cognitive complexity within the allowed threshold. Additionally, by collapsing the repeated catch blocks into one multi-catch, the duplicated response.getWriter().println(...) and e.printStackTrace(...) and throw new ServletException(e) statements are each written only once, addressing the code smell about duplicated string literals.

--- a/src/main/java/org/owasp/benchmark/testcode/BenchmarkTest01823.java
+++ b/src/main/java/org/owasp/benchmark/testcode/BenchmarkTest01823.java
@@ -127,31 +118,6 @@ public class BenchmarkTest01823 extends HttpServlet {
-        } catch (java.security.NoSuchAlgorithmException e) {
-            response.getWriter()
-                    .println(
-                            "Problem executing crypto - javax.crypto.Cipher.getInstance(java.lang.String,java.security.Provider) Test Case");
-            e.printStackTrace(response.getWriter());
-            throw new ServletException(e);
-        } catch (javax.crypto.NoSuchPaddingException e) {
-            response.getWriter()
-                    .println(
-                            "Problem executing crypto - javax.crypto.Cipher.getInstance(java.lang.String,java.security.Provider) Test Case");
-            e.printStackTrace(response.getWriter());
-            throw new ServletException(e);
-        } catch (javax.crypto.IllegalBlockSizeException e) {
-            response.getWriter()
-                    .println(
-                            "Problem executing crypto - javax.crypto.Cipher.getInstance(java.lang.String,java.security.Provider) Test Case");
-            e.printStackTrace(response.getWriter());
-            throw new ServletException(e);
-        } catch (javax.crypto.BadPaddingException e) {
-            response.getWriter()
-                    .println(
-                            "Problem executing crypto - javax.crypto.Cipher.getInstance(java.lang.String,java.security.Provider) Test Case");
-            e.printStackTrace(response.getWriter());
-            throw new ServletException(e);
-        } catch (java.security.InvalidKeyException e) {
-            response.getWriter()
-                    .println(
-                            "Problem executing crypto - javax.crypto.Cipher.getInstance(java.lang.String,java.security.Provider) Test Case");
-            e.printStackTrace(response.getWriter());
-            throw new ServletException(e);
-        } catch (java.security.InvalidAlgorithmParameterException e) {
+        } catch (java.security.NoSuchAlgorithmException
+                | javax.crypto.NoSuchPaddingException
+                | javax.crypto.IllegalBlockSizeException
+                | javax.crypto.BadPaddingException
+                | java.security.InvalidKeyException
+                | java.security.InvalidAlgorithmParameterException e) {

Have a suggestion or found an issue? Share your feedback here.

---

SonarQube Remediation Agent uses AI. Check for mistakes.

[sonarqube-agent]

⚠️ This repository does not have a CODEOWNERS file. The PR has been created but has not been automatically assigned to any reviewer. To ensure PRs are reviewed promptly, consider adding a CODEOWNERS file to your repository.