What we found

• AppSecAI Vulnerability ID: 69c734eb
• Vulnerability: CWE-89: SQL Injection
• Severity: Medium
• File: src/main/java/org/owasp/benchmark/testcode/BenchmarkTest02355.java:67
• Detected By: OpenGrep
• Detection Rule: Tainted Sql From Http Request

Description: Detected input from an HTTPServletRequest flowing into a SQL sink without proper parameterization. The PASSWORD clause of a SQL query is constructed via string concatenation with user-controlled input, allowing SQL injection attacks.

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.

Risk level: High - Direct database access with authentication bypass potential

Attack scenario: An attacker supplies a malicious HTTP parameter name such as ' OR '1'='1 which flows through the application logic and is concatenated into the PASSWORD clause. This breaks out of the string literal and alters the query's WHERE clause logic, allowing unauthorized database access.

Why we're changing it

Status: Confirmed vulnerability

Data flow analysis:

1. Line 45-58: param is assigned the HTTP parameter name (not value) from request.getParameterNames() — attacker-controlled input.
2. Lines 85-92 (doSomething): Constructs ["safe", param, "moresafe"], removes index 0, returns index 0 — which is param unchanged. No sanitization occurs.
3. Line 62: bar (the unmodified param) is concatenated directly into the SQL string:

   String sql = "SELECT * from USERS where USERNAME=? and PASSWORD='" + bar + "'";

4. Lines 67-69: A PreparedStatement is created and setString(1, "foo") binds only the USERNAME placeholder. The PASSWORD clause is not parameterized — it contains the raw concatenated string.

The PreparedStatement provides zero protection here because parameterization was applied to USERNAME=? but PASSWORD='<bar>' is already baked into the SQL string before prepareStatement() is called. An attacker supplies a parameter name like ' OR '1'='1 to break out of the password literal and alter query semantics.

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["param = request.getParameterNames()"]  
    B --> C["doSomething() returns param<br/>(No Sanitization)"]
    C --> D["bar = param"]
    D --> E["String Concatenation:<br/>PASSWORD='" + bar + "'"]
    E --> F["SQL String Built Before<br/>prepareStatement()"]
    F --> G["❌ PreparedStatement Too Late<br/>Structure Already Compromised"]
    G --> H["💥 SQL Injection Possible<br/>Query Logic Altered"]
    
    style A fill:#EDE9FE,stroke:#7C3AED
    style E fill:#FFE5E5,stroke:#F65A5A
    style G fill:#FEF3C7,stroke:#F59E0B
    style H fill:#FEF3C7,stroke:#F59E0B

How we confirmed

1. Traced HTTP parameter names from request.getParameterNames() through doSomething() which performs no transformation
2. Verified that bar (containing the parameter name) is concatenated directly into the SQL string at line 62
3. Confirmed that prepareStatement() is called after string concatenation, making parameterization ineffective for the PASSWORD clause
4. Validated that only the USERNAME placeholder is bound via setString(1, "foo"), leaving PASSWORD unprotected
5. Confirmed that parameter names are fully attacker-controllable and can contain SQL metacharacters

Vulnerability Flow Diagram

See diagram above under "Why we're changing it".

Vulnerable flow: src/main/java/org/owasp/benchmark/testcode/BenchmarkTest02355.java:67

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

Root cause: The SQL query used a parameterized placeholder for the USERNAME field but concatenated the user-controlled bar variable directly into the PASSWORD field via string interpolation.

Fix approach: Replacing the string concatenation in the PASSWORD clause with a second parameterized placeholder (?) and binding bar via statement.setString(2, bar) ensures the JDBC driver treats the value as a literal string regardless of its content. Parameterized queries separate SQL structure from data at the protocol level, making SQL injection structurally impossible for bound parameters.

Changes made:

Line 62 now reads:

String sql = "SELECT * from USERS where USERNAME=? and PASSWORD=?";

And statement.setString(2, bar) binds the user-supplied value as data rather than SQL syntax:

statement.setString(1, "foo");
statement.setString(2, bar);

Alternatives considered:

• Input sanitization/escaping — rejected because escaping is error-prone, encoding-dependent, and does not eliminate the root cause; a parameterized query is the correct fix per OWASP SQL Injection Prevention Cheat Sheet
• Allowlist validation on bar — rejected because it is a secondary defense that does not replace parameterization and would require knowing all valid input values

Vulnerabilities Addressed

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

How we validated it

• Verified that both USERNAME and PASSWORD placeholders are now bound via setString() calls
• Confirmed that the SQL string structure is fixed before any user input is bound
• Tested with malicious parameter names containing SQL metacharacters (' OR '1'='1, ; DROP TABLE USERS; --) to ensure they are treated as literal strings
• Validated that query behavior is unchanged for legitimate parameter names
• Confirmed no functionality regression in authentication logic

How to verify

1. Review the SQL query construction at line 62 to confirm both USERNAME and PASSWORD use ? placeholders
2. Verify that statement.setString(1, "foo") and statement.setString(2, bar) are both present
3. Confirm that no string concatenation occurs between the SQL string definition and prepareStatement() call
4. Test with parameter names containing SQL metacharacters to ensure they are safely escaped by the JDBC driver

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

echo "=== Verification: SQL Injection Fix ==="

# Step 1: Check that both USERNAME and PASSWORD use parameterized placeholders
echo "Step 1: Verifying parameterized query structure..."
if grep -q 'SELECT \* from USERS where USERNAME=? and PASSWORD=?' src/main/java/org/owasp/benchmark/testcode/BenchmarkTest02355.java; then
    echo "✓ SQL query uses parameterized placeholders for both USERNAME and PASSWORD"
else
    echo "✗ SQL query does not use proper parameterization"
    exit 1
fi

# Step 2: Verify setString calls for both parameters
echo "Step 2: Verifying parameter binding..."
if grep -A 2 'prepareStatement(sql' src/main/java/org/owasp/benchmark/testcode/BenchmarkTest02355.java | grep -q 'setString(1'; then
    echo "✓ USERNAME parameter is bound via setString(1, ...)"
else
    echo "✗ USERNAME parameter binding not found"
    exit 1
fi

if grep -A 3 'prepareStatement(sql' src/main/java/org/owasp/benchmark/testcode/BenchmarkTest02355.java | grep -q 'setString(2'; then
    echo "✓ PASSWORD parameter is bound via setString(2, ...)"
else
    echo "✗ PASSWORD parameter binding not found"
    exit 1
fi

# Step 3: Verify no string concatenation in SQL construction
echo "Step 3: Checking for string concatenation in SQL..."
if grep -q 'PASSWORD=.*+.*bar' src/main/java/org/owasp/benchmark/testcode/BenchmarkTest02355.java; then
    echo "✗ String concatenation still present in SQL query"
    exit 1
else
    echo "✓ No string concatenation in PASSWORD clause"
fi

# Step 4: Compile the fixed code
echo "Step 4: Compiling fixed code..."
if mvn clean compile -q -DskipTests 2>/dev/null; then
    echo "✓ Code compiles successfully"
else
    echo "✗ Compilation failed"
    exit 1
fi

echo ""
echo "=== All verification checks passed ==="

Before you merge

• Fix addresses the root cause (parameterization of PASSWORD clause), not just the symptom
• No new security vulnerabilities introduced
• Code follows project conventions and JDBC best practices
• Edge cases handled (null input, empty strings, special characters all treated as data)
• No functionality regression in authentication or query logic
• All SQL queries in this file use parameterized statements
• Reviewer confirms that both placeholders are bound before query execution

Learn more

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

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