diff --git a/java-checks-test-sources/default/src/main/java/checks/SQLInjection.java b/java-checks-test-sources/default/src/main/java/checks/SQLInjection.java index 98b6b497dba..ad953a62733 100644 --- a/java-checks-test-sources/default/src/main/java/checks/SQLInjection.java +++ b/java-checks-test-sources/default/src/main/java/checks/SQLInjection.java @@ -187,7 +187,7 @@ private static void makeQuery(Connection p_con) { PersistenceManager pm; void jdo(int id, String name) { - javax.jdo.Query q = pm.newQuery(Test.class, id + " > query_id "); // Noncompliant + javax.jdo.Query q = pm.newQuery(Test.class, id + " > query_id "); // Compliant q.setFilter("name == " + name); // Noncompliant } @@ -391,4 +391,15 @@ public void plusAssignmentClassName(String input) throws SQLException { query3 += SQLInjection.class.getName() + input; this.stmt.execute(query3); // Noncompliant } + + public void constantTernaryAndPrimitives(boolean asc, int offset, Integer limit, String input, EntityManager em) { + em.createNativeQuery("SELECT * FROM entity ORDER BY x " + (asc ? "ASC" : "DESC")); // Compliant + em.createNativeQuery("SELECT * FROM entity OFFSET " + offset + " LIMIT " + limit); // Compliant + String query = "SELECT * FROM entity OFFSET " + offset; + query += " LIMIT " + limit; + em.createNativeQuery(query); // Compliant + em.createNativeQuery("SELECT * FROM entity ORDER BY " + (asc ? "ASC" : input)); // Noncompliant + em.createNativeQuery("SELECT * FROM entity ORDER BY " + (asc ? input : "DESC")); // Noncompliant + em.createNativeQuery("SELECT * FROM entity ORDER BY " + (asc ? (offset > 0 ? "A" : "B") : "C")); // Compliant + } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/SQLInjectionCheck.java b/java-checks/src/main/java/org/sonar/java/checks/SQLInjectionCheck.java index c5af4692578..88e4ed08291 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/SQLInjectionCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/SQLInjectionCheck.java @@ -35,6 +35,7 @@ import org.sonar.plugins.java.api.semantic.Type; import org.sonar.plugins.java.api.tree.AssignmentExpressionTree; import org.sonar.plugins.java.api.tree.BinaryExpressionTree; +import org.sonar.plugins.java.api.tree.ConditionalExpressionTree; import org.sonar.plugins.java.api.tree.ExpressionTree; import org.sonar.plugins.java.api.tree.IdentifierTree; import org.sonar.plugins.java.api.tree.MethodInvocationTree; @@ -226,15 +227,22 @@ private static boolean isDynamicConcatenation(ExpressionTree arg) { private static boolean hasDynamicConcatenationOperand(ExpressionTree arg) { ExpressionTree expression = ExpressionUtils.skipParentheses(arg); - if (expression.asConstant().isPresent() || isSafeClassNameExpression(expression, new HashSet<>())) { + if (expression.asConstant().isPresent() || isPrimitiveOrWrapper(expression.symbolType()) || isSafeClassNameExpression(expression, new HashSet<>())) { return false; } + if (expression instanceof ConditionalExpressionTree conditional) { + return hasDynamicConcatenationOperand(conditional.trueExpression()) || hasDynamicConcatenationOperand(conditional.falseExpression()); + } if (expression instanceof BinaryExpressionTree binaryExpression && expression.is(Tree.Kind.PLUS)) { return hasDynamicConcatenationOperand(binaryExpression.leftOperand()) || hasDynamicConcatenationOperand(binaryExpression.rightOperand()); } return true; } + private static boolean isPrimitiveOrWrapper(Type type) { + return type.isPrimitive() || type.isPrimitiveWrapper(); + } + private static boolean isSafeClassNameExpression(ExpressionTree arg, Set visitedSymbols) { ExpressionTree expression = ExpressionUtils.skipParentheses(arg); if (expression instanceof MethodInvocationTree methodInvocation) { @@ -266,7 +274,7 @@ private static boolean hasDynamicStringParameters(MethodInvocationTree mit) { // `format` has a variant with Locale as the first argument - we do not need to check that parameter. boolean isFirstLocaleArgument = firstArg && type.is("java.util.Locale"); // Primitives will not lead to SQL injection, so the code is compliant. - if (!isFirstLocaleArgument && !type.isUnknown() && !type.isPrimitive() && !type.isPrimitiveWrapper() + if (!isFirstLocaleArgument && !type.isUnknown() && !isPrimitiveOrWrapper(type) && arg.asConstant().isEmpty() && !isSafeClassNameExpression(arg, new HashSet<>())) { return true; }