From 6055ceeac6f4e6f6ac4e1e0a2fcc5a4e7e74b17e Mon Sep 17 00:00:00 2001 From: Luqmansonar Date: Tue, 29 Sep 2026 13:52:35 +0200 Subject: [PATCH 1/9] SONARJAVA-6438: S4790 suppress weak hash algorithms on file-sourced data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Do not raise S4790 when weak hash algorithms (MD5, SHA-1) are used exclusively on file-derived data for fingerprinting (ETags, content addressing, dedup keys), where cryptographic collision resistance is irrelevant. Detects file sources via: - NIO: Files.newInputStream/readAllBytes, Channels.newInputStream - Classic IO: FileInputStream, FileReader - Apache Commons: IOUtils.toByteArray, FileUtils.readFileToByteArray - Guava: Files.asByteSource/read, Hashing.md5/sha1 - Spring: MultipartFile.getBytes/getInputStream - Servlet: Part.getInputStream - Stream wrapping: DigestInputStream/DigestOutputStream over file streams Limitation: Source heuristic detects file-derived data but does not prove exclusive use for fingerprinting—file content could be hashed and later applied to security decisions. Accepts false negatives to eliminate noise on common fingerprinting patterns. --- .../security/DataHashingCheckSample.java | 46 ++++++++- .../checks/AbstractHashAlgorithmChecker.java | 97 +++++++++++++++++++ 2 files changed, 140 insertions(+), 3 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/security/DataHashingCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/security/DataHashingCheckSample.java index 38d847125e6..a362d139c54 100644 --- a/java-checks-test-sources/default/src/main/java/checks/security/DataHashingCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/security/DataHashingCheckSample.java @@ -156,11 +156,51 @@ class SpringDigestUtils { void digestUtils() throws IOException { org.springframework.util.DigestUtils.appendMd5DigestAsHex(new byte[10], new StringBuilder()); // Noncompliant {{Make sure this weak hash algorithm is not used in a sensitive context here.}} - org.springframework.util.DigestUtils.appendMd5DigestAsHex(new FileInputStream(""), new StringBuilder()); // Noncompliant + org.springframework.util.DigestUtils.appendMd5DigestAsHex(new FileInputStream(""), new StringBuilder()); org.springframework.util.DigestUtils.md5Digest(new byte[10]); // Noncompliant - org.springframework.util.DigestUtils.md5Digest(new FileInputStream("")); // Noncompliant {{Make sure this weak hash algorithm is not used in a sensitive context here.}} + org.springframework.util.DigestUtils.md5Digest(new FileInputStream("")); org.springframework.util.DigestUtils.md5DigestAsHex(new byte[10]); // Noncompliant - org.springframework.util.DigestUtils.md5DigestAsHex(new FileInputStream("")); // Noncompliant + org.springframework.util.DigestUtils.md5DigestAsHex(new FileInputStream("")); + } + +} + +class FileSourceHashing { + + void nioFiles(java.nio.file.Path path) throws IOException, NoSuchAlgorithmException { + MessageDigest.getInstance("MD5").digest(java.nio.file.Files.readAllBytes(path)); + MessageDigest.getInstance("SHA-1").digest(java.nio.file.Files.readAllBytes(path)); + } + + void nioInputStream(java.nio.file.Path path) throws IOException, NoSuchAlgorithmException { + MessageDigest md = MessageDigest.getInstance("MD5"); + try (java.io.InputStream is = java.nio.file.Files.newInputStream(path)) { + is.transferTo(java.io.OutputStream.nullOutputStream()); + } + } + + void classicFileInputStream(String path) throws IOException { + DigestUtils.md5Hex(new FileInputStream(path)); + DigestUtils.md5(new FileInputStream(path)); + DigestUtils.sha1Hex(new FileInputStream(path)); + } + + void guavaHashing() { + com.google.common.hash.Hashing.md5().hashBytes(new byte[0]); + } + + void apacheCommonsIO(java.io.File file) throws IOException { + DigestUtils.md5Hex(org.apache.commons.io.IOUtils.toByteArray(new FileInputStream(file))); + DigestUtils.md5Hex(org.apache.commons.io.FileUtils.readFileToByteArray(file)); + } + + void springMultipart(org.springframework.web.multipart.MultipartFile file) throws IOException { + DigestUtils.md5Hex(file.getBytes()); + DigestUtils.sha1Hex(file.getInputStream()); + } + + void servletPart(javax.servlet.http.Part part) throws IOException { + DigestUtils.md5Hex(part.getInputStream()); } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/AbstractHashAlgorithmChecker.java b/java-checks/src/main/java/org/sonar/java/checks/AbstractHashAlgorithmChecker.java index 868928b93dd..31d363ddfd3 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/AbstractHashAlgorithmChecker.java +++ b/java-checks/src/main/java/org/sonar/java/checks/AbstractHashAlgorithmChecker.java @@ -18,6 +18,7 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Optional; @@ -30,6 +31,7 @@ import org.sonar.plugins.java.api.tree.IdentifierTree; import org.sonar.plugins.java.api.tree.MethodInvocationTree; import org.sonar.plugins.java.api.tree.NewClassTree; +import org.sonar.plugins.java.api.tree.Tree; import static org.sonar.plugins.java.api.semantic.MethodMatchers.ANY; @@ -158,6 +160,9 @@ protected void onMethodInvocationFound(MethodInvocationTree mit) { reportIssue(methodName, message.get()); return; } + if (isDataFromFileSource(mit)) { + return; + } InsecureAlgorithm algorithm = ALGORITHM_BY_METHOD_NAME.get(methodName.name()); if (algorithm == null) { algorithm = algorithm(mit.arguments().get(0)).orElse(null); @@ -232,4 +237,96 @@ private static Optional algorithm(ExpressionTree invocationAr return Optional.empty(); } + private static boolean isDataFromFileSource(MethodInvocationTree mit) { + String methodName = ExpressionUtils.methodName(mit).name(); + List arguments = mit.arguments(); + + if (arguments.isEmpty()) { + return false; + } + + ExpressionTree dataArg = getDataArgument(methodName, arguments); + if (dataArg == null) { + return false; + } + + return isFileSourceExpression(dataArg); + } + + private static ExpressionTree getDataArgument(String methodName, List arguments) { + if (methodName.endsWith("Hex") || methodName.endsWith("Digest") || + methodName.equals("md5") || methodName.equals("md2") || + methodName.equals("sha") || methodName.equals("sha1")) { + return arguments.get(0); + } + if (methodName.equals("appendMd5DigestAsHex") || + methodName.equals("md5DigestAsHex") || + methodName.equals("md5Digest")) { + return arguments.get(0); + } + if (methodName.equals("hashBytes")) { + return arguments.get(0); + } + return null; + } + + private static boolean isFileSourceExpression(ExpressionTree expr) { + if (expr.is(Tree.Kind.METHOD_INVOCATION)) { + MethodInvocationTree mit = (MethodInvocationTree) expr; + String fqn = mit.methodSymbol().owner().type().fullyQualifiedName(); + String methodName = ExpressionUtils.methodName(mit).name(); + + if (isFileIOMethod(fqn, methodName)) { + return true; + } + + if (mit.arguments().size() > 0) { + ExpressionTree firstArg = mit.arguments().get(0); + if (isFileSourceExpression(firstArg)) { + return true; + } + } + } else if (expr.is(Tree.Kind.NEW_CLASS)) { + NewClassTree nct = (NewClassTree) expr; + String fqn = nct.symbolType().fullyQualifiedName(); + if (isFileConstructor(fqn)) { + return true; + } + } + + return false; + } + + private static boolean isFileIOMethod(String fqn, String methodName) { + if ("java.nio.file.Files".equals(fqn)) { + return methodName.equals("newInputStream") || methodName.equals("readAllBytes") || + methodName.equals("readString"); + } + if ("java.nio.channels.Channels".equals(fqn)) { + return methodName.equals("newInputStream"); + } + if ("org.apache.commons.io.IOUtils".equals(fqn)) { + return methodName.equals("toByteArray"); + } + if ("org.apache.commons.io.FileUtils".equals(fqn)) { + return methodName.equals("readFileToByteArray"); + } + if ("com.google.common.io.Files".equals(fqn)) { + return methodName.equals("read") || methodName.equals("asByteSource"); + } + if ("org.springframework.web.multipart.MultipartFile".equals(fqn)) { + return methodName.equals("getBytes") || methodName.equals("getInputStream"); + } + if ("javax.servlet.http.Part".equals(fqn)) { + return methodName.equals("getInputStream"); + } + return false; + } + + private static boolean isFileConstructor(String fqn) { + return "java.io.FileInputStream".equals(fqn) || + "java.io.FileReader".equals(fqn) || + "java.nio.file.FileChannel".equals(fqn); + } + } From ea969a9e5405e89b61b32a8c76979e7183ebaa67 Mon Sep 17 00:00:00 2001 From: Luqmansonar Date: Tue, 29 Sep 2026 14:48:46 +0200 Subject: [PATCH 2/9] SONARJAVA-6438: Rework S4790 file-content exemption to follow the data into the digest The previous approach only looked at arguments of the matched call, so MessageDigest.getInstance and Hashing.md5() were never exempted, and it recursed into the first argument of any method, hiding mixed data. Exemption logic now lives in DataHashingCheck behind a single hook in the base class. MD5 and SHA-1 are exempted only when every byte reaching the digest is proven to come from a file: one-shot DigestUtils calls, chained digest/update/doFinal/hashBytes calls, local digest variables whose every usage is file-fed or neutral, DigestInputStream over a file stream, and Guava ByteSource.hash. Anything unproven is still reported. --- .../security/DataHashingCheckSample.java | 115 ++++++-- .../checks/AbstractHashAlgorithmChecker.java | 103 +------ .../checks/security/DataHashingCheck.java | 253 ++++++++++++++++++ 3 files changed, 352 insertions(+), 119 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/security/DataHashingCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/security/DataHashingCheckSample.java index a362d139c54..a1893b7b12e 100644 --- a/java-checks-test-sources/default/src/main/java/checks/security/DataHashingCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/security/DataHashingCheckSample.java @@ -156,51 +156,124 @@ class SpringDigestUtils { void digestUtils() throws IOException { org.springframework.util.DigestUtils.appendMd5DigestAsHex(new byte[10], new StringBuilder()); // Noncompliant {{Make sure this weak hash algorithm is not used in a sensitive context here.}} - org.springframework.util.DigestUtils.appendMd5DigestAsHex(new FileInputStream(""), new StringBuilder()); + org.springframework.util.DigestUtils.appendMd5DigestAsHex(new FileInputStream(""), new StringBuilder()); // Compliant, file content org.springframework.util.DigestUtils.md5Digest(new byte[10]); // Noncompliant - org.springframework.util.DigestUtils.md5Digest(new FileInputStream("")); + org.springframework.util.DigestUtils.md5Digest(new FileInputStream("")); // Compliant, file content org.springframework.util.DigestUtils.md5DigestAsHex(new byte[10]); // Noncompliant - org.springframework.util.DigestUtils.md5DigestAsHex(new FileInputStream("")); + org.springframework.util.DigestUtils.md5DigestAsHex(new FileInputStream("")); // Compliant, file content } } -class FileSourceHashing { +class FileSourcedData { - void nioFiles(java.nio.file.Path path) throws IOException, NoSuchAlgorithmException { + void oneShot(java.nio.file.Path path, java.io.File file) throws IOException { + DigestUtils.md5Hex(java.nio.file.Files.newInputStream(path)); + DigestUtils.sha1Hex(java.nio.file.Files.readAllBytes(path)); + DigestUtils.md5Hex(java.nio.file.Files.readString(path)); + DigestUtils.md5Hex(new java.io.BufferedInputStream(new FileInputStream(file))); + DigestUtils.md5Hex(org.apache.commons.io.IOUtils.toByteArray(new FileInputStream(file))); + DigestUtils.md5Hex(org.apache.commons.io.IOUtils.toByteArray(new java.io.FileReader(file))); + DigestUtils.md5Hex(org.apache.commons.io.FileUtils.readFileToByteArray(file)); + DigestUtils.md5Hex(java.nio.file.Files.newInputStream(path).readAllBytes()); + DigestUtils.md5Hex(com.google.common.io.Files.asByteSource(file).read()); + DigestUtils.md5Hex(java.nio.channels.Channels.newInputStream(java.nio.channels.FileChannel.open(path))); + } + + void uploads(org.springframework.web.multipart.MultipartFile multipartFile, javax.servlet.http.Part javaxPart, jakarta.servlet.http.Part jakartaPart) throws IOException { + DigestUtils.md5Hex(multipartFile.getBytes()); + DigestUtils.md5Hex(multipartFile.getInputStream()); + DigestUtils.sha1Hex(javaxPart.getInputStream()); + DigestUtils.sha1Hex(jakartaPart.getInputStream()); + } + + void variables(java.nio.file.Path path) throws IOException { + try (java.io.InputStream resource = java.nio.file.Files.newInputStream(path)) { + DigestUtils.sha1Hex(resource); + } + } + + void chained(java.nio.file.Path path) throws IOException, NoSuchAlgorithmException { MessageDigest.getInstance("MD5").digest(java.nio.file.Files.readAllBytes(path)); MessageDigest.getInstance("SHA-1").digest(java.nio.file.Files.readAllBytes(path)); + DigestUtils.getMd5Digest().digest(java.nio.file.Files.readAllBytes(path)); + com.google.common.hash.Hashing.md5().hashBytes(java.nio.file.Files.readAllBytes(path)); } - void nioInputStream(java.nio.file.Path path) throws IOException, NoSuchAlgorithmException { + void streaming(java.nio.file.Path path) throws IOException, NoSuchAlgorithmException { MessageDigest md = MessageDigest.getInstance("MD5"); - try (java.io.InputStream is = java.nio.file.Files.newInputStream(path)) { + try (java.io.InputStream is = new java.security.DigestInputStream(java.nio.file.Files.newInputStream(path), md)) { is.transferTo(java.io.OutputStream.nullOutputStream()); } + md.digest(); } - void classicFileInputStream(String path) throws IOException { - DigestUtils.md5Hex(new FileInputStream(path)); - DigestUtils.md5(new FileInputStream(path)); - DigestUtils.sha1Hex(new FileInputStream(path)); + void updates(java.nio.file.Path first, java.nio.file.Path second) throws IOException, NoSuchAlgorithmException { + MessageDigest md = MessageDigest.getInstance("SHA-1"); + md.update(java.nio.file.Files.readAllBytes(first)); + md.update(java.nio.file.Files.readAllBytes(second)); + md.digest(); } - void guavaHashing() { - com.google.common.hash.Hashing.md5().hashBytes(new byte[0]); + void mac(java.nio.file.Path path, javax.crypto.SecretKey key) throws IOException, java.security.GeneralSecurityException { + javax.crypto.Mac mac = javax.crypto.Mac.getInstance("HmacSHA1"); + mac.init(key); + mac.doFinal(java.nio.file.Files.readAllBytes(path)); } - void apacheCommonsIO(java.io.File file) throws IOException { - DigestUtils.md5Hex(org.apache.commons.io.IOUtils.toByteArray(new FileInputStream(file))); - DigestUtils.md5Hex(org.apache.commons.io.FileUtils.readFileToByteArray(file)); + void guavaByteSource(java.io.File file) throws IOException { + com.google.common.io.Files.asByteSource(file).hash(com.google.common.hash.Hashing.md5()); } +} + +class NotProvenToBeFileData { - void springMultipart(org.springframework.web.multipart.MultipartFile file) throws IOException { - DigestUtils.md5Hex(file.getBytes()); - DigestUtils.sha1Hex(file.getInputStream()); + private final MessageDigest fieldDigest = DigestUtils.getMd5Digest(); // Noncompliant + + static byte[] concat(byte[] first, byte[] second) { + return first; + } + + void consume(MessageDigest digest) { + } + + void mixedData(java.nio.file.Path path, String password) throws IOException, NoSuchAlgorithmException { + DigestUtils.md5Hex(concat(java.nio.file.Files.readAllBytes(path), password.getBytes())); // Noncompliant + MessageDigest md = MessageDigest.getInstance("MD5"); // Noncompliant + md.update(java.nio.file.Files.readAllBytes(path)); + md.update(password.getBytes()); + md.digest(); + } + + void escapingDigest(java.nio.file.Path path) throws IOException, NoSuchAlgorithmException { + MessageDigest md = MessageDigest.getInstance("MD5"); // Noncompliant + md.update(java.nio.file.Files.readAllBytes(path)); + consume(md); + } + + void neverFed() throws NoSuchAlgorithmException { + MessageDigest md = MessageDigest.getInstance("MD5"); // Noncompliant + md.digest(); } - void servletPart(javax.servlet.http.Part part) throws IOException { - DigestUtils.md5Hex(part.getInputStream()); + void notFileData(byte[] bytes, java.io.InputStream stream, java.net.Socket socket) throws IOException { + DigestUtils.md5Hex(bytes); // Noncompliant + DigestUtils.md5Hex(new java.io.ByteArrayInputStream(bytes)); // Noncompliant + DigestUtils.md5Hex(new java.io.BufferedInputStream(stream)); // Noncompliant + DigestUtils.md5Hex(org.apache.commons.io.IOUtils.toByteArray(socket.getInputStream())); // Noncompliant + DigestUtils.md5Hex(java.nio.channels.Channels.newInputStream(java.nio.channels.Channels.newChannel(stream))); // Noncompliant + com.google.common.hash.Hashing.md5().hashBytes(bytes); // Noncompliant + com.google.common.io.ByteSource.wrap(bytes).hash(com.google.common.hash.Hashing.md5()); // Noncompliant } + void reassigned(java.nio.file.Path path, byte[] bytes) throws IOException { + java.io.InputStream stream = java.nio.file.Files.newInputStream(path); + stream = new java.io.ByteArrayInputStream(bytes); + DigestUtils.md5Hex(stream); // Noncompliant + } + + void otherAlgorithms(java.nio.file.Path path) throws IOException, NoSuchAlgorithmException { + DigestUtils.md2Hex(java.nio.file.Files.readAllBytes(path)); // Noncompliant + MessageDigest.getInstance("MD2").digest(java.nio.file.Files.readAllBytes(path)); // Noncompliant + } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/AbstractHashAlgorithmChecker.java b/java-checks/src/main/java/org/sonar/java/checks/AbstractHashAlgorithmChecker.java index 31d363ddfd3..ef2fac40ea8 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/AbstractHashAlgorithmChecker.java +++ b/java-checks/src/main/java/org/sonar/java/checks/AbstractHashAlgorithmChecker.java @@ -18,7 +18,6 @@ import java.util.ArrayList; import java.util.Arrays; -import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Optional; @@ -31,7 +30,6 @@ import org.sonar.plugins.java.api.tree.IdentifierTree; import org.sonar.plugins.java.api.tree.MethodInvocationTree; import org.sonar.plugins.java.api.tree.NewClassTree; -import org.sonar.plugins.java.api.tree.Tree; import static org.sonar.plugins.java.api.semantic.MethodMatchers.ANY; @@ -160,18 +158,19 @@ protected void onMethodInvocationFound(MethodInvocationTree mit) { reportIssue(methodName, message.get()); return; } - if (isDataFromFileSource(mit)) { - return; - } InsecureAlgorithm algorithm = ALGORITHM_BY_METHOD_NAME.get(methodName.name()); if (algorithm == null) { algorithm = algorithm(mit.arguments().get(0)).orElse(null); } - if (algorithm != null) { + if (algorithm != null && !isExempt(mit, algorithm)) { reportIssue(methodName, getMessageForAlgorithm(algorithm.toString())); } } + protected boolean isExempt(MethodInvocationTree mit, InsecureAlgorithm algorithm) { + return false; + } + @Override protected void onConstructorFound(NewClassTree newClassTree) { getMessageForClass(newClassTree.identifier().symbolType().fullyQualifiedName()) @@ -237,96 +236,4 @@ private static Optional algorithm(ExpressionTree invocationAr return Optional.empty(); } - private static boolean isDataFromFileSource(MethodInvocationTree mit) { - String methodName = ExpressionUtils.methodName(mit).name(); - List arguments = mit.arguments(); - - if (arguments.isEmpty()) { - return false; - } - - ExpressionTree dataArg = getDataArgument(methodName, arguments); - if (dataArg == null) { - return false; - } - - return isFileSourceExpression(dataArg); - } - - private static ExpressionTree getDataArgument(String methodName, List arguments) { - if (methodName.endsWith("Hex") || methodName.endsWith("Digest") || - methodName.equals("md5") || methodName.equals("md2") || - methodName.equals("sha") || methodName.equals("sha1")) { - return arguments.get(0); - } - if (methodName.equals("appendMd5DigestAsHex") || - methodName.equals("md5DigestAsHex") || - methodName.equals("md5Digest")) { - return arguments.get(0); - } - if (methodName.equals("hashBytes")) { - return arguments.get(0); - } - return null; - } - - private static boolean isFileSourceExpression(ExpressionTree expr) { - if (expr.is(Tree.Kind.METHOD_INVOCATION)) { - MethodInvocationTree mit = (MethodInvocationTree) expr; - String fqn = mit.methodSymbol().owner().type().fullyQualifiedName(); - String methodName = ExpressionUtils.methodName(mit).name(); - - if (isFileIOMethod(fqn, methodName)) { - return true; - } - - if (mit.arguments().size() > 0) { - ExpressionTree firstArg = mit.arguments().get(0); - if (isFileSourceExpression(firstArg)) { - return true; - } - } - } else if (expr.is(Tree.Kind.NEW_CLASS)) { - NewClassTree nct = (NewClassTree) expr; - String fqn = nct.symbolType().fullyQualifiedName(); - if (isFileConstructor(fqn)) { - return true; - } - } - - return false; - } - - private static boolean isFileIOMethod(String fqn, String methodName) { - if ("java.nio.file.Files".equals(fqn)) { - return methodName.equals("newInputStream") || methodName.equals("readAllBytes") || - methodName.equals("readString"); - } - if ("java.nio.channels.Channels".equals(fqn)) { - return methodName.equals("newInputStream"); - } - if ("org.apache.commons.io.IOUtils".equals(fqn)) { - return methodName.equals("toByteArray"); - } - if ("org.apache.commons.io.FileUtils".equals(fqn)) { - return methodName.equals("readFileToByteArray"); - } - if ("com.google.common.io.Files".equals(fqn)) { - return methodName.equals("read") || methodName.equals("asByteSource"); - } - if ("org.springframework.web.multipart.MultipartFile".equals(fqn)) { - return methodName.equals("getBytes") || methodName.equals("getInputStream"); - } - if ("javax.servlet.http.Part".equals(fqn)) { - return methodName.equals("getInputStream"); - } - return false; - } - - private static boolean isFileConstructor(String fqn) { - return "java.io.FileInputStream".equals(fqn) || - "java.io.FileReader".equals(fqn) || - "java.nio.file.FileChannel".equals(fqn); - } - } diff --git a/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java b/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java index 04889c18812..dd59dfae704 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java @@ -16,10 +16,24 @@ */ package org.sonar.java.checks.security; +import java.util.EnumSet; import java.util.Optional; import java.util.Set; +import javax.annotation.CheckForNull; import org.sonar.check.Rule; import org.sonar.java.checks.AbstractHashAlgorithmChecker; +import org.sonar.java.checks.helpers.ExpressionsHelper; +import org.sonar.java.model.ExpressionUtils; +import org.sonar.plugins.java.api.semantic.MethodMatchers; +import org.sonar.plugins.java.api.semantic.Symbol; +import org.sonar.plugins.java.api.tree.Arguments; +import org.sonar.plugins.java.api.tree.ExpressionTree; +import org.sonar.plugins.java.api.tree.IdentifierTree; +import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; +import org.sonar.plugins.java.api.tree.MethodInvocationTree; +import org.sonar.plugins.java.api.tree.NewClassTree; +import org.sonar.plugins.java.api.tree.Tree; +import org.sonar.plugins.java.api.tree.VariableTree; import org.sonarsource.analyzer.commons.collections.SetUtils; @Rule(key = "S4790") @@ -35,8 +49,124 @@ public class DataHashingCheck extends AbstractHashAlgorithmChecker { DeprecatedSpringPasswordEncoder.STANDARD.classFqn ); + private static final Set EXEMPTABLE_ALGORITHMS = EnumSet.of(InsecureAlgorithm.MD5, InsecureAlgorithm.SHA, InsecureAlgorithm.SHA1); + + private static final Set DATA_SINKS = Set.of("update", "digest", "doFinal", "hashBytes"); + private static final Set FINALIZERS = Set.of("digest", "doFinal"); + private static final Set NEUTRAL_CALLS = Set.of("init", "reset"); + + private static final MethodMatchers ONE_SHOT_DIGESTS = MethodMatchers.or( + MethodMatchers.create() + .ofTypes("org.apache.commons.codec.digest.DigestUtils") + .names("md5", "md5Hex", "sha1", "sha1Hex", "sha", "shaHex") + .withAnyParameters() + .build(), + MethodMatchers.create() + .ofTypes("org.springframework.util.DigestUtils") + .names("md5Digest", "md5DigestAsHex", "appendMd5DigestAsHex") + .withAnyParameters() + .build()); + + private static final MethodMatchers DIGEST_FACTORIES = MethodMatchers.or( + MethodMatchers.create() + .ofTypes("java.security.MessageDigest", "javax.crypto.Mac") + .names(GET_INSTANCE) + .withAnyParameters() + .build(), + MethodMatchers.create() + .ofTypes("org.apache.commons.codec.digest.DigestUtils") + .names("getDigest", "getMd5Digest", "getShaDigest", "getSha1Digest") + .withAnyParameters() + .build(), + MethodMatchers.create() + .ofTypes("com.google.common.hash.Hashing") + .names("md5", "sha1") + .addWithoutParametersMatcher() + .build()); + + private static final MethodMatchers FILE_SOURCES = MethodMatchers.or( + MethodMatchers.create() + .ofTypes("java.nio.file.Files") + .names("newInputStream", "readAllBytes", "readString") + .withAnyParameters() + .build(), + MethodMatchers.create() + .ofTypes("org.apache.commons.io.FileUtils") + .names("readFileToByteArray") + .withAnyParameters() + .build(), + MethodMatchers.create() + .ofSubTypes("org.springframework.web.multipart.MultipartFile") + .names("getBytes", "getInputStream") + .addWithoutParametersMatcher() + .build(), + MethodMatchers.create() + .ofSubTypes("javax.servlet.http.Part", "jakarta.servlet.http.Part") + .names("getInputStream") + .addWithoutParametersMatcher() + .build(), + MethodMatchers.create() + .ofTypes("java.io.FileInputStream", "java.io.FileReader") + .constructor() + .withAnyParameters() + .build()); + + private static final MethodMatchers CHANNEL_STREAM = MethodMatchers.create() + .ofTypes("java.nio.channels.Channels") + .names("newInputStream") + .withAnyParameters() + .build(); + + private static final MethodMatchers TO_BYTE_ARRAY = MethodMatchers.create() + .ofTypes("org.apache.commons.io.IOUtils") + .names("toByteArray") + .withAnyParameters() + .build(); + + private static final MethodMatchers STREAM_WRAPPERS = MethodMatchers.create() + .ofTypes("java.io.BufferedInputStream", "java.security.DigestInputStream") + .constructor() + .withAnyParameters() + .build(); + + private static final MethodMatchers DIGEST_INPUT_STREAM = MethodMatchers.create() + .ofTypes("java.security.DigestInputStream") + .constructor() + .withAnyParameters() + .build(); + + private static final MethodMatchers READ_ALL_BYTES = MethodMatchers.create() + .ofSubTypes("java.io.InputStream") + .names("readAllBytes") + .addWithoutParametersMatcher() + .build(); + + private static final MethodMatchers AS_BYTE_SOURCE = MethodMatchers.create() + .ofTypes("com.google.common.io.Files") + .names("asByteSource") + .withAnyParameters() + .build(); + + private static final MethodMatchers BYTE_SOURCE_READ = MethodMatchers.create() + .ofSubTypes("com.google.common.io.ByteSource") + .names("read") + .addWithoutParametersMatcher() + .build(); + + private static final MethodMatchers BYTE_SOURCE_HASH = MethodMatchers.create() + .ofSubTypes("com.google.common.io.ByteSource") + .names("hash") + .withAnyParameters() + .build(); + private static final String MESSAGE = "Make sure this weak hash algorithm is not used in a sensitive context here."; + private enum Use { + SINK, + NEUTRAL, + OTHER + } + @Override protected Optional getMessageForClass(String className) { return DEPRECATED_HASH_CLASSES.contains(className) ? Optional.of(MESSAGE) : Optional.empty(); @@ -46,4 +176,127 @@ protected Optional getMessageForClass(String className) { protected String getMessageForAlgorithm(String algorithmName) { return MESSAGE; } + + /** + * MD5 and SHA-1 are exempted only when every byte reaching the digest is proven to come from a file. + * Anything that cannot be proven (unknown symbol, field, digest passed to another method, mixed data) is still reported. + */ + @Override + protected boolean isExempt(MethodInvocationTree mit, InsecureAlgorithm algorithm) { + if (!EXEMPTABLE_ALGORITHMS.contains(algorithm)) { + return false; + } + if (ONE_SHOT_DIGESTS.matches(mit)) { + return !mit.arguments().isEmpty() && isFileSourced(mit.arguments().get(0)); + } + return DIGEST_FACTORIES.matches(mit) && isOnlyFedFileData(mit); + } + + private static boolean isOnlyFedFileData(MethodInvocationTree factoryCall) { + Tree parent = factoryCall.parent(); + if (parent.is(Tree.Kind.VARIABLE) && ((VariableTree) parent).initializer() == factoryCall) { + Symbol variable = ((VariableTree) parent).symbol(); + if (!variable.isVariableSymbol() || !variable.owner().isMethodSymbol()) { + return false; + } + boolean hasSink = false; + for (IdentifierTree usage : variable.usages()) { + Use use = classify(usage); + if (use == Use.OTHER) { + return false; + } + hasSink |= use == Use.SINK; + } + return hasSink; + } + return classify(factoryCall) == Use.SINK; + } + + private static Use classify(ExpressionTree use) { + Tree parent = use.parent(); + if (parent.is(Tree.Kind.MEMBER_SELECT) + && ((MemberSelectExpressionTree) parent).expression() == use + && parent.parent().is(Tree.Kind.METHOD_INVOCATION) + && ((MethodInvocationTree) parent.parent()).methodSelect() == parent) { + return classifyCall(((MemberSelectExpressionTree) parent).identifier().name(), ((MethodInvocationTree) parent.parent()).arguments()); + } + if (parent.is(Tree.Kind.ARGUMENTS)) { + return classifyArgument(use, parent.parent()); + } + return Use.OTHER; + } + + private static Use classifyCall(String name, Arguments arguments) { + if (DATA_SINKS.contains(name) && !arguments.isEmpty()) { + return isFileSourced(arguments.get(0)) ? Use.SINK : Use.OTHER; + } + if (NEUTRAL_CALLS.contains(name) || (arguments.isEmpty() && FINALIZERS.contains(name))) { + return Use.NEUTRAL; + } + return Use.OTHER; + } + + private static Use classifyArgument(ExpressionTree argument, Tree owner) { + if (owner.is(Tree.Kind.NEW_CLASS)) { + NewClassTree newClass = (NewClassTree) owner; + Arguments arguments = newClass.arguments(); + boolean feedsFromFileStream = DIGEST_INPUT_STREAM.matches(newClass) + && arguments.size() == 2 + && arguments.get(1) == argument + && isFileSourced(arguments.get(0)); + return feedsFromFileStream ? Use.SINK : Use.OTHER; + } + if (owner.is(Tree.Kind.METHOD_INVOCATION) && BYTE_SOURCE_HASH.matches((MethodInvocationTree) owner)) { + ExpressionTree receiver = receiver((MethodInvocationTree) owner); + return receiver != null && isFileSourced(receiver) ? Use.SINK : Use.OTHER; + } + return Use.OTHER; + } + + private static boolean isFileSourced(ExpressionTree expression) { + ExpressionTree expr = ExpressionUtils.skipParentheses(expression); + if (expr.is(Tree.Kind.IDENTIFIER)) { + return isFileSourcedVariable(((IdentifierTree) expr).symbol()); + } + if (expr.is(Tree.Kind.NEW_CLASS)) { + NewClassTree newClass = (NewClassTree) expr; + return FILE_SOURCES.matches(newClass) + || (STREAM_WRAPPERS.matches(newClass) && !newClass.arguments().isEmpty() && isFileSourced(newClass.arguments().get(0))); + } + if (expr.is(Tree.Kind.METHOD_INVOCATION)) { + return isFileSourcedCall((MethodInvocationTree) expr); + } + return false; + } + + private static boolean isFileSourcedVariable(Symbol symbol) { + if (!symbol.isVariableSymbol() || !symbol.owner().isMethodSymbol()) { + return false; + } + ExpressionTree value = ExpressionsHelper.getSingleWriteUsage(symbol); + return value != null && isFileSourced(value); + } + + private static boolean isFileSourcedCall(MethodInvocationTree call) { + if (FILE_SOURCES.matches(call) || AS_BYTE_SOURCE.matches(call)) { + return true; + } + Arguments arguments = call.arguments(); + if (CHANNEL_STREAM.matches(call)) { + return !arguments.isEmpty() && arguments.get(0).symbolType().isSubtypeOf("java.nio.channels.FileChannel"); + } + if (TO_BYTE_ARRAY.matches(call)) { + return !arguments.isEmpty() && isFileSourced(arguments.get(0)); + } + if (READ_ALL_BYTES.matches(call) || BYTE_SOURCE_READ.matches(call)) { + ExpressionTree receiver = receiver(call); + return receiver != null && isFileSourced(receiver); + } + return false; + } + + @CheckForNull + private static ExpressionTree receiver(MethodInvocationTree call) { + return call.methodSelect().is(Tree.Kind.MEMBER_SELECT) ? ((MemberSelectExpressionTree) call.methodSelect()).expression() : null; + } } From be113f3d1fd456189a72ad5e8e0d1c7ec3fbdccc Mon Sep 17 00:00:00 2001 From: Luqmansonar Date: Tue, 29 Sep 2026 15:14:45 +0200 Subject: [PATCH 3/9] SONARJAVA-6438: Make isExempt abstract to drop unused parameters --- .../org/sonar/java/checks/AbstractHashAlgorithmChecker.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/AbstractHashAlgorithmChecker.java b/java-checks/src/main/java/org/sonar/java/checks/AbstractHashAlgorithmChecker.java index ef2fac40ea8..f7de23d3b38 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/AbstractHashAlgorithmChecker.java +++ b/java-checks/src/main/java/org/sonar/java/checks/AbstractHashAlgorithmChecker.java @@ -167,9 +167,7 @@ protected void onMethodInvocationFound(MethodInvocationTree mit) { } } - protected boolean isExempt(MethodInvocationTree mit, InsecureAlgorithm algorithm) { - return false; - } + protected abstract boolean isExempt(MethodInvocationTree mit, InsecureAlgorithm algorithm); @Override protected void onConstructorFound(NewClassTree newClassTree) { From a4e78f18d2881e9d4c899cd4a8da2689a29f8ff7 Mon Sep 17 00:00:00 2001 From: Luqmansonar Date: Tue, 29 Sep 2026 15:15:03 +0200 Subject: [PATCH 4/9] SONARJAVA-6438: Update S4790 ruling expectation for sonar-server --- .../src/test/resources/expected/sonar-server/java-S4790.json | 3 --- 1 file changed, 3 deletions(-) diff --git a/its/ruling/src/test/resources/expected/sonar-server/java-S4790.json b/its/ruling/src/test/resources/expected/sonar-server/java-S4790.json index 541ac539209..d6dd837293b 100644 --- a/its/ruling/src/test/resources/expected/sonar-server/java-S4790.json +++ b/its/ruling/src/test/resources/expected/sonar-server/java-S4790.json @@ -1,7 +1,4 @@ { -"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/batch/BatchIndex.java": [ -62 -], "org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/computation/task/projectanalysis/step/PersistFileSourcesStep.java": [ 135 ], From 7c7b37c6d941086fe28c2f6b116aabf59ee05bef Mon Sep 17 00:00:00 2001 From: Luqmansonar Date: Tue, 29 Sep 2026 16:17:54 +0200 Subject: [PATCH 5/9] SONARJAVA-6438: Stop treating MultipartFile.getInputStream and Part.getInputStream as file sources --- .../main/java/checks/security/DataHashingCheckSample.java | 6 +++--- .../org/sonar/java/checks/security/DataHashingCheck.java | 7 +------ 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/security/DataHashingCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/security/DataHashingCheckSample.java index a1893b7b12e..87c0d8e9623 100644 --- a/java-checks-test-sources/default/src/main/java/checks/security/DataHashingCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/security/DataHashingCheckSample.java @@ -182,9 +182,9 @@ void oneShot(java.nio.file.Path path, java.io.File file) throws IOException { void uploads(org.springframework.web.multipart.MultipartFile multipartFile, javax.servlet.http.Part javaxPart, jakarta.servlet.http.Part jakartaPart) throws IOException { DigestUtils.md5Hex(multipartFile.getBytes()); - DigestUtils.md5Hex(multipartFile.getInputStream()); - DigestUtils.sha1Hex(javaxPart.getInputStream()); - DigestUtils.sha1Hex(jakartaPart.getInputStream()); + DigestUtils.md5Hex(multipartFile.getInputStream()); // Noncompliant + DigestUtils.sha1Hex(javaxPart.getInputStream()); // Noncompliant + DigestUtils.sha1Hex(jakartaPart.getInputStream()); // Noncompliant } void variables(java.nio.file.Path path) throws IOException { diff --git a/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java b/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java index dd59dfae704..9fbb971819e 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java @@ -97,12 +97,7 @@ public class DataHashingCheck extends AbstractHashAlgorithmChecker { .build(), MethodMatchers.create() .ofSubTypes("org.springframework.web.multipart.MultipartFile") - .names("getBytes", "getInputStream") - .addWithoutParametersMatcher() - .build(), - MethodMatchers.create() - .ofSubTypes("javax.servlet.http.Part", "jakarta.servlet.http.Part") - .names("getInputStream") + .names("getBytes") .addWithoutParametersMatcher() .build(), MethodMatchers.create() From 23af8abf09d5fbaff1c89167db9d2bee74cde0d0 Mon Sep 17 00:00:00 2001 From: Luqmansonar Date: Tue, 29 Sep 2026 16:18:44 +0200 Subject: [PATCH 6/9] SONARJAVA-6438: Stop treating MultipartFile.getBytes as a file source --- .../main/java/checks/security/DataHashingCheckSample.java | 2 +- .../org/sonar/java/checks/security/DataHashingCheck.java | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/security/DataHashingCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/security/DataHashingCheckSample.java index 87c0d8e9623..902773e5377 100644 --- a/java-checks-test-sources/default/src/main/java/checks/security/DataHashingCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/security/DataHashingCheckSample.java @@ -181,7 +181,7 @@ void oneShot(java.nio.file.Path path, java.io.File file) throws IOException { } void uploads(org.springframework.web.multipart.MultipartFile multipartFile, javax.servlet.http.Part javaxPart, jakarta.servlet.http.Part jakartaPart) throws IOException { - DigestUtils.md5Hex(multipartFile.getBytes()); + DigestUtils.md5Hex(multipartFile.getBytes()); // Noncompliant DigestUtils.md5Hex(multipartFile.getInputStream()); // Noncompliant DigestUtils.sha1Hex(javaxPart.getInputStream()); // Noncompliant DigestUtils.sha1Hex(jakartaPart.getInputStream()); // Noncompliant diff --git a/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java b/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java index 9fbb971819e..479252d70cb 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java @@ -95,11 +95,6 @@ public class DataHashingCheck extends AbstractHashAlgorithmChecker { .names("readFileToByteArray") .withAnyParameters() .build(), - MethodMatchers.create() - .ofSubTypes("org.springframework.web.multipart.MultipartFile") - .names("getBytes") - .addWithoutParametersMatcher() - .build(), MethodMatchers.create() .ofTypes("java.io.FileInputStream", "java.io.FileReader") .constructor() From 34f6c922383654250b8033984537471fc746cd95 Mon Sep 17 00:00:00 2001 From: Luqmansonar Date: Tue, 29 Sep 2026 16:21:24 +0200 Subject: [PATCH 7/9] SONARJAVA-6438: Only exempt array variables used exactly once, since element writes are not tracked --- .../security/DataHashingCheckSample.java | 20 +++++++++++++++++++ .../checks/security/DataHashingCheck.java | 3 +++ 2 files changed, 23 insertions(+) diff --git a/java-checks-test-sources/default/src/main/java/checks/security/DataHashingCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/security/DataHashingCheckSample.java index 902773e5377..175f03cf191 100644 --- a/java-checks-test-sources/default/src/main/java/checks/security/DataHashingCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/security/DataHashingCheckSample.java @@ -191,6 +191,8 @@ void variables(java.nio.file.Path path) throws IOException { try (java.io.InputStream resource = java.nio.file.Files.newInputStream(path)) { DigestUtils.sha1Hex(resource); } + byte[] content = java.nio.file.Files.readAllBytes(path); + DigestUtils.md5Hex(content); } void chained(java.nio.file.Path path) throws IOException, NoSuchAlgorithmException { @@ -266,6 +268,24 @@ void notFileData(byte[] bytes, java.io.InputStream stream, java.net.Socket socke com.google.common.io.ByteSource.wrap(bytes).hash(com.google.common.hash.Hashing.md5()); // Noncompliant } + void arrayOverwritten(java.nio.file.Path path, byte[] secret) throws IOException { + byte[] data = java.nio.file.Files.readAllBytes(path); + System.arraycopy(secret, 0, data, 0, secret.length); + DigestUtils.md5Hex(data); // Noncompliant + } + + void arrayXored(java.nio.file.Path path, byte[] key) throws IOException { + byte[] data = java.nio.file.Files.readAllBytes(path); + data[0] ^= key[0]; + DigestUtils.md5Hex(data); // Noncompliant + } + + void arrayMixed(java.nio.file.Path path, byte[] secret) throws IOException { + byte[] data = java.nio.file.Files.readAllBytes(path); + concat(data, secret); + DigestUtils.md5Hex(data); // Noncompliant + } + void reassigned(java.nio.file.Path path, byte[] bytes) throws IOException { java.io.InputStream stream = java.nio.file.Files.newInputStream(path); stream = new java.io.ByteArrayInputStream(bytes); diff --git a/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java b/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java index 479252d70cb..b3c1b8ad5d3 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java @@ -263,6 +263,9 @@ private static boolean isFileSourcedVariable(Symbol symbol) { if (!symbol.isVariableSymbol() || !symbol.owner().isMethodSymbol()) { return false; } + if (symbol.type().isArray() && symbol.usages().size() != 1) { + return false; + } ExpressionTree value = ExpressionsHelper.getSingleWriteUsage(symbol); return value != null && isFileSourced(value); } From b306092eef91005ea8a67129475ce8273b2ba92f Mon Sep 17 00:00:00 2001 From: Luqmansonar Date: Wed, 30 Sep 2026 12:11:26 +0200 Subject: [PATCH 8/9] SONARJAVA-6438: Merge AbstractHashAlgorithmChecker into DataHashingCheck and simplify classify --- .agents/RIS_PREPROCESSING.md | 2 - .../checks/AbstractHashAlgorithmChecker.java | 237 ------------------ .../checks/security/DataHashingCheck.java | 229 +++++++++++++++-- 3 files changed, 207 insertions(+), 261 deletions(-) delete mode 100644 java-checks/src/main/java/org/sonar/java/checks/AbstractHashAlgorithmChecker.java diff --git a/.agents/RIS_PREPROCESSING.md b/.agents/RIS_PREPROCESSING.md index 90f83dfe08a..65d27a51c8c 100644 --- a/.agents/RIS_PREPROCESSING.md +++ b/.agents/RIS_PREPROCESSING.md @@ -15,7 +15,6 @@ This analysis is based on direct inspection of the following source files: **Abstract Base Classes:** - `java-checks/src/main/java/org/sonar/java/checks/AbstractForLoopRule.java` -- `java-checks/src/main/java/org/sonar/java/checks/AbstractHashAlgorithmChecker.java` - `java-checks/src/main/java/org/sonar/java/checks/regex/AbstractRegexCheck.java` - `java-checks-common/src/main/java/org/sonar/java/checks/methods/AbstractMethodDetection.java` @@ -70,7 +69,6 @@ Based on file listings and @Rule annotation searches, the main java-checks modul - `AbstractRegexCheck` for regex validation rules - `AbstractMethodDetection` for method invocation-based rules - `AbstractForLoopRule` for for-loop analysis rules - - `AbstractHashAlgorithmChecker` for cryptography rules 3. **Module organization:** - Core rules in `java-checks` diff --git a/java-checks/src/main/java/org/sonar/java/checks/AbstractHashAlgorithmChecker.java b/java-checks/src/main/java/org/sonar/java/checks/AbstractHashAlgorithmChecker.java deleted file mode 100644 index f7de23d3b38..00000000000 --- a/java-checks/src/main/java/org/sonar/java/checks/AbstractHashAlgorithmChecker.java +++ /dev/null @@ -1,237 +0,0 @@ -/* - * SonarQube Java - * Copyright (C) SonarSource Sàrl - * mailto:info AT sonarsource DOT com - * - * You can redistribute and/or modify this program under the terms of - * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - * See the Sonar Source-Available License for more details. - * - * You should have received a copy of the Sonar Source-Available License - * along with this program; if not, see https://sonarsource.com/license/ssal/ - */ -package org.sonar.java.checks; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Locale; -import java.util.Map; -import java.util.Optional; -import org.sonar.java.checks.helpers.JavaPropertiesHelper; -import org.sonar.java.checks.methods.AbstractMethodDetection; -import org.sonarsource.analyzer.commons.collections.MapBuilder; -import org.sonar.java.model.ExpressionUtils; -import org.sonar.plugins.java.api.semantic.MethodMatchers; -import org.sonar.plugins.java.api.tree.ExpressionTree; -import org.sonar.plugins.java.api.tree.IdentifierTree; -import org.sonar.plugins.java.api.tree.MethodInvocationTree; -import org.sonar.plugins.java.api.tree.NewClassTree; - -import static org.sonar.plugins.java.api.semantic.MethodMatchers.ANY; - -public abstract class AbstractHashAlgorithmChecker extends AbstractMethodDetection { - - public static final String GET_INSTANCE = "getInstance"; - public static final String JAVA_LANG_STRING = "java.lang.String"; - - protected static final Map ALGORITHM_BY_METHOD_NAME = MapBuilder.newMap() - .put("getMd2Digest", InsecureAlgorithm.MD2) - .put("getMd5Digest", InsecureAlgorithm.MD5) - .put("getShaDigest", InsecureAlgorithm.SHA1) - .put("getSha1Digest", InsecureAlgorithm.SHA1) - .put("md2", InsecureAlgorithm.MD2) - .put("md2Hex", InsecureAlgorithm.MD2) - .put("md5", InsecureAlgorithm.MD5) - .put("md5Hex", InsecureAlgorithm.MD5) - .put("sha1", InsecureAlgorithm.SHA1) - .put("sha1Hex", InsecureAlgorithm.SHA1) - .put("sha", InsecureAlgorithm.SHA1) - .put("shaHex", InsecureAlgorithm.SHA1) - .put("md5Digest", InsecureAlgorithm.MD5) - .put("md5DigestAsHex", InsecureAlgorithm.MD5) - .put("appendMd5DigestAsHex", InsecureAlgorithm.MD5) - .build(); - - private static final String CONSTRUCTOR = ""; - - /** - * These APIs have static getInstance method to get an implementation of some crypto algorithm. - * javax.crypto.Cipher is missing from this list, because it is covered by rule S5547 {@link StrongCipherAlgorithmCheck} - * Details can be found here Security Standard Names - */ - private static final String[] CRYPTO_APIS = { - "java.security.AlgorithmParameters", - "java.security.AlgorithmParameterGenerator", - "java.security.MessageDigest", - "java.security.KeyFactory", - "java.security.KeyPairGenerator", - "java.security.Signature", - "javax.crypto.Mac", - "javax.crypto.KeyGenerator" - }; - - public enum InsecureAlgorithm { - MD2, MD4, MD5, MD6, RIPEMD, - HAVAL128 { - @Override - public String toString() { - return "HAVAL-128"; - } - }, - SHA { - @Override - public boolean match(String algorithm) { - // exact match required for SHA, so it doesn't match compliant SHA-512 - return "SHA".equals(algorithm); - } - }, - SHA0 { - @Override - public String toString() { - return "SHA-0"; - } - }, - SHA1 { - @Override - public String toString() { - return "SHA-1"; - } - }, - SHA224 { - @Override - public String toString() { - return "SHA-224"; - } - }, - DSA { - @Override - public boolean match(String algorithm) { - // exact match required for DSA, so it doesn't match ECDSA - return "DSA".equals(algorithm); - } - }; - - public boolean match(String algorithm) { - String normalizedName = algorithm.replace("-", "").toLowerCase(Locale.ENGLISH); - return normalizedName.contains(name().toLowerCase(Locale.ENGLISH)); - } - } - - public enum DeprecatedSpringPasswordEncoder { - MD5("org.springframework.security.authentication.encoding.Md5PasswordEncoder", CONSTRUCTOR), - SHA("org.springframework.security.authentication.encoding.ShaPasswordEncoder", CONSTRUCTOR), - LDAP("org.springframework.security.crypto.password.LdapShaPasswordEncoder", CONSTRUCTOR), - MD4("org.springframework.security.crypto.password.Md4PasswordEncoder", CONSTRUCTOR), - MESSAGE_DIGEST("org.springframework.security.crypto.password.MessageDigestPasswordEncoder", CONSTRUCTOR), - STANDARD("org.springframework.security.crypto.password.StandardPasswordEncoder", CONSTRUCTOR), - NO_OP("org.springframework.security.crypto.password.NoOpPasswordEncoder", GET_INSTANCE); - - public final String classFqn; - public final String methodName; - public final String className; - - DeprecatedSpringPasswordEncoder(String fqn, String methodName) { - this.classFqn = fqn; - this.methodName = methodName; - String[] fqnParts = fqn.split("\\."); - this.className = fqnParts[fqnParts.length - 1]; - } - } - - protected abstract Optional getMessageForClass(String className); - protected abstract String getMessageForAlgorithm(String algorithmName); - - @Override - protected MethodMatchers getMethodInvocationMatchers() { - return getWeakHashMethodInvocationMatchers(); - } - - @Override - protected void onMethodInvocationFound(MethodInvocationTree mit) { - IdentifierTree methodName = ExpressionUtils.methodName(mit); - Optional message = getMessageForClass(methodName.symbol().owner().type().fullyQualifiedName()); - if (message.isPresent()) { - reportIssue(methodName, message.get()); - return; - } - InsecureAlgorithm algorithm = ALGORITHM_BY_METHOD_NAME.get(methodName.name()); - if (algorithm == null) { - algorithm = algorithm(mit.arguments().get(0)).orElse(null); - } - if (algorithm != null && !isExempt(mit, algorithm)) { - reportIssue(methodName, getMessageForAlgorithm(algorithm.toString())); - } - } - - protected abstract boolean isExempt(MethodInvocationTree mit, InsecureAlgorithm algorithm); - - @Override - protected void onConstructorFound(NewClassTree newClassTree) { - getMessageForClass(newClassTree.identifier().symbolType().fullyQualifiedName()) - .ifPresent(message -> reportIssue(newClassTree.identifier(), message)); - } - - private static MethodMatchers getWeakHashMethodInvocationMatchers() { - ArrayList matchers = new ArrayList<>(); - matchers - .add(MethodMatchers.create() - .ofTypes("org.apache.commons.codec.digest.DigestUtils") - .names("getDigest") - .addParametersMatcher(JAVA_LANG_STRING) - .build()); - - matchers - .add(MethodMatchers.create() - .ofTypes("org.apache.commons.codec.digest.DigestUtils") - .name(ALGORITHM_BY_METHOD_NAME::containsKey) - .withAnyParameters() - .build()); - - matchers - .add(MethodMatchers.create() - .ofTypes(CRYPTO_APIS) - .names(GET_INSTANCE) - .addParametersMatcher(JAVA_LANG_STRING) - .addParametersMatcher(JAVA_LANG_STRING, ANY) - .build()); - - matchers - .add(MethodMatchers.create() - .ofTypes("org.springframework.util.DigestUtils") - .names("appendMd5DigestAsHex", "md5Digest", "md5DigestAsHex") - .withAnyParameters() - .build()); - - for (DeprecatedSpringPasswordEncoder pe : DeprecatedSpringPasswordEncoder.values()) { - matchers.add(MethodMatchers.create().ofTypes(pe.classFqn).names(pe.methodName).withAnyParameters().build()); - } - - matchers.add(MethodMatchers.create() - .ofTypes("com.google.common.hash.Hashing") - .names("md5", "sha1") - .addWithoutParametersMatcher().build()); - - return MethodMatchers.or(matchers); - } - - private static Optional algorithm(ExpressionTree invocationArgument) { - ExpressionTree expectedAlgorithm = invocationArgument; - ExpressionTree defaultPropertyValue = JavaPropertiesHelper.retrievedPropertyDefaultValue(invocationArgument); - if (defaultPropertyValue != null) { - expectedAlgorithm = defaultPropertyValue; - } - Optional stringConstant = expectedAlgorithm.asConstant(String.class); - if (stringConstant.isPresent()) { - String algorithmName = stringConstant.get(); - return Arrays.stream(InsecureAlgorithm.values()) - .filter(alg -> alg.match(algorithmName)) - .findFirst(); - } - return Optional.empty(); - } - -} diff --git a/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java b/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java index b3c1b8ad5d3..deab387e5c5 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java @@ -16,13 +16,18 @@ */ package org.sonar.java.checks.security; +import java.util.ArrayList; +import java.util.Arrays; import java.util.EnumSet; +import java.util.Locale; +import java.util.Map; import java.util.Optional; import java.util.Set; import javax.annotation.CheckForNull; import org.sonar.check.Rule; -import org.sonar.java.checks.AbstractHashAlgorithmChecker; import org.sonar.java.checks.helpers.ExpressionsHelper; +import org.sonar.java.checks.helpers.JavaPropertiesHelper; +import org.sonar.java.checks.methods.AbstractMethodDetection; import org.sonar.java.model.ExpressionUtils; import org.sonar.plugins.java.api.semantic.MethodMatchers; import org.sonar.plugins.java.api.semantic.Symbol; @@ -34,10 +39,116 @@ import org.sonar.plugins.java.api.tree.NewClassTree; import org.sonar.plugins.java.api.tree.Tree; import org.sonar.plugins.java.api.tree.VariableTree; +import org.sonarsource.analyzer.commons.collections.MapBuilder; import org.sonarsource.analyzer.commons.collections.SetUtils; +import static org.sonar.plugins.java.api.semantic.MethodMatchers.ANY; + @Rule(key = "S4790") -public class DataHashingCheck extends AbstractHashAlgorithmChecker { +public class DataHashingCheck extends AbstractMethodDetection { + + private static final String GET_INSTANCE = "getInstance"; + private static final String JAVA_LANG_STRING = "java.lang.String"; + private static final String CONSTRUCTOR = ""; + + private static final Map ALGORITHM_BY_METHOD_NAME = MapBuilder.newMap() + .put("getMd2Digest", InsecureAlgorithm.MD2) + .put("getMd5Digest", InsecureAlgorithm.MD5) + .put("getShaDigest", InsecureAlgorithm.SHA1) + .put("getSha1Digest", InsecureAlgorithm.SHA1) + .put("md2", InsecureAlgorithm.MD2) + .put("md2Hex", InsecureAlgorithm.MD2) + .put("md5", InsecureAlgorithm.MD5) + .put("md5Hex", InsecureAlgorithm.MD5) + .put("sha1", InsecureAlgorithm.SHA1) + .put("sha1Hex", InsecureAlgorithm.SHA1) + .put("sha", InsecureAlgorithm.SHA1) + .put("shaHex", InsecureAlgorithm.SHA1) + .put("md5Digest", InsecureAlgorithm.MD5) + .put("md5DigestAsHex", InsecureAlgorithm.MD5) + .put("appendMd5DigestAsHex", InsecureAlgorithm.MD5) + .build(); + + /** + * These APIs have static getInstance method to get an implementation of some crypto algorithm. + * javax.crypto.Cipher is missing from this list, because it is covered by rule S5547 {@link org.sonar.java.checks.StrongCipherAlgorithmCheck} + * Details can be found here Security Standard Names + */ + private static final String[] CRYPTO_APIS = { + "java.security.AlgorithmParameters", + "java.security.AlgorithmParameterGenerator", + "java.security.MessageDigest", + "java.security.KeyFactory", + "java.security.KeyPairGenerator", + "java.security.Signature", + "javax.crypto.Mac", + "javax.crypto.KeyGenerator" + }; + + private enum InsecureAlgorithm { + MD2, MD4, MD5, MD6, RIPEMD, + HAVAL128 { + @Override + public String toString() { + return "HAVAL-128"; + } + }, + SHA { + @Override + public boolean match(String algorithm) { + // exact match required for SHA, so it doesn't match compliant SHA-512 + return "SHA".equals(algorithm); + } + }, + SHA0 { + @Override + public String toString() { + return "SHA-0"; + } + }, + SHA1 { + @Override + public String toString() { + return "SHA-1"; + } + }, + SHA224 { + @Override + public String toString() { + return "SHA-224"; + } + }, + DSA { + @Override + public boolean match(String algorithm) { + // exact match required for DSA, so it doesn't match ECDSA + return "DSA".equals(algorithm); + } + }; + + public boolean match(String algorithm) { + String normalizedName = algorithm.replace("-", "").toLowerCase(Locale.ENGLISH); + return normalizedName.contains(name().toLowerCase(Locale.ENGLISH)); + } + } + + private enum DeprecatedSpringPasswordEncoder { + MD5("org.springframework.security.authentication.encoding.Md5PasswordEncoder", CONSTRUCTOR), + SHA("org.springframework.security.authentication.encoding.ShaPasswordEncoder", CONSTRUCTOR), + LDAP("org.springframework.security.crypto.password.LdapShaPasswordEncoder", CONSTRUCTOR), + MD4("org.springframework.security.crypto.password.Md4PasswordEncoder", CONSTRUCTOR), + MESSAGE_DIGEST("org.springframework.security.crypto.password.MessageDigestPasswordEncoder", CONSTRUCTOR), + STANDARD("org.springframework.security.crypto.password.StandardPasswordEncoder", CONSTRUCTOR), + NO_OP("org.springframework.security.crypto.password.NoOpPasswordEncoder", GET_INSTANCE); + + private final String classFqn; + private final String methodName; + + DeprecatedSpringPasswordEncoder(String fqn, String methodName) { + this.classFqn = fqn; + this.methodName = methodName; + } + } private static final Set DEPRECATED_HASH_CLASSES = SetUtils.immutableSetOf( DeprecatedSpringPasswordEncoder.MD5.classFqn, @@ -157,22 +268,11 @@ private enum Use { OTHER } - @Override - protected Optional getMessageForClass(String className) { - return DEPRECATED_HASH_CLASSES.contains(className) ? Optional.of(MESSAGE) : Optional.empty(); - } - - @Override - protected String getMessageForAlgorithm(String algorithmName) { - return MESSAGE; - } - /** * MD5 and SHA-1 are exempted only when every byte reaching the digest is proven to come from a file. * Anything that cannot be proven (unknown symbol, field, digest passed to another method, mixed data) is still reported. */ - @Override - protected boolean isExempt(MethodInvocationTree mit, InsecureAlgorithm algorithm) { + private static boolean isExempt(MethodInvocationTree mit, InsecureAlgorithm algorithm) { if (!EXEMPTABLE_ALGORITHMS.contains(algorithm)) { return false; } @@ -203,15 +303,13 @@ private static boolean isOnlyFedFileData(MethodInvocationTree factoryCall) { } private static Use classify(ExpressionTree use) { - Tree parent = use.parent(); - if (parent.is(Tree.Kind.MEMBER_SELECT) - && ((MemberSelectExpressionTree) parent).expression() == use - && parent.parent().is(Tree.Kind.METHOD_INVOCATION) - && ((MethodInvocationTree) parent.parent()).methodSelect() == parent) { - return classifyCall(((MemberSelectExpressionTree) parent).identifier().name(), ((MethodInvocationTree) parent.parent()).arguments()); + if (use.parent() instanceof MemberSelectExpressionTree memberSelect + && memberSelect.expression() == use + && memberSelect.parent() instanceof MethodInvocationTree invocation) { + return classifyCall(memberSelect.identifier().name(), invocation.arguments()); } - if (parent.is(Tree.Kind.ARGUMENTS)) { - return classifyArgument(use, parent.parent()); + if (use.parent().is(Tree.Kind.ARGUMENTS)) { + return classifyArgument(use, use.parent().parent()); } return Use.OTHER; } @@ -292,4 +390,91 @@ private static boolean isFileSourcedCall(MethodInvocationTree call) { private static ExpressionTree receiver(MethodInvocationTree call) { return call.methodSelect().is(Tree.Kind.MEMBER_SELECT) ? ((MemberSelectExpressionTree) call.methodSelect()).expression() : null; } + + @Override + protected MethodMatchers getMethodInvocationMatchers() { + return getWeakHashMethodInvocationMatchers(); + } + + @Override + protected void onMethodInvocationFound(MethodInvocationTree mit) { + IdentifierTree methodName = ExpressionUtils.methodName(mit); + if (DEPRECATED_HASH_CLASSES.contains(methodName.symbol().owner().type().fullyQualifiedName())) { + reportIssue(methodName, MESSAGE); + return; + } + InsecureAlgorithm algorithm = ALGORITHM_BY_METHOD_NAME.get(methodName.name()); + if (algorithm == null) { + algorithm = algorithm(mit.arguments().get(0)).orElse(null); + } + if (algorithm != null && !isExempt(mit, algorithm)) { + reportIssue(methodName, MESSAGE); + } + } + + @Override + protected void onConstructorFound(NewClassTree newClassTree) { + if (DEPRECATED_HASH_CLASSES.contains(newClassTree.identifier().symbolType().fullyQualifiedName())) { + reportIssue(newClassTree.identifier(), MESSAGE); + } + } + + private static MethodMatchers getWeakHashMethodInvocationMatchers() { + ArrayList matchers = new ArrayList<>(); + matchers + .add(MethodMatchers.create() + .ofTypes("org.apache.commons.codec.digest.DigestUtils") + .names("getDigest") + .addParametersMatcher(JAVA_LANG_STRING) + .build()); + + matchers + .add(MethodMatchers.create() + .ofTypes("org.apache.commons.codec.digest.DigestUtils") + .name(ALGORITHM_BY_METHOD_NAME::containsKey) + .withAnyParameters() + .build()); + + matchers + .add(MethodMatchers.create() + .ofTypes(CRYPTO_APIS) + .names(GET_INSTANCE) + .addParametersMatcher(JAVA_LANG_STRING) + .addParametersMatcher(JAVA_LANG_STRING, ANY) + .build()); + + matchers + .add(MethodMatchers.create() + .ofTypes("org.springframework.util.DigestUtils") + .names("appendMd5DigestAsHex", "md5Digest", "md5DigestAsHex") + .withAnyParameters() + .build()); + + for (DeprecatedSpringPasswordEncoder pe : DeprecatedSpringPasswordEncoder.values()) { + matchers.add(MethodMatchers.create().ofTypes(pe.classFqn).names(pe.methodName).withAnyParameters().build()); + } + + matchers.add(MethodMatchers.create() + .ofTypes("com.google.common.hash.Hashing") + .names("md5", "sha1") + .addWithoutParametersMatcher().build()); + + return MethodMatchers.or(matchers); + } + + private static Optional algorithm(ExpressionTree invocationArgument) { + ExpressionTree expectedAlgorithm = invocationArgument; + ExpressionTree defaultPropertyValue = JavaPropertiesHelper.retrievedPropertyDefaultValue(invocationArgument); + if (defaultPropertyValue != null) { + expectedAlgorithm = defaultPropertyValue; + } + Optional stringConstant = expectedAlgorithm.asConstant(String.class); + if (stringConstant.isPresent()) { + String algorithmName = stringConstant.get(); + return Arrays.stream(InsecureAlgorithm.values()) + .filter(alg -> alg.match(algorithmName)) + .findFirst(); + } + return Optional.empty(); + } } From 8b5350cd0e599ff3d6ed0458c6ee589342b7979a Mon Sep 17 00:00:00 2001 From: Luqmansonar Date: Wed, 30 Sep 2026 13:10:38 +0200 Subject: [PATCH 9/9] SONARJAVA-6438: Extract duplicated string literals into constants --- .../checks/security/DataHashingCheck.java | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java b/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java index deab387e5c5..2b6b28c277f 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/security/DataHashingCheck.java @@ -50,6 +50,11 @@ public class DataHashingCheck extends AbstractMethodDetection { private static final String GET_INSTANCE = "getInstance"; private static final String JAVA_LANG_STRING = "java.lang.String"; private static final String CONSTRUCTOR = ""; + private static final String COMMONS_DIGEST_UTILS = "org.apache.commons.codec.digest.DigestUtils"; + private static final String SPRING_DIGEST_UTILS = "org.springframework.util.DigestUtils"; + private static final String MD5_DIGEST = "md5Digest"; + private static final String MD5_DIGEST_AS_HEX = "md5DigestAsHex"; + private static final String APPEND_MD5_DIGEST_AS_HEX = "appendMd5DigestAsHex"; private static final Map ALGORITHM_BY_METHOD_NAME = MapBuilder.newMap() .put("getMd2Digest", InsecureAlgorithm.MD2) @@ -64,9 +69,9 @@ public class DataHashingCheck extends AbstractMethodDetection { .put("sha1Hex", InsecureAlgorithm.SHA1) .put("sha", InsecureAlgorithm.SHA1) .put("shaHex", InsecureAlgorithm.SHA1) - .put("md5Digest", InsecureAlgorithm.MD5) - .put("md5DigestAsHex", InsecureAlgorithm.MD5) - .put("appendMd5DigestAsHex", InsecureAlgorithm.MD5) + .put(MD5_DIGEST, InsecureAlgorithm.MD5) + .put(MD5_DIGEST_AS_HEX, InsecureAlgorithm.MD5) + .put(APPEND_MD5_DIGEST_AS_HEX, InsecureAlgorithm.MD5) .build(); /** @@ -168,13 +173,13 @@ private enum DeprecatedSpringPasswordEncoder { private static final MethodMatchers ONE_SHOT_DIGESTS = MethodMatchers.or( MethodMatchers.create() - .ofTypes("org.apache.commons.codec.digest.DigestUtils") + .ofTypes(COMMONS_DIGEST_UTILS) .names("md5", "md5Hex", "sha1", "sha1Hex", "sha", "shaHex") .withAnyParameters() .build(), MethodMatchers.create() - .ofTypes("org.springframework.util.DigestUtils") - .names("md5Digest", "md5DigestAsHex", "appendMd5DigestAsHex") + .ofTypes(SPRING_DIGEST_UTILS) + .names(MD5_DIGEST, MD5_DIGEST_AS_HEX, APPEND_MD5_DIGEST_AS_HEX) .withAnyParameters() .build()); @@ -185,7 +190,7 @@ private enum DeprecatedSpringPasswordEncoder { .withAnyParameters() .build(), MethodMatchers.create() - .ofTypes("org.apache.commons.codec.digest.DigestUtils") + .ofTypes(COMMONS_DIGEST_UTILS) .names("getDigest", "getMd5Digest", "getShaDigest", "getSha1Digest") .withAnyParameters() .build(), @@ -423,14 +428,14 @@ private static MethodMatchers getWeakHashMethodInvocationMatchers() { ArrayList matchers = new ArrayList<>(); matchers .add(MethodMatchers.create() - .ofTypes("org.apache.commons.codec.digest.DigestUtils") + .ofTypes(COMMONS_DIGEST_UTILS) .names("getDigest") .addParametersMatcher(JAVA_LANG_STRING) .build()); matchers .add(MethodMatchers.create() - .ofTypes("org.apache.commons.codec.digest.DigestUtils") + .ofTypes(COMMONS_DIGEST_UTILS) .name(ALGORITHM_BY_METHOD_NAME::containsKey) .withAnyParameters() .build()); @@ -445,8 +450,8 @@ private static MethodMatchers getWeakHashMethodInvocationMatchers() { matchers .add(MethodMatchers.create() - .ofTypes("org.springframework.util.DigestUtils") - .names("appendMd5DigestAsHex", "md5Digest", "md5DigestAsHex") + .ofTypes(SPRING_DIGEST_UTILS) + .names(APPEND_MD5_DIGEST_AS_HEX, MD5_DIGEST, MD5_DIGEST_AS_HEX) .withAnyParameters() .build());