Skip to content

Static security scanner false positive: injection/sql-concat matches comments and non-SQL concatenation #485

Description

@ajianaz

Problem

The injection/sql-concat security pattern uses an overly broad regex that produces false positives on comments, documentation, and non-SQL string concatenation.

Current Regex

regex: r"(?i)(?:SELECT|INSERT|UPDATE|DELETE)\s+.*\+",

This matches ANY line containing a SQL keyword (case-insensitive) followed by + anywhere after it — including comments, prose, and unrelated code.

False Positive Examples

Input Why it matches Why it should not
// SELECT a + b FROM joined SELECT + + Comment, not actual SQL injection
# SELECT all users: id + name SELECT + + Documentation/prose
// INSERT: column_a + column_b INSERT + + Comment describing insert logic
"UPDATE total = count + 1" UPDATE + + String literal, not dynamic SQL

Root Cause

Same architectural issue as #483 (CORS wildcard false positive):

  1. Regex matches keywords without requiring actual code/assignment context
  2. No file-type filtering — .md files are scanned
  3. No post-match negation filter for comment context (//, #, --, /*)

Suggested Fix (Same 3-layer approach as #483)

  1. Narrow regex: Require SQL string context ("SELECT ... " + or format!("SELECT ...") +)
  2. Post-match filter: Skip lines starting with comment markers (//, #, --, /*, *)
  3. Leverage existing .md skip: Already implemented via is_doc_file() in fix(scanner): narrow CORS wildcard regex + skip doc files + negation filter #484

Reproduction

echo "// SELECT a + b" > test.rs
cora review --format sarif  # Reports injection/sql-concat on test.rs

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions