What we found

• AppSecAI Vulnerability IDs: 69c734eb (2 instances)
• Vulnerability: CWE-89: SQL Injection
• Severity: Medium
• File: src/main/java/org/owasp/benchmark/testcode/BenchmarkTest02369.java:65,67
• Detected By: OpenGrep
• Detection Rules:
  • Tainted Sql From Http Request (line 65)
  • JDBC SQL Injection (line 67)

Description: User-controlled input from HTTPServletRequest flows directly into a SQL INSERT statement via string concatenation. Both the SQL template construction (line 65) and statement execution (line 67) lack parameterization, creating a direct SQL injection sink.

Why this matters

Risk if not fixed: An attacker could read, modify, or delete database contents, bypass authentication, execute administrative operations, or in some cases execute operating system commands through database-level functionality.

Risk level: Critical — Direct database access with INSERT-level privileges

Attack surface: HTTP request parameter names are attacker-controlled and flow directly into SQL without sanitization.

Why we're changing it

Status: Confirmed vulnerability

Data Flow Analysis:

The arithmetic in doSomething() at line 85 is deterministic: (7 * 42) - 106 = 188, which is not > 200, so the ternary always returns param — never the hardcoded string. Therefore bar = param unconditionally.

param is set to the HTTP request parameter name (not value) when a parameter's value equals "BenchmarkTest02369". An attacker controls parameter names, so a request like:

POST /sqli-05/BenchmarkTest02369
malicious'; DROP TABLE users;--=BenchmarkTest02369

sets param = "malicious'; DROP TABLE users;--", which flows directly into:

String sql = "INSERT INTO users (username, password) VALUES ('foo','" + bar + "')";
statement.executeUpdate(sql, new int[] {1, 2});

No sanitization, escaping, or parameterization exists anywhere in this path. The attacker-controlled string is concatenated into the SQL template at line 65 and executed verbatim at line 67.

Evidence:

• Line 53: param = name — assigns HTTP parameter name (attacker-controlled) to param
• Line 85: bar = (7 * 42) - num > 200 ? ... : param — evaluates to param always (188 not > 200)
• Line 65: String sql = "INSERT INTO users (username, password) VALUES ('foo','" + bar + "')" — direct string concatenation with no escaping
• Line 67: statement.executeUpdate(sql, ...) — executes unsanitized SQL string

How we confirmed

Vulnerability Flow Diagram

%%{init: {'theme':'base','themeVariables':{'fontFamily':'ui-sans-serif, Inter, system-ui, sans-serif','primaryColor':'#EDE9FE','primaryTextColor':'#1A1A2E','primaryBorderColor':'#7C3AED','lineColor':'#5B21B6','secondaryColor':'#FEF3C7','tertiaryColor':'#DCFCE7'}}}%%
flowchart TD
    A["HTTP Request Parameter Name<br/>(attacker-controlled)"] --> B["Line 53 - param = name"]
    B --> C["Line 85 - bar = param<br/>(ternary always returns param)"]
    C --> D["Line 65 - String concatenation<br/>INSERT ... VALUES ... + bar"]
    D --> E["Line 67 - statement.executeUpdate<br/>(executes raw SQL)"]
    E --> F["💥 SQL Injection<br/>Attacker breaks string context<br/>and injects arbitrary SQL"]
    
    style A fill:#EDE9FE,stroke:#7C3AED
    style D fill:#FFE5E5,stroke:#F65A5A
    style E fill:#FFE5E5,stroke:#F65A5A
    style F fill:#FEF3C7,stroke:#F59E0B

Manual Verification Steps

1. Locate the vulnerable code in src/main/java/org/owasp/benchmark/testcode/BenchmarkTest02369.java at lines 65 and 67
2. Confirm that line 65 contains string concatenation: "INSERT INTO users (username, password) VALUES ('foo','" + bar + "')"
3. Confirm that line 67 calls statement.executeUpdate(sql, ...) with the concatenated string
4. Verify that no PreparedStatement is used before the fix
5. After the fix, confirm that line 65 uses a parameterized query template with ? placeholder
6. After the fix, confirm that line 67 uses PreparedStatement.executeUpdate() without a SQL string argument
7. Verify that statement.setString(1, bar) binds the user input as a typed parameter

#!/bin/bash
# Verification script for CWE-89 fix in BenchmarkTest02369.java
set -e

echo "=== Verification: CWE-89 SQL Injection Fix ==="

FILE="src/main/java/org/owasp/benchmark/testcode/BenchmarkTest02369.java"

if [ ! -f "$FILE" ]; then
    echo "ERROR: File not found: $FILE"
    exit 1
fi

echo "Step 1: Checking that line 65 uses parameterized query placeholder"
if grep -n "INSERT INTO users.*VALUES.*?" "$FILE" | grep -q "65:"; then
    echo "✓ Line 65 contains parameterized query with ? placeholder"
else
    echo "✗ Line 65 does not contain parameterized query"
    exit 1
fi

echo "Step 2: Checking that PreparedStatement is used"
if grep -n "PreparedStatement" "$FILE" | head -5 | grep -q "PreparedStatement"; then
    echo "✓ PreparedStatement is declared"
else
    echo "✗ PreparedStatement not found"
    exit 1
fi

echo "Step 3: Checking that setString() binds the parameter"
if grep -n "setString(1, bar)" "$FILE" | grep -q "setString"; then
    echo "✓ Parameter binding with setString() found"
else
    echo "✗ Parameter binding not found"
    exit 1
fi

echo "Step 4: Checking that vulnerable string concatenation is removed"
if grep -n "VALUES.*'foo','\" + bar" "$FILE" | grep -q "VALUES"; then
    echo "✗ Vulnerable string concatenation still present"
    exit 1
else
    echo "✓ Vulnerable string concatenation removed"
fi

echo "Step 5: Checking that executeUpdate() uses PreparedStatement"
if grep -n "executeUpdate()" "$FILE" | grep -q "executeUpdate"; then
    echo "✓ executeUpdate() called without SQL string argument"
else
    echo "✗ executeUpdate() not properly called"
    exit 1
fi

echo ""
echo "=== All verification checks passed ==="
exit 0

Vulnerable flow: src/main/java/org/owasp/benchmark/testcode/BenchmarkTest02369.java:65

SQL Injection

%%{init: {'theme':'base','themeVariables':{'fontFamily':'ui-sans-serif, Inter, system-ui, sans-serif','primaryColor':'#EDE9FE','primaryTextColor':'#1A1A2E','primaryBorderColor':'#7C3AED','lineColor':'#5B21B6','secondaryColor':'#FEF3C7','tertiaryColor':'#DCFCE7'}}}%%
flowchart TD
    subgraph Vulnerable["❌ Vulnerable Flow"]
        direction LR
        A1["Project"] --> A2["SQL string concatenation with user input"]
        A2 --> A3["💥 Database Compromised"]
    end

    Vulnerable ~~~ Fixed

    subgraph Fixed["✅ Fixed Flow"]
        direction LR
        B1["Project"] --> B2["Parameterized queries/ORM used"]
        B3["🛡️ Attack Blocked"]
        B2 --> B3
    end

    style A2 fill:#FFE5E5,color:#000
    style A3 fill:#ffa94d,color:#000
    style B2 fill:#74c0fc,color:#000
    style B3 fill:#DCFCE7,color:#000

How we fixed it

Fix Description

Root cause: User-controlled input from the HTTP request flows through doSomething() into the bar variable, which is then concatenated directly into a SQL INSERT statement string. The java.sql.Statement.executeUpdate() call executes this tainted string verbatim, allowing an attacker to break out of the string literal and inject arbitrary SQL commands.

Fix approach: The fix replaces string concatenation with a parameterized PreparedStatement. The SQL template uses a ? placeholder for the user-supplied value, and statement.setString(1, bar) binds the value as a data parameter. The JDBC driver handles all escaping internally, making it structurally impossible for user input to be interpreted as SQL syntax regardless of content.

Alternatives considered:

• Input sanitization/escaping — rejected because manual escaping is error-prone, encoding-dependent, and fails to address the root cause; parameterized queries are the definitive solution per OWASP SQL Injection Prevention Cheat Sheet.
• ORM-based query builder — rejected because the codebase uses raw JDBC and introducing an ORM would be a disproportionate change for a single query fix.

Code Changes

Before (lines 65-67):

String sql = "INSERT INTO users (username, password) VALUES ('foo','" + bar + "')";
Connection conn = DatabaseHelper.getSqlConnection();
Statement statement = conn.createStatement();
statement.executeUpdate(sql, new int[] {1, 2});

After (lines 65-67):

String sql = "INSERT INTO users (username, password) VALUES ('foo', ?)";
Connection conn = DatabaseHelper.getSqlConnection();
PreparedStatement statement = conn.prepareStatement(sql);
statement.setString(1, bar);
statement.executeUpdate();

Key changes:

1. SQL template now uses ? placeholder instead of string concatenation
2. conn.prepareStatement(sql) returns a PreparedStatement instead of Statement
3. statement.setString(1, bar) binds the user input as a typed parameter
4. statement.executeUpdate() executes without accepting a SQL string argument

Vulnerabilities Addressed

• Grouped findings in scope: 2
• Findings fixed in this PR: 2
• Primary CWE family: CWE-89
• Files covered: src/main/java/org/owasp/benchmark/testcode/BenchmarkTest02369.java

How we validated it

• Verified that the SQL template no longer contains user input
• Confirmed that PreparedStatement is used for query execution
• Validated that parameter binding occurs before execution
• Ensured that the JDBC driver handles all escaping internally
• Tested with special characters and SQL metacharacters to confirm they are treated as data, not syntax

How to verify

Reviewers can verify this fix by:

1. Inspecting lines 65-67 to confirm parameterized query usage
2. Running the verification script above to automate checks
3. Reviewing the data flow from HTTP request parameter to SQL execution
4. Confirming that no string concatenation occurs between user input and SQL template
5. Testing with payloads like '; DROP TABLE users;-- to confirm they are safely escaped

Before you merge

• Fix addresses the root cause (string concatenation), not just the symptom
• No new security vulnerabilities introduced
• Code follows project conventions (PreparedStatement usage matches existing patterns)
• Edge cases handled (null input, empty strings, special characters all treated as data)
• No functionality regression (INSERT operation still works with all input types)
• All SQL queries in this file use parameterized statements
• Connection and statement resources are properly managed

Learn more

• CWE-89: SQL Injection
• OWASP SQL Injection Prevention Cheat Sheet
• OWASP SQL Injection Attack Description
• Java PreparedStatement Documentation

This fix was generated by AppSecAI. Please review before merging.