False positive Spring jdbcTemplate SQL Injection

[ZPetrovich]

Describe the bug [Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')] when using PreparedStatementSetter. Due to the use of PreparedStatement it's a false positive error.

Please note, this was already fixed in corresponding find-sec-bugs plugin, however not sure whether that plugin somehow related to Semgrep Project: https://github.com/find-sec-bugs/find-sec-bugs/issues/538

To Reproduce

  private static final String SQL_STATEMENT = "SELECT *
            "        FROM MyTable t" +
            "        WHERE t.id = ?";

  @Autowired
  private JdbcTemplate jdbcTemplate;

  public WebHookConfiguration findById(Long id) {
       List<WebHookConfiguration> configurationList = jdbcTemplate.query(
                SQL_STATEMENT,
                new PreparedStatementSetter() {
                    @Override
                    public void setValues(PreparedStatement preparedStatement) throws SQLException {
                        preparedStatement.setLong(1, id);
                    }
                },
                new WebhookRowMapper()
        );
..............

Expected behavior Do not signal about SQL Injection vulnerability

What is the priority of the bug to you?

• [ ] P0: blocking your adoption of Semgrep or workflow
• [x] P1: important to fix or quite annoying
• [ ] P2: regular bug that should get fixed

Environment Run as part of GitLab Vulnerability Report

$ /analyzer run
[INFO] [Semgrep] [2022-10-13T17:00:00Z] ▶ GitLab Semgrep analyzer v3.7.3
[INFO] [Semgrep] [2022-10-13T17:00:00Z] ▶ Detecting project
[INFO] [Semgrep] [2022-10-13T17:00:00Z] ▶ Analyzer will attempt to analyze all projects in the repository
[INFO] [Semgrep] [2022-10-13T17:00:00Z] ▶ Running analyzer
[INFO] [Semgrep] [2022-10-13T17:00:16Z] ▶ Creating report

Use case Ability to remove some False Positive issues from Vulnerability Report on GitLab

[r2c-demo]

This issue is synced in Linear at https://linear.app/r2c/issue/PA-2018/false-positive-spring-jdbctemplate-sql-injection. Note: this link is for r2c use only and is not accessible publicly.

[ievans]

Thanks @ZPetrovich , I transferred this issue to our repository of rules for Semgrep!

[0xDC0DE]

Thanks for reporting @ZPetrovich , I am taking a look!

[0xDC0DE]

I can't seem to reproduce this. I am scanning your example code with all our java rules and it results in 0 findings. Could you point me to the exact rule that is firing for this piece of code on your end? @ZPetrovich

[ZPetrovich]

@pieterdc1 many thanks for your time! Please note, we are not running code analyzers just like you did - from command line. We have GitLab integration with Semgrep (see version and log messages in my initial post). Here is report we got from code I have provided:

{
      "id": "26e20b1c46ae7d96125c7a60a28b2e99080cdfce40e2a91cf2c7615829287f6a",
      "category": "sast",
      "message": "Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')",
      "description": "The input values included in SQL queries need to be passed in safely. Bind\nvariables in prepared statements can be used to easily mitigate the risk of\nSQL injection.\n",
      "cve": "",
      "severity": "Critical",
      "scanner": {
        "id": "semgrep",
        "name": "Semgrep"
      },
      "location": {
        "file": "<PATH_NOT_DISCLOSED>/service/webhook/dao/WebHookConfigurationDAO.java",
        "start_line": 42,
        "end_line": 46
      },
      "identifiers": [
        {
          "type": "semgrep_id",
          "name": "find_sec_bugs.SQL_INJECTION_SPRING_JDBC-1.SQL_INJECTION_JPA-1.SQL_INJECTION_JDO-1.SQL_INJECTION_JDBC-1.SQL_NONCONSTANT_STRING_PASSED_TO_EXECUTE-1",
          "value": "find_sec_bugs.SQL_INJECTION_SPRING_JDBC-1.SQL_INJECTION_JPA-1.SQL_INJECTION_JDO-1.SQL_INJECTION_JDBC-1.SQL_NONCONSTANT_STRING_PASSED_TO_EXECUTE-1"
        },
        {
          "type": "cwe",
          "name": "CWE-89",
          "value": "89",
          "url": "https://cwe.mitre.org/data/definitions/89.html"
        },
        {
          "type": "find_sec_bugs_type",
          "name": "Find Security Bugs-SQL_INJECTION_SPRING_JDBC",
          "value": "SQL_INJECTION_SPRING_JDBC"
        },
        {
          "type": "find_sec_bugs_type",
          "name": "Find Security Bugs-SQL_INJECTION_JPA",
          "value": "SQL_INJECTION_JPA"
        },
        {
          "type": "find_sec_bugs_type",
          "name": "Find Security Bugs-SQL_INJECTION_JDO",
          "value": "SQL_INJECTION_JDO"
        },
        {
          "type": "find_sec_bugs_type",
          "name": "Find Security Bugs-SQL_INJECTION_JDBC",
          "value": "SQL_INJECTION_JDBC"
        },
        {
          "type": "find_sec_bugs_type",
          "name": "Find Security Bugs-SQL_NONCONSTANT_STRING_PASSED_TO_EXECUTE",
          "value": "SQL_NONCONSTANT_STRING_PASSED_TO_EXECUTE"
        }
      ],
      "tracking": {
        "type": "source",
        "items": [
          {
            "file": "<PATH_NOT_DISCLOSED>/service/webhook/dao/WebHookConfigurationDAO.java",
            "line_start": 42,
            "line_end": 42,
            "signatures": [
              {
                "algorithm": "scope_offset",
                "value": "<PATH_NOT_DISCLOSED>/service/webhook/dao/WebHookConfigurationDAO.java|WebHookConfigurationDAO[0]|WebHookConfiguration[0]:1"
              }
            ]
          }
        ]
      }
    },

[0xDC0DE]

Ah! This rule is actually maintained by GitLab: https://gitlab.com/gitlab-org/security-products/analyzers/semgrep/-/blob/main/rules/find_sec_bugs.yml#L1613 You might want to file an issue there instead.

[connorg]

Hi folks! GitLab PM for Static Analysis here. Just wanted to apologize for the confusion here. We're working on some improvements to documentation + product experience to try to clarify where issues like this should be reported.

I also wanted to share that our Vulnerability Research team has merged a related fix.