Security: Fix CWE-89 (SQL Injection) vulnerability in src/main/java/org/owasp/benchmark/testcode/BenchmarkTest01963.java:56#661
Open
appsecai-app[bot] wants to merge 1 commit intomainfrom
Conversation
Replace string concatenation with parameterized query using BatchPreparedStatementSetter and PreparedStatement.setString() to eliminate SQL injection vulnerability in HTTP header input. Fixes: 69c734eb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What we found
69c734ebsrc/main/java/org/owasp/benchmark/testcode/BenchmarkTest01963.java:56Description: User-controlled input from an HTTP header flows directly into a SQL query via string concatenation without parameterization. This allows attackers to inject arbitrary SQL commands.
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-specific extensions.
Attack surface: The vulnerability is reachable via HTTP headers, which are trivial for an attacker to control. No special privileges or preconditions are required.
Exploitability: Standard SQL injection payloads (e.g.,
' OR '1'='1, UNION-based extraction, time-based blind injection) work directly against the PASSWORD clause.Why we're changing it
Root cause analysis:
The data flow is unambiguous and undefended:
paramassigned fromrequest.getHeader("BenchmarkTest01963")— fully attacker-controlled HTTP header.URLDecoder.decode()applied — URL-decoding is not a security control; it expands encoded payloads rather than neutralizing them.doSomething()is an identity function (bar = param; return bar;) — zero transformation or validation.barconcatenated directly into SQL string:"SELECT * from USERS where USERNAME='foo' and PASSWORD='" + bar + "'"— no escaping or parameterization.JdbcTemplate.batchUpdate(sql)— no prepared statement, no bind parameters.No mitigations exist anywhere in the call chain. The attacker controls the PASSWORD clause completely.
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 Header - BenchmarkTest01963"] --> B["URLDecoder.decode()<br/>(Line 49)"] B --> C["doSomething() Identity Function<br/>(Lines 75-77)"] C --> D["String Concatenation<br/>PASSWORD='" + bar + "'<br/>(Line 54)"] D --> E["JdbcTemplate.batchUpdate(sql)<br/>(Line 56)"] E --> F["💥 SQL Injection Executed<br/>Attacker controls PASSWORD clause"] style A fill:#EDE9FE,stroke:#7C3AED style B fill:#EDE9FE,stroke:#7C3AED style C fill:#EDE9FE,stroke:#7C3AED style D fill:#FFE5E5,stroke:#F65A5A style E fill:#FFE5E5,stroke:#F65A5A style F fill:#FEF3C7,stroke:#F59E0BHow we confirmed
Manual verification steps
BenchmarkTest01963fromrequest.getHeader()at line 45 through thedoSomething()identity function to the SQL concatenation at line 54.URLDecoder.decode()does not sanitize SQL metacharacters; it only performs URL decoding.batchUpdate(String sql)without any parameterized query mechanism.' OR '1'='1in the header to confirm SQL injection is possible.Runnable Verification Script (click to expand)
Save this script and run with
bash verify_fix.sh:Vulnerable flow: src/main/java/org/owasp/benchmark/testcode/BenchmarkTest01963.java:56
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["HTTP Header Input"] --> A2["URLDecoder.decode() (BenchmarkTest01963.java)"] A2 --> A3["doSomething() returns raw param as bar"] A3 --> A4["String concat at line 56 PASSWORD='" + bar + "'"] A4 --> A5["batchUpdate(sql) executes raw SQL"] A5 --> A6["💥 Database Compromised"] end Vulnerable ~~~ Fixed subgraph Fixed["✅ Fixed Flow"] direction LR B1["HTTP Header Input"] --> B2["URLDecoder.decode() (BenchmarkTest01963.java)"] B2 --> B3["doSomething() returns raw param as bar"] B3 --> B4["Parameterized query PASSWORD=?"] B4 --> B5["BatchPreparedStatementSetter ps.setString(1, bar)"] B5 --> B6["🛡️ Attack Blocked"] end style A4 fill:#FFE5E5,color:#1A1A2E style A6 fill:#ffa94d,color:#000 style B4 fill:#74c0fc,color:#000 style B6 fill:#DCFCE7,color:#000How we fixed it
Fix approach: The fix replaces string concatenation with a parameterized query using Spring's
BatchPreparedStatementSetter. The SQL template uses a positional placeholder?and the user-supplied value is bound viaPreparedStatement.setString(), which the JDBC driver treats as opaque data rather than executable SQL syntax. This completely eliminates the injection path regardless of what characters the attacker supplies.Key changes:
"SELECT * from USERS where USERNAME='foo' and PASSWORD=?"JdbcTemplate.batchUpdate(String sql, List<Object[]> batchArgs)or implementBatchPreparedStatementSetterto bind the user input viaPreparedStatement.setString(1, bar)',--,OR, etc.) inert regardless of input contentWhy this works: Parameterized queries enforce a strict separation between SQL structure and data. The database driver parses the SQL template first, then substitutes parameter values as opaque strings. Attackers cannot inject SQL syntax through data parameters.
Alternatives considered and rejected:
batchUpdate()call pattern to minimize behavioral change.Vulnerabilities Addressed
CWE-89
Tainted Sql From Http Request
How we validated it
?placeholders for all user-controlled valuesPreparedStatement.setString()is called for each parameter' OR '1'='1,'; DROP TABLE USERS; --) to confirm they are treated as literal strings, not SQL commandsHow to verify
Before you merge
Learn more
This fix was generated by AppSecAI. Please review before merging.