Checkmarx report sql injection JpaRepository

Question

im running Checkmarx on my code and im getting an sql injection vulnerability. this is the simplified method im using

public String assignRole(String userId, String roleId) {

Optional<RoleEntity> roleEntity = roleRepository.findById(roleId)

if (roleEntity.isPresent()) {
  UserEntity user = UserEntity.builder().userId(userId).role(roleEntity.get()).build();
  userRepository.save(user);
  return "SUCCESS";
} else {
  throw new ServiceException("ERROR");
}}

and the analysis of checkmax says:

The application's assignRole method executes an SQL query with save, at line xx of src/Service.java. The application constructs this SQL query by embedding an untrusted string into the query without proper sanitization. The concatenated string is submitted to the database, where it is parsed and executed accordingly. An attacker would be able to inject arbitrary syntax and data into the SQL query, by crafting a malicious payload and providing it via the input roleId; This may enable an SQL Injection attack.

im a little bit confused because im not building a native query or concatenating strings in a query, or maybe im missing something.

any ideas for fix this? or maybe is a false positive.

Answer 1

This sounds like a false positive.

If your Java code uses Spring, I recommend configuring your scan (Settings > Scan Settings > Preset Manager) with the SQL_Injection and Second_Order_SQL_Injection items under the Java section disabled (unchecked) to avoid false positives from these items.

If your code persists data to the database exclusively via the Spring save action, it is not at risk of SQL injection exploitation. That’s because Spring saves (inserts or updates) to the database using an object mapped to your db (the ORM approach) which does not allow additional sql to be maliciously forced in.

Nonetheless, from what I’ve seen, Checkmarx marks Spring save actions, falsely, as high severity SQL Injection vulnerabilities (SQL_Injection and/or Second_Order_SQL_Injection). Given this, disabling those rules seems to me valid and in fact the only viable way around this.

But if you do take this approach, make sure your code does not for some reason also use some other some other sql approach such as a String containing SQL that’s not sanitized via the use of PreparedStatement. Those statements are vulnerable to sql injection exploits and I believe are what the Checkmarx SQL injection rules are intending to identify.