Security hotspot detection for java:S2077 produces Review Work for perectly safe code

Rules and Languages Report False-positive / False-negative...
1

  • What language is this for?
    java

  • Which rule?
    java:S2077

  • Why do you believe it’s a false-positive/false-negative?

    @Override
    public List fetchKeys(Long key) {
    String jpql =
    String.format(QueryConstants.KEY_JPQL_FORMAT, “:key”);
    return entityManager.createQuery(jpql, Long.class).setParameter(“key”, key)
    -->”Make sure using a dynamically formatted SQL query is safe here."
    .getResultList();
    }

  • Are you using

    • SonarQube Server - which version?
      Enterprise Edition - v2025.4.4 (119049)
    • How can we reproduce the problem? Give us a self-contained snippet of code (formatted text, no screenshots)
      • The rule detecting “Make sure using a dynamically formatted SQL query is safe here.“ is triggered by far too much, since it does not detect using constants, class names etc. Only using User provided input should trigger this Security Hotspot!
      • In our case ~90% of all detection are false positives because the rule logic is not intelligent enough
      • please use the above perfectly safe example to reproduce.
Sonar Community Roundup; Feb 21 - 27
4

Hello @patrik.jetzer,

Unfortunately we couldn’t reproduce the issue locally.

  • Could you provide us a specific query (QueryConstants.KEY_JPQL_FORMAT) that is used when raising the issue?
  • Could you provide us also the list of imports used in the class affected? Specially interested in type of persistence (EntityManager) that is used here.

Thanks in advance!

6

Hi, the requested Infos:

QueryConstants.KEY_JPQL_FORMAT

String KEY_JPQL_FORMAT = "SELECT order.key "
        + " FROM Order order "
        + " WHERE order.key = %1$s "
        + " AND order.assignmentSequence > 0";

Imports:

import javax.enterprise.context.ApplicationScoped;
import javax.inject.Inject;
import javax.persistence.EntityManager;
import javax.persistence.NoResultException;
import javax.persistence.NonUniqueResultException;
import javax.persistence.Parameter;
import javax.persistence.TypedQuery;

import org.eclipse.persistence.jpa.JpaQuery;

Remark: the false positives complain in cases like this refer to situations as shown in this example. Yes, we have a dynamically formatted sql/jpql query but none of the provided parameters is added to the dynamically formatted query where an sql injection can be triggered within the method. Since the Method Parameter “Long key“ is not part of the dynamical sql and also from the Type itself not vulnerable for SQL injection.

8

Hi @patrik.jetzer ,

Thank you for detailed response! Unfortunately it didn’t help to find the cause.

Could you please clarify one more thing? How is the query string (jpql) provided to the fetchKeys method? You added the following snippet:

Is it a real case or it’s provided say as a parameter of the method?

Thanks!

9

I have stripped our implementation to provide you the minimum example showing the Issue:

The following TestQueries class produces two Issues “java:S2077“ but in both of them in my understanding there is no SQL injection ever possible!

/*
 * -----------------------------------------------------------------------------
 * © Swisslog AG
 * Swisslog is not liable for any usage of this source code that is not agreed on between Swisslog and the other party.
 * The mandatory legal liability remains unaffected.
 * -----------------------------------------------------------------------------
 */
package com.sonar.hotspot.impl.stresstest;

import java.util.List;
import javax.enterprise.context.ApplicationScoped;
import javax.inject.Inject;
import javax.persistence.EntityManager;

/**
 * Queries methods to support stress tests.
 */
@ApplicationScoped
public class TestQueries {

    @Inject
    protected EntityManager em;

    static class QueryConstants {

        static Object[] var10001 = new Object[]{"NEW", "RELEASED"};
        public static final String FETCH_KEY_JPQL_FORMAT1 = "SELECT t.key FROM Transport t WHERE t.key != %1$s "
                + String.format(" AND t.status IN ('%1$s', '%2$s') ", var10001);

        private QueryConstants() {
        }
    }

    /**
     * Returns a list of Keys.
     */
    public List<Long> fetchKeys(Long key) {
        String jpql = String.format(QueryConstants.FETCH_KEY_JPQL_FORMAT1, ":key");
        return em.createQuery(jpql, Long.class).setParameter("key", key).getResultList();
    }

    /**
     * Returns a list of Keys, query class.
     */
    public List<Long> fetchKeysExt(Long key) {
        String jpql = "SELECT t.key FROM Transport t WHERE t.type = "+ QueryConstants.class;
        return em.createQuery(jpql, Long.class).getResultList();
    }

}

Issue 1: jpql is not vulnerable sincy only constant values are added

image
image992×726 48.8 KB

Issue 2: jpql is not vulnerable sincy only constant values are added

image
image974×766 51.1 KB

10

Hi @patrik.jetzer ,

at first, thanks a lot for your detailed answer! Now we could reproduce the issues.

It’s a great findings. We’ve created a JIRA ticket SONARJAVA-6158 with related details and plan working on it.

For a while, I can suggest simple fixes for these cases:

For the first one:

static String FETCH_KEY_JPQL_FORMAT1 = "SELECT t.key FROM Transport t WHERE t.key != %1$s " 
        + " AND t.status IN ('NEW', 'RELEASED') ";

For the second one:

private static final String CLASS_NAME = "class <package>.QueryConstants";
...
String jpql = "SELECT t.key FROM Transport t WHERE t.type = " + CLASS_NAME;

Hope it helps!

12

Thank you for the response,

Of course you are correct and these two examples could be changed to not raise a Hotspot Issue. Nevertheless it creates significant effort to change the code to just get around the False Positive finding. Code is perfectly safe to use!
In my case it is easier to mark the 100 occurences as Safe than to implement a workaround fix.
Looking forward to a more intelligent sql injection detection, thank you.