Skip to content

Commit 38a69d4

Browse files
irina-batinic-sonarsourceirina-batinic-sonarsource
authored
SONARJAVA-4866 S2077: Support detection of formatted SQL queries in Spring (#4750)
1 parent f7286e5 commit 38a69d4

6 files changed

Lines changed: 135 additions & 10 deletions

File tree

‎its/autoscan/src/test/resources/autoscan/diffs/diff_S2077.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"ruleKey": "S2077",
33
"hasTruePositives": true,
4-
"falseNegatives": 10,
4+
"falseNegatives": 47,
55
"falsePositives": 0
6-
}
6+
}

‎its/autoscan/src/test/resources/autoscan/diffs/diff_S3740.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"ruleKey": "S3740",
33
"hasTruePositives": true,
4-
"falseNegatives": 50,
4+
"falseNegatives": 51,
55
"falsePositives": 0
6-
}
6+
}

‎java-checks-test-sources/spring-3.2/pom.xml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,16 @@
5858
<version>6.2.3</version>
5959
<scope>provided</scope>
6060
</dependency>
61+
<dependency>
62+
<groupId>org.springframework.security</groupId>
63+
<artifactId>spring-security-core</artifactId>
64+
<version>6.2.3</version>
65+
</dependency>
66+
<dependency>
67+
<groupId>org.springframework</groupId>
68+
<artifactId>spring-jdbc</artifactId>
69+
<version>6.1.5</version>
70+
</dependency>
6171
</dependencies>
6272

6373
<profiles>

‎java-checks-test-sources/spring-3.2/src/main/java/checks/SQLInjectionCheckSample.java‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
package checks;
2+
3+
import org.springframework.jdbc.core.JdbcOperations;
4+
import org.springframework.jdbc.core.JdbcTemplate;
5+
import org.springframework.jdbc.core.PreparedStatementCallback;
6+
import org.springframework.jdbc.core.RowMapper;
7+
import org.springframework.jdbc.core.RowMapperResultSetExtractor;
8+
import org.springframework.jdbc.core.namedparam.BeanPropertySqlParameterSource;
9+
import org.springframework.jdbc.core.namedparam.EmptySqlParameterSource;
10+
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
11+
import org.springframework.jdbc.core.namedparam.SqlParameterSource;
12+
import org.springframework.jdbc.core.simple.JdbcClient;
13+
import org.springframework.security.core.userdetails.jdbc.JdbcDaoImpl;
14+
import org.springframework.security.provisioning.JdbcUserDetailsManager;
15+
16+
public class SQLInjectionCheckSample {
17+
18+
void testJdbcClient(String input, JdbcClient jdbcClient) {
19+
jdbcClient.sql("SELECT " + input).query(); // Noncompliant
20+
}
21+
22+
void testJdbcDao(String input, JdbcDaoImpl jdbcDao) {
23+
jdbcDao.setAuthoritiesByUsernameQuery("SELECT " + input); // Noncompliant
24+
jdbcDao.setGroupAuthoritiesByUsernameQuery("SELECT " + input); // Noncompliant
25+
jdbcDao.setUsersByUsernameQuery("SELECT " + input); // Noncompliant
26+
}
27+
28+
void testJdbcOperations(String input, JdbcOperations jdbcOperations, RowMapper<String> rowMapper) {
29+
jdbcOperations.execute("SELECT " + input); // Noncompliant
30+
jdbcOperations.queryForStream("SELECT " + input, rowMapper); // Noncompliant
31+
}
32+
33+
void testJdbcTemplate(String input, JdbcTemplate jdbcTemplate, RowMapper<String> rowMapper) {
34+
jdbcTemplate.execute("SELECT " + input); // Noncompliant
35+
jdbcTemplate.queryForStream("SELECT " + input, rowMapper); // Noncompliant
36+
}
37+
38+
void testNamedParameterJdbcTemplate(String input, NamedParameterJdbcTemplate namedParameterJdbcTemplate, RowMapper<String> rowMapper,
39+
PreparedStatementCallback<String> preparedStatementCallback) {
40+
namedParameterJdbcTemplate.batchUpdate("SELECT " + input, new SqlParameterSource[1]); // Noncompliant
41+
namedParameterJdbcTemplate.execute("SELECT " + input, preparedStatementCallback); // Noncompliant
42+
namedParameterJdbcTemplate.query("SELECT " + input, new BeanPropertySqlParameterSource(new Object()), new RowMapperResultSetExtractor(rowMapper)); // Noncompliant
43+
namedParameterJdbcTemplate.queryForList("SELECT " + input, EmptySqlParameterSource.INSTANCE); // Noncompliant
44+
namedParameterJdbcTemplate.queryForMap("SELECT " + input, EmptySqlParameterSource.INSTANCE); // Noncompliant
45+
namedParameterJdbcTemplate.queryForObject("SELECT " + input, EmptySqlParameterSource.INSTANCE, rowMapper); // Noncompliant
46+
namedParameterJdbcTemplate.queryForRowSet("SELECT " + input, EmptySqlParameterSource.INSTANCE); // Noncompliant
47+
namedParameterJdbcTemplate.queryForStream("SELECT " + input, EmptySqlParameterSource.INSTANCE, rowMapper); // Noncompliant
48+
namedParameterJdbcTemplate.update("SELECT " + input, EmptySqlParameterSource.INSTANCE); // Noncompliant
49+
}
50+
51+
void testJdbcUserDetailsManager(String input) {
52+
JdbcUserDetailsManager jdbcUserDetailsManager = new JdbcUserDetailsManager();
53+
jdbcUserDetailsManager.setChangePasswordSql("SELECT " + input); // Noncompliant
54+
jdbcUserDetailsManager.setCreateAuthoritySql("SELECT " + input); // Noncompliant
55+
jdbcUserDetailsManager.setCreateUserSql("SELECT " + input); // Noncompliant
56+
jdbcUserDetailsManager.setDeleteGroupAuthoritiesSql("SELECT " + input); // Noncompliant
57+
jdbcUserDetailsManager.setDeleteGroupAuthoritySql("SELECT " + input); // Noncompliant
58+
jdbcUserDetailsManager.setDeleteGroupMemberSql("SELECT " + input); // Noncompliant
59+
jdbcUserDetailsManager.setDeleteGroupMembersSql("SELECT " + input); // Noncompliant
60+
jdbcUserDetailsManager.setDeleteGroupSql("SELECT " + input); // Noncompliant
61+
jdbcUserDetailsManager.setDeleteUserAuthoritiesSql("SELECT " + input); // Noncompliant
62+
jdbcUserDetailsManager.setDeleteUserSql("SELECT " + input); // Noncompliant
63+
jdbcUserDetailsManager.setFindAllGroupsSql("SELECT " + input); // Noncompliant
64+
jdbcUserDetailsManager.setFindGroupIdSql("SELECT " + input); // Noncompliant
65+
jdbcUserDetailsManager.setFindUsersInGroupSql("SELECT " + input); // Noncompliant
66+
jdbcUserDetailsManager.setGroupAuthoritiesSql("SELECT " + input); // Noncompliant
67+
jdbcUserDetailsManager.setInsertGroupAuthoritySql("SELECT " + input); // Noncompliant
68+
jdbcUserDetailsManager.setInsertGroupMemberSql("SELECT " + input); // Noncompliant
69+
jdbcUserDetailsManager.setInsertGroupSql("SELECT " + input); // Noncompliant
70+
jdbcUserDetailsManager.setRenameGroupSql("SELECT " + input); // Noncompliant
71+
jdbcUserDetailsManager.setUpdateUserSql("SELECT " + input); // Noncompliant
72+
jdbcUserDetailsManager.setUserExistsSql("SELECT " + input); // Noncompliant
73+
}
74+
75+
}

‎java-checks/src/main/java/org/sonar/java/checks/SQLInjectionCheck.java‎

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919
*/
2020
package org.sonar.java.checks;
2121

22-
import java.util.Arrays;
2322
import java.util.ArrayList;
23+
import java.util.Arrays;
2424
import java.util.List;
2525
import java.util.Optional;
2626
import java.util.stream.Collectors;
@@ -70,9 +70,9 @@ public class SQLInjectionCheck extends IssuableSubscriptionVisitor {
7070
.names("createNativeQuery", "createQuery")
7171
.withAnyParameters()
7272
.build(),
73-
MethodMatchers.create().ofSubTypes(SPRING_JDBC_OPERATIONS)
73+
MethodMatchers.create().ofSubTypes(SPRING_JDBC_OPERATIONS, "org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate")
7474
.names("batchUpdate", "execute", "query", "queryForList", "queryForMap", "queryForObject",
75-
"queryForRowSet", "queryForInt", "queryForLong", "update")
75+
"queryForRowSet", "queryForInt", "queryForLong", "update", "queryForStream")
7676
.withAnyParameters()
7777
.build(),
7878
MethodMatchers.create()
@@ -89,8 +89,32 @@ public class SQLInjectionCheck extends IssuableSubscriptionVisitor {
8989
.ofSubTypes("javax.jdo.Query")
9090
.names("setFilter", "setGrouping")
9191
.withAnyParameters()
92+
.build(),
93+
MethodMatchers.create()
94+
.ofSubTypes("org.springframework.security.core.userdetails.jdbc.JdbcDaoImpl")
95+
.names("setAuthoritiesByUsernameQuery", "setGroupAuthoritiesByUsernameQuery", "setUsersByUsernameQuery")
96+
.withAnyParameters()
97+
.build(),
98+
MethodMatchers.create()
99+
.ofSubTypes("org.springframework.security.provisioning.JdbcUserDetailsManager")
100+
.names("setChangePasswordSql", "setCreateAuthoritySql", "setCreateUserSql", "setDeleteGroupAuthoritiesSql",
101+
"setDeleteGroupAuthoritySql", "setDeleteGroupMemberSql", "setDeleteGroupMembersSql", "setDeleteGroupSql",
102+
"setDeleteUserAuthoritiesSql", "setDeleteUserSql", "setFindAllGroupsSql", "setFindGroupIdSql", "setFindUsersInGroupSql",
103+
"setGroupAuthoritiesSql", "setInsertGroupAuthoritySql", "setInsertGroupMemberSql", "setInsertGroupSql", "setRenameGroupSql",
104+
"setUpdateUserSql", "setUserExistsSql")
105+
.withAnyParameters()
106+
.build(),
107+
MethodMatchers.create()
108+
.ofSubTypes("org.springframework.jdbc.core.simple.JdbcClient")
109+
.names("sql")
110+
.withAnyParameters()
111+
.build(),
112+
MethodMatchers.create()
113+
.ofTypes("org.springframework.data.r2dbc.repository.query.StringBasedR2dbcQuery")
114+
.names(CONSTRUCTOR)
115+
.withAnyParameters()
92116
.build());
93-
117+
94118
private static final String MAIN_MESSAGE = "Make sure using a dynamically formatted SQL query is safe here.";
95119

96120
@Override
@@ -128,7 +152,8 @@ private static List<JavaFileScannerContext.Location> secondaryLocations(@Nullabl
128152
List<AssignmentExpressionTree> reassignments,
129153
String identifierName) {
130154
List<JavaFileScannerContext.Location> secondaryLocations = reassignments.stream()
131-
.map(assignment -> new JavaFileScannerContext.Location(String.format("SQL Query is assigned to '%s'", getVariableName(assignment)), assignment.expression()))
155+
.map(assignment -> new JavaFileScannerContext.Location(String.format("SQL Query is assigned to '%s'", getVariableName(assignment)),
156+
assignment.expression()))
132157
.collect(Collectors.toCollection(ArrayList::new));
133158

134159
if (initializerOrExpression != null) {
@@ -138,7 +163,7 @@ private static List<JavaFileScannerContext.Location> secondaryLocations(@Nullabl
138163
}
139164
return secondaryLocations;
140165
}
141-
166+
142167
private static String getVariableName(AssignmentExpressionTree assignment) {
143168
ExpressionTree variable = assignment.variable();
144169
return ((IdentifierTree) variable).name();

‎java-checks/src/test/java/org/sonar/java/checks/SQLInjectionCheckTest.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,19 @@
1919
*/
2020
package org.sonar.java.checks;
2121

22+
import java.io.File;
23+
import java.util.List;
2224
import org.junit.jupiter.api.Test;
2325
import org.sonar.java.checks.verifier.CheckVerifier;
26+
import org.sonar.java.test.classpath.TestClasspathUtils;
2427

2528
import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
29+
import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPathInModule;
2630

2731
class SQLInjectionCheckTest {
2832

33+
private static final List<File> SPRING_3_2_CLASSPATH = TestClasspathUtils.loadFromFile(Constants.SPRING_3_2_CLASSPATH);
34+
2935
@Test
3036
void test() {
3137
CheckVerifier.newVerifier()
@@ -34,4 +40,13 @@ void test() {
3440
.verifyIssues();
3541
}
3642

43+
@Test
44+
void test_with_spring_3_2() {
45+
CheckVerifier.newVerifier()
46+
.onFile(mainCodeSourcesPathInModule(Constants.SPRING_3_2, "checks/SQLInjectionCheckSample.java"))
47+
.withCheck(new SQLInjectionCheck())
48+
.withClassPath(SPRING_3_2_CLASSPATH)
49+
.verifyIssues();
50+
}
51+
3752
}

0 commit comments

Comments
 (0)