SONARJAVA-6528 S2068, S6418, and S6437: Make use of common secret exclusion filter #5745
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| { | ||
| "ruleKey": "S6437", | ||
| "hasTruePositives": true, | ||
| "falseNegatives": 62, | ||
| "falsePositives": 0 | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,5 @@ | ||
| { | ||
| "org.regex-examples:regex-examples:src/main/java/org/regex/examples/RegexDatabase8.java": [ | ||
| 727 | ||
| ] | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -32,10 +32,11 @@ | |||||||
| import static java.lang.System.getProperty; | ||||||||
|
|
||||||||
| public class HardCodedCredentialsShouldNotBeUsedCheckSample { | ||||||||
| static final String FINAL_SECRET_STRING = "hunter2"; // Compliant, fake value filter | ||||||||
| static final String FINAL_SECRET_STRING_2 = "xvxf6_gaa"; | ||||||||
| // ^^^^^^^^^^^> | ||||||||
| static final byte[] FINAL_SECRET_BYTE_ARRAY = FINAL_SECRET_STRING_2.getBytes(StandardCharsets.UTF_8); | ||||||||
| // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^> | ||||||||
| private static String secretStringField = "hunter2"; | ||||||||
| private static byte[] secretByteArrayField = new byte[]{0xC, 0xA, 0xF, 0xE}; | ||||||||
| private static char[] secretCharArrayField = new char[]{0xC, 0xA, 0xF, 0xE}; | ||||||||
|
|
@@ -54,39 +55,44 @@ public static void nonCompliant(byte[] message, boolean condition, Charset encod | |||||||
| // ^^^ | ||||||||
| SHA256.getHMAC(effectivelyConstantString.getBytes(), message); // Noncompliant | ||||||||
| SHA256.getHMAC("anotherS3cr37".getBytes(), message); // Noncompliant | ||||||||
| // FINAL_SECRET_STRING.getBytes() cannot be resolved to the value "hunter2" that should be filtered -> FPs | ||||||||
| SHA256.getHMAC(FINAL_SECRET_STRING.getBytes(), message); // Noncompliant | ||||||||
| SHA256.getHMAC(FINAL_SECRET_STRING.getBytes(StandardCharsets.UTF_8), message); // Noncompliant | ||||||||
| SHA256.getHMAC(FINAL_SECRET_STRING.getBytes("UTF-8"), message); // Noncompliant | ||||||||
| SHA256.getHMAC((FINAL_SECRET_STRING).getBytes("UTF-8"), message); // Noncompliant | ||||||||
| // FP should be filtered as short string value | ||||||||
| SHA256.getHMAC(new byte[]{(byte) 0xC, (byte) 0xA, (byte) 0xF, (byte) 0xE}, message); // Noncompliant | ||||||||
|
There was a problem hiding this comment.
Suggested change
This is about a hardcoded key for |
||||||||
|
|
||||||||
| // String based | ||||||||
| HttpServletRequest request = new HttpServletRequestWrapper(null); | ||||||||
| request.login("user", "password"); // Compliant, fake value filter | ||||||||
| request.login("user", "xvxf6_gaa"); // Noncompliant | ||||||||
| request.login("user", effectivelyConstantString); // Noncompliant | ||||||||
| // ^^^^^^^^^^^^^^^^^^^^^^^^^ | ||||||||
| request.login("user", FINAL_SECRET_STRING); // Compliant, fake values filter | ||||||||
| request.login("user", FINAL_SECRET_STRING_2); // Noncompliant | ||||||||
| // ^^^^^^^^^^^^^^^^^^^^^ | ||||||||
| String plainTextSecret = new String("BOOM"); // Compliant, short values filter | ||||||||
| String plainTextSecret2 = new String("BOOMBOOM"); | ||||||||
| // ^^^^^^^^^^^^^^^^^^^^^^> | ||||||||
| request.login("user", plainTextSecret2); // Noncompliant | ||||||||
| // ^^^^^^^^^^^^^^^^ | ||||||||
| request.login("user", new String("secret")); // Noncompliant | ||||||||
|
There was a problem hiding this comment. Was this not already suppressed, even before SonarSource/sonar-analyzer-commons#418? |
||||||||
| request.login("user", new String(FINAL_SECRET_BYTE_ARRAY, 0, 7)); // Noncompliant | ||||||||
| // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||||||||
| // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^@38< | ||||||||
| // ^^^^^^^^^^^@36< | ||||||||
| request.login("user", new String(FINAL_SECRET_BYTE_ARRAY, encoding)); // Noncompliant | ||||||||
| // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||||||||
| // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^@38< | ||||||||
| // ^^^^^^^^^^^@36< | ||||||||
|
|
||||||||
| String conditionalButPredictable = condition ? FINAL_SECRET_STRING_2 : plainTextSecret2; | ||||||||
| // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^> | ||||||||
| request.login("user", conditionalButPredictable); // Noncompliant | ||||||||
| // ^^^^^^^^^^^^^^^^^^^^^^^^^ | ||||||||
| // ^^^^^^^^^^^@36< | ||||||||
| // ^^^^^^^^^^^^^^^^^^^^^^@76< | ||||||||
| request.login("user", Json.MEDIA_TYPE); // Noncompliant | ||||||||
| // ^^^^^^^^^^^^^^^ | ||||||||
| String concatenatedPassword = "abc" + true + ":" + 12 + ":" + 43L + ":" + 'a' + ":" + 0.2f + ":" + 0.2d; | ||||||||
|
|
@@ -95,7 +101,8 @@ public static void nonCompliant(byte[] message, boolean condition, Charset encod | |||||||
| // ^^^^^^^^^^^^^^^^^^^^ | ||||||||
|
|
||||||||
| jakarta.servlet.http.HttpServletRequest requestJakarta = new jakarta.servlet.http.HttpServletRequestWrapper(null); | ||||||||
| requestJakarta.login("user", "password"); // Compliant | ||||||||
| requestJakarta.login("user", "xvxf6_gaa"); // Noncompliant | ||||||||
| KeyStore store = KeyStore.getInstance(null); | ||||||||
|
|
||||||||
| store.getKey("", new char[]{0xC, 0xA, 0xF, 0xE}); // Noncompliant | ||||||||
|
|
@@ -118,38 +125,52 @@ public static void nonCompliant(byte[] message, boolean condition, Charset encod | |||||||
|
|
||||||||
| Encryptors.delux(effectivelyConstantString.subSequence(0, effectivelyConstantString.length()), effectivelyConstantString); // Noncompliant | ||||||||
| // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||||||||
| // "password".subSequence(0, 0) cannot be resolved to the value "password" that should be filtered -> FPs | ||||||||
| Encryptors.delux("password".subSequence(0, 0), "salt"); // Noncompliant | ||||||||
|
Comment on lines
+128
to
129
There was a problem hiding this comment.
Suggested change
I would follow the style where the comment for the marker is on the same line, but worth confirming how it is done elsewhere in the project. |
||||||||
| // TP | ||||||||
| Encryptors.delux("xvxf6_gaa".subSequence(0, 0), "salt"); // Noncompliant | ||||||||
|
Comment on lines
+130
to
+131
There was a problem hiding this comment.
Suggested change
|
||||||||
|
|
||||||||
| new Pbkdf2PasswordEncoder("secret"); // Compliant, fake value filter | ||||||||
| new Pbkdf2PasswordEncoder("xvxf6_gaa"); // Noncompliant | ||||||||
| new Pbkdf2PasswordEncoder(("xvxf6_gaa")); // Noncompliant | ||||||||
|
|
||||||||
|
|
||||||||
| String notInitialized; | ||||||||
| notInitialized = "abc"; | ||||||||
| new Pbkdf2PasswordEncoder(notInitialized); // Compliant, short value filter | ||||||||
|
|
||||||||
| String notInitialized2; | ||||||||
| notInitialized2 = "xvxf6_gaa"; | ||||||||
| new Pbkdf2PasswordEncoder(notInitialized2); // Noncompliant | ||||||||
|
|
||||||||
| String longString = "abcdefghiklmnop"; // Cpmpliant fake value contains 'abcd' | ||||||||
| // longString.substring(2) cannot be resolved to the value "abcdefghiklmnop" that should be filtered -> FPs | ||||||||
|
|
||||||||
| new Pbkdf2PasswordEncoder(longString.substring(2)); // Noncompliant | ||||||||
| String longString2 = "67uLmZeieGxDtp!cUm324D*7vji294"; | ||||||||
| new Pbkdf2PasswordEncoder(longString2.substring(2)); // Noncompliant | ||||||||
| new Pbkdf2PasswordEncoder(longString2.substring(0, 2)); // Noncompliant | ||||||||
| new Pbkdf2PasswordEncoder(longString2.trim()); // Noncompliant | ||||||||
| new Pbkdf2PasswordEncoder(longString2.strip()); // Noncompliant | ||||||||
| new Pbkdf2PasswordEncoder(longString2.stripIndent()); // Noncompliant | ||||||||
| new Pbkdf2PasswordEncoder(longString2.stripLeading()); // Noncompliant | ||||||||
| new Pbkdf2PasswordEncoder(longString2.stripTrailing()); // Noncompliant | ||||||||
| new Pbkdf2PasswordEncoder(longString2.translateEscapes()); // Noncompliant | ||||||||
| new Pbkdf2PasswordEncoder(longString2.intern()); // Noncompliant | ||||||||
| new Pbkdf2PasswordEncoder(longString2.toLowerCase()); // Noncompliant | ||||||||
| new Pbkdf2PasswordEncoder(longString2.toLowerCase(Locale.ROOT)); // Noncompliant | ||||||||
| new Pbkdf2PasswordEncoder(longString2.toUpperCase()); // Noncompliant | ||||||||
| new Pbkdf2PasswordEncoder(longString2.toUpperCase(Locale.ROOT)); // Noncompliant | ||||||||
| new Pbkdf2PasswordEncoder(longString2.toString()); // Noncompliant | ||||||||
|
|
||||||||
| Object stringAsObject = "abc"; | ||||||||
| new Pbkdf2PasswordEncoder(stringAsObject.toString()); // Noncompliant | ||||||||
|
|
||||||||
| java.util.function.Consumer<String> lambda = (arg) -> { | ||||||||
| new Pbkdf2PasswordEncoder(arg); | ||||||||
| String variableWithNullOwner = "abc"; | ||||||||
| new Pbkdf2PasswordEncoder(variableWithNullOwner); // Compliant, short value filter | ||||||||
| String variableWithNullOwner2 = "xvxf6_gaa"; | ||||||||
| new Pbkdf2PasswordEncoder(variableWithNullOwner2); // Noncompliant | ||||||||
| }; | ||||||||
|
|
||||||||
| byte[] secretBytes = FINAL_SECRET_STRING.getBytes("UTF-8"); | ||||||||
|
|
@@ -163,7 +184,7 @@ public static void nonCompliant(byte[] message, boolean condition, Charset encod | |||||||
| .signWith(paremSignatureAlgorithm, secretBytes); // Noncompliant | ||||||||
|
|
||||||||
| Jwts.builder() | ||||||||
| .signWith(SignatureAlgorithm.HS256, FINAL_SECRET_STRING); // Compliant, "hunter2" rejected by the fake value filter | ||||||||
|
|
||||||||
| Jwts.builder() | ||||||||
| .signWith(SignatureAlgorithm.HS256, TextCodec.BASE64.encode(FINAL_SECRET_STRING)); // Noncompliant | ||||||||
|
|
||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should one of the names indicate that it's a fake? Alas, that would balloon the diff :-(