Skip to content

Commit 2b8782e

Browse files
SONARJAVA-4895: Factor out logic for detecting dynamically generated byte arrays into separate class
This will allow us to extend the logic in follow up commits so that user-defined safe factory methods are taken into account.
1 parent 8ba2de4 commit 2b8782e

File tree

1 file changed

+43
-40
lines changed

1 file changed

+43
-40
lines changed

java-checks/src/main/java/org/sonar/java/checks/security/CipherBlockChainingCheck.java

Lines changed: 43 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838

3939
@Rule(key = "S3329")
4040
public class CipherBlockChainingCheck extends AbstractMethodDetection {
41+
private final SecureByteArrayGeneratorDetector secureByteArrayGeneratorDetector = new SecureByteArrayGeneratorDetector();
4142

4243
private static final MethodMatchers SECURE_RANDOM_GENERATE_SEED = MethodMatchers.create()
4344
.ofTypes("java.security.SecureRandom")
@@ -54,7 +55,7 @@ protected MethodMatchers getMethodInvocationMatchers() {
5455

5556
@Override
5657
protected void onConstructorFound(NewClassTree newClassTree) {
57-
if (newClassTree.arguments().isEmpty() || isDynamicallyGenerated(newClassTree.arguments().get(0))) {
58+
if (newClassTree.arguments().isEmpty() || secureByteArrayGeneratorDetector.isDynamicallyGenerated(newClassTree.arguments().get(0))) {
5859
return;
5960
}
6061

@@ -72,45 +73,6 @@ protected void onConstructorFound(NewClassTree newClassTree) {
7273
}
7374
}
7475

75-
private static boolean isDynamicallyGenerated(ExpressionTree tree) {
76-
if (tree instanceof IdentifierTree identifierTree) {
77-
Symbol symbol = identifierTree.symbol();
78-
if (symbol.isParameter()) {
79-
return true;
80-
}
81-
}
82-
83-
return findConstructingMethods(tree)
84-
.anyMatch(SECURE_RANDOM_GENERATE_SEED::matches);
85-
}
86-
87-
private static Stream<MethodInvocationTree> findConstructingMethods(ExpressionTree expressionTree) {
88-
if (expressionTree instanceof MethodInvocationTree methodInvocationTree) {
89-
return Stream.of(methodInvocationTree);
90-
}
91-
92-
if (!(expressionTree instanceof IdentifierTree identifierTree) || !(identifierTree.symbol() instanceof Symbol.VariableSymbol variableSymbol)) {
93-
return Stream.empty();
94-
}
95-
96-
var declaration = variableSymbol.declaration();
97-
if (declaration == null) {
98-
return Stream.empty();
99-
}
100-
101-
var initializerStream = Stream.<MethodInvocationTree>of();
102-
if (declaration.initializer() instanceof MethodInvocationTree methodInvocationTree) {
103-
initializerStream = Stream.of(methodInvocationTree);
104-
}
105-
106-
var reassignments = getReassignments(declaration, variableSymbol.usages())
107-
.stream()
108-
.map(assignmentExpressionTree -> assignmentExpressionTree.expression() instanceof MethodInvocationTree methodInvocationTree ? methodInvocationTree : null)
109-
.filter(Objects::nonNull);
110-
111-
return Stream.concat(initializerStream, reassignments);
112-
}
113-
11476
private static class SecureInitializationFinder extends BaseTreeVisitor {
11577

11678
private boolean hasBeenSecurelyInitialized = false;
@@ -241,4 +203,45 @@ private static Symbol ivSymbol(NewClassTree newClassTree) {
241203
return Symbol.UNKNOWN_SYMBOL;
242204
}
243205
}
206+
207+
private static class SecureByteArrayGeneratorDetector {
208+
public boolean isDynamicallyGenerated(ExpressionTree tree) {
209+
if (tree instanceof IdentifierTree identifierTree) {
210+
Symbol symbol = identifierTree.symbol();
211+
if (symbol.isParameter()) {
212+
return true;
213+
}
214+
}
215+
216+
return findConstructingMethods(tree)
217+
.anyMatch(SECURE_RANDOM_GENERATE_SEED::matches);
218+
}
219+
220+
private static Stream<MethodInvocationTree> findConstructingMethods(ExpressionTree expressionTree) {
221+
if (expressionTree instanceof MethodInvocationTree methodInvocationTree) {
222+
return Stream.of(methodInvocationTree);
223+
}
224+
225+
if (!(expressionTree instanceof IdentifierTree identifierTree) || !(identifierTree.symbol() instanceof Symbol.VariableSymbol variableSymbol)) {
226+
return Stream.empty();
227+
}
228+
229+
var declaration = variableSymbol.declaration();
230+
if (declaration == null) {
231+
return Stream.empty();
232+
}
233+
234+
var initializerStream = Stream.<MethodInvocationTree>of();
235+
if (declaration.initializer() instanceof MethodInvocationTree methodInvocationTree) {
236+
initializerStream = Stream.of(methodInvocationTree);
237+
}
238+
239+
var reassignments = getReassignments(declaration, variableSymbol.usages())
240+
.stream()
241+
.map(assignmentExpressionTree -> assignmentExpressionTree.expression() instanceof MethodInvocationTree methodInvocationTree ? methodInvocationTree : null)
242+
.filter(Objects::nonNull);
243+
244+
return Stream.concat(initializerStream, reassignments);
245+
}
246+
}
244247
}

0 commit comments

Comments
 (0)