3

I have a Springboot application which uses Spring Data JPA module for database operations. When we scan the code, checkmarx is reporting lot of high&medium rated issues w.r.t SQL_Injection attacks. Following is one of the use cases, I need help in whether to mark the issue as False-Positive or not. If it is NOT False-Positive what should I do to fix the issue.?

AppController.Java

@Controller
public class AppController
{
    private static final Logger logger = LoggerFactory.getLogger(AppController.class);

    @Autowired
    private AppService appService;
    
    @RequestMapping(value = "/propertiesHistory", method = RequestMethod.POST)
    public String getPropertiesHistory(@ModelAttribute("propSearchForm") @Validated PropertiesSearch propertiesSearch, BindingResult result, Model model, final RedirectAttributes redirectAttributes)
    {
        String instanceName = propertiesSearch.getInstanceName();
        
        if (!propertiesSearch.getInstanceName().equalsIgnoreCase("NONE"))
        {
            List<String> propVersionDates = appService.getPropertyHistoryDates(instanceName);
            //Some Businees Logic
        }

        if (result.hasErrors())
        {
            logger.warn("getPropertiesHistory() : Binding error - " + result.getAllErrors());
        }
        else
        {
            //Some Businees Logic
        }
        return "app/prophist";
    }
}

AppService.java

@Service
public class AppService
{
    private static final Logger logger = LoggerFactory.getLogger(AppService.class);
    
    @Autowired
    private AppRepository appRepository;
    
    public List<String> getPropertyHistoryDates(String instanceName)
    {
        List<String> list = new ArrayList<String>();
        try
        {
            list = appRepository.findAllMDateDESCByProNotEmptyAndInstanceName(instanceName);
        }
        catch (Exception e)
        {
            logger.error("getPropertyHistoryDates(): Error while fetching data from database - ", e);
        }
        
        return list;
    }
}

AppRepository.java

public interface AppRepository extends JpaRepository<AppDataEntity, Long>
{
    @Query(value="SELECT mdate FROM tablexyz WHERE properties IS NOT NULL AND instanceName =:instanceName ORDER BY mdate DESC",nativeQuery=true)
    List<String> findAllMDateDESCByProNotEmptyAndInstanceName(@Param("instanceName") String instanceName);
}

I also have some methods like List<AppDataEntity> findAllByInstanceName(String instanceName); in the repository which makes use of Proxy class implementation but not the native query. In such cases also I am getting this Checkmarx issue - SQL_Injection.

I read that Spring Data doesn't change the way Hibernate works with entities as per the accepted answer here. Is it true and applicable for @Query(value="some query",nativeQuery=true).?

Ravi
  • 1,614
  • 4
  • 14
  • 27
  • 2
    I'm pretty sure that Spring Boot will use `PreparedStatement` for native queries as well so your query should be safe from SQL injection. Nevertheless it might be a good idea to dig a little in order to understand how those kind of attacks work and how/why `PreparedStatement` should take care of them. – Thomas Sep 06 '22 at 06:54
  • 2
    This is an almost dup to https://stackoverflow.com/questions/52791121/is-java-spring-jpa-native-query-sql-injection-proof Don't want to close vote, since that would close it immediately. – Jens Schauder Sep 06 '22 at 07:11
  • @JensSchauder, Thanks for not marking this question as duplicate even though it is almost same that I'm also looking for. I have read your answer (which is accepted by the owner). So the bottom line is that 'Spring Data JPA is secure against SQL injection.' no matter whether is native query or JPQL right?. If so, I'll write answer by myself by referring to your answer. Is that okay for you? – Ravi Sep 06 '22 at 07:24
  • 1
    Yes, that is correct. And sure go ahead. – Jens Schauder Sep 06 '22 at 07:44
  • @JensSchauder, as **Thomas** said it is great if I get any article on how PreparedStatement is taking care of SQL_Injection attacks. Do you have any info or article about it?. – Ravi Sep 06 '22 at 07:56
  • 1
    Nope. I just know that bind variables are send separately to the database as you can tell when you look at the statements run by your database. You'll find that it executes sql statements that still have the bind variables in them. – Jens Schauder Sep 06 '22 at 08:05
  • @JensSchauder: I found the very convincing logical reason here (https://stackoverflow.com/a/8265319/9145082) – Ravi Sep 06 '22 at 08:57
  • @RaviKumarB, can you share the version of the Checkmarx SAST you are using? – yaloner Sep 06 '22 at 10:15
  • @yaloner: Version 9.4.2 HF4 – Ravi Sep 06 '22 at 10:17
  • @Ravi, this issue was handled in Checkmarx SAST 9.4.4 – yaloner Sep 12 '22 at 18:22

2 Answers2

4

SpringBoot Data JPA Repository is SAFE against SQL_Injection attacks as long as if we have named or indexed/positional parameters in @Query(JPQL) or @Query(nativeQuery).

Yes, for the below question. It is applicable for @Query() only condition is that the query should have either named (:paramname) or positional (?1) parameters.

I read that Spring Data doesn't change the way Hibernate works with entities as per the accepted answer here. Is it true and applicable for @Query(value="some query",nativeQuery=true).?

The following is SAFE against sql injection.

@Query(value="SELECT mdate FROM tablexyz WHERE properties IS NOT NULL AND instanceName =:instanceName ORDER BY mdate DESC",nativeQuery=true)
    List<String> findAllMDateDESCByProNotEmptyAndInstanceName(@Param("instanceName") String instanceName);

Simple reason is that Spring Data JPA uses hibernate behind the scenes which in turn uses PreparedStatements way to deal with database.

A very detailed explanation on how PreparedStatements way is protecting our application from SQL_Injection attacks can be found here and here as well

Ravi
  • 1,614
  • 4
  • 14
  • 27
  • 1
    The example you claim is not safe against SQL injection is extremely safe, since it doesn't compile and therefore can't be executed at all except when `instanceName` is a compile time constant, which of course would make it again perfectly safe against SQL injection. – Jens Schauder Sep 06 '22 at 11:17
2

As long as you use placeholders (:instanceName) instead of custom SQL with appended parameters your SQLs are not vulnerable for SQL injection.

Most probably you will be using Hibernate implementation underneath Spring Data JPA interfaces so it is taken care as PreparedStatements are used.

I have experience running Checkmarx on some of the repositories I used to code and it gives a lot of False Positives when it comes to SQL Injection and XSS in REST APIs. The reason being it can not see context or libraries used within the project.

It reported a lot of SQL Injection vulnerabilities for us even though we have used Spring JDBC Template which uses PreparedStatements heavily.

shazin
  • 21,379
  • 3
  • 54
  • 71