Skip to content

Commit 555439b

Browse files
felickzCopilot
andauthored
Add Java SQL injection sinks for Spring R2DBC and io.r2dbc.spi (#204)
Models Spring Data R2DBC's DatabaseClient and the underlying io.r2dbc.spi SPI as SQL injection sinks in codeql/java-all, closing a false negative where unvalidated values concatenated into SQL text and executed via DatabaseClient.sql(String) were not flagged by java/sql-injection. Adds 4 new sql-injection sink rows across two manual data extension files: - java/ext/manual/org.springframework.r2dbc.model.yml - DatabaseClient.sql(String) -> Argument[0] - DatabaseClient.sql(Supplier<String>) -> the deferred/lazy overload, modeled via a summaryModel that bridges taint from the supplier's return value onto the GenericExecuteSpec object, with the sink placed on the terminal fetch() call's receiver - java/ext/manual/io.r2dbc.spi.model.yml - Connection.createStatement(String) -> Argument[0] - Batch.add(String) -> Argument[0] io.r2dbc.spi is the low-level, driver-agnostic SPI that DatabaseClient (and every other reactive relational driver) is built on top of, so both files are needed for full coverage of code that either uses Spring's higher-level API or drops down to the raw R2DBC SPI directly. Adds java/test/security/CWE-089/spring-r2dbc/ with self-contained stub sources for both APIs and a DatabaseClientSqlInjection.ql test validating all 4 sinks (plus safe/negative parameter-binding cases) via the real QueryInjectionFlow::flow used by the standard java/sql-injection query. Bumps java/ext's qlpack.yml version (0.7.1 -> 0.8.0) per CONTRIBUTING.md. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f02ef23b-e35d-4536-bd58-f2fbc2976ba9
1 parent c4de11a commit 555439b

13 files changed

Lines changed: 297 additions & 1 deletion
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/java-all
4+
extensible: sinkModel
5+
data:
6+
# `io.r2dbc.spi` is the low-level, driver-agnostic R2DBC SPI that every
7+
# reactive relational driver (Postgres, MySQL, H2, SQL Server, ...)
8+
# implements, and that higher-level APIs such as Spring's
9+
# `DatabaseClient` are themselves built on top of. Modeling these two
10+
# sinks closes the false negative for any code (Spring-based or not)
11+
# that drops down to the raw R2DBC SPI to build/execute SQL text.
12+
#
13+
# `Connection.createStatement(String)` is the R2DBC equivalent of
14+
# `java.sql.Connection.prepareStatement(String)`: the SQL text is fixed
15+
# at statement-creation time and executed (with any bound parameters)
16+
# via `Statement.execute()`.
17+
- ["io.r2dbc.spi", "Connection", True, "createStatement", "(String)", "", "Argument[0]", "sql-injection", "manual"]
18+
# `Batch.add(String)` is the R2DBC equivalent of
19+
# `java.sql.Statement.addBatch(String)`: it appends a raw SQL statement
20+
# to a batch that is later executed via `Batch.execute()`.
21+
- ["io.r2dbc.spi", "Batch", True, "add", "(String)", "", "Argument[0]", "sql-injection", "manual"]
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/java-all
4+
extensible: sinkModel
5+
data:
6+
# `DatabaseClient.sql(String)` executes the given SQL text. Neither the
7+
# SQL text nor any subsequently bound parameters are validated by the
8+
# framework, so an unvalidated string reaching this argument is a SQL
9+
# injection sink.
10+
- ["org.springframework.r2dbc.core", "DatabaseClient", True, "sql", "(String)", "", "Argument[0]", "sql-injection", "manual"]
11+
# `DatabaseClient.sql(Supplier<String>)` defers resolution of the SQL
12+
# text until execution. The supplier's return value is not itself an
13+
# argument/return position that MaD can target directly as a sink, so
14+
# taint is instead propagated (via the summaryModel below) onto the
15+
# `GenericExecuteSpec` object returned by `sql(...)`. The sink is
16+
# placed on `fetch()`, the point where the deferred query is finalized
17+
# and enters the fluent execution chain; actual execution remains
18+
# reactive/deferred until the returned publisher is subscribed to, but
19+
# the tainted SQL text has already been captured by that point.
20+
- ["org.springframework.r2dbc.core", "DatabaseClient$GenericExecuteSpec", True, "fetch", "", "", "Argument[this]", "sql-injection", "manual"]
21+
- addsTo:
22+
pack: codeql/java-all
23+
extensible: summaryModel
24+
data:
25+
# Propagate taint from the deferred SQL supplier's return value onto
26+
# the `GenericExecuteSpec` returned by `sql(Supplier<String>)`, so it
27+
# can reach the `fetch()` sink modeled above.
28+
- ["org.springframework.r2dbc.core", "DatabaseClient", True, "sql", "(java.util.function.Supplier)", "", "Argument[0].ReturnValue", "ReturnValue", "taint", "manual"]

‎java/ext/qlpack.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
library: true
22
name: githubsecuritylab/codeql-java-extensions
3-
version: 0.7.1
3+
version: 0.8.0
44
extensionTargets:
55
codeql/java-all: '9.2.1'
66
dataExtensions:
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
| com/example/DeferredQueryHandler.java:24:23:24:30 | source(...) | com/example/DeferredQueryHandler.java:25:12:26:77 | sql(...) |
2+
| com/example/RawR2dbcHandler.java:28:23:28:30 | source(...) | com/example/RawR2dbcHandler.java:31:13:31:69 | ... + ... |
3+
| com/example/RawR2dbcHandler.java:36:23:36:30 | source(...) | com/example/RawR2dbcHandler.java:38:15:38:71 | ... + ... |
4+
| com/example/StringConcatQueryHandler.java:39:23:39:30 | source(...) | com/example/StringConcatQueryHandler.java:43:31:43:35 | query |
5+
| com/example/StringConcatQueryHandler.java:40:21:40:28 | source(...) | com/example/StringConcatQueryHandler.java:43:31:43:35 | query |
6+
| com/example/StringConcatQueryHandler.java:41:24:41:31 | source(...) | com/example/StringConcatQueryHandler.java:43:31:43:35 | query |
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/**
2+
* Test that the standard `java/sql-injection` query (`QueryInjectionFlow`,
3+
* from `semmle.code.java.security.SqlInjectionQuery`) recognizes taint
4+
* reaching the Spring R2DBC (`org.springframework.r2dbc.core.DatabaseClient`)
5+
* and raw R2DBC SPI (`io.r2dbc.spi`) sinks modeled in
6+
* `java/ext/manual/org.springframework.r2dbc.model.yml` and
7+
* `java/ext/manual/io.r2dbc.spi.model.yml`.
8+
*/
9+
10+
import java
11+
import semmle.code.java.dataflow.FlowSources
12+
import semmle.code.java.security.SqlInjectionQuery
13+
14+
/**
15+
* A fake remote flow source used by the test code to mark a value as
16+
* tainted, matching calls to a `source()` method. This isolates the test
17+
* from real-world remote-flow-source modeling (e.g. `@RequestParam`) so it
18+
* focuses purely on validating the new sink models.
19+
*/
20+
private class SourceMethodSource extends RemoteFlowSource {
21+
SourceMethodSource() { this.asExpr().(MethodCall).getMethod().hasName("source") }
22+
23+
override string getSourceType() { result = "source" }
24+
}
25+
26+
from DataFlow::Node source, DataFlow::Node sink
27+
where QueryInjectionFlow::flow(source, sink)
28+
select source, sink
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package com.example;
2+
3+
import org.springframework.r2dbc.core.DatabaseClient;
4+
5+
/**
6+
* Test source for the `DatabaseClient.sql(Supplier<String>)` overload: SQL
7+
* text is deferred behind a lambda instead of being passed directly as a
8+
* String.
9+
*/
10+
public class DeferredQueryHandler {
11+
12+
// Fake remote flow source, recognized by DatabaseClientSqlInjection.ql.
13+
public static String source() {
14+
return null;
15+
}
16+
17+
private final DatabaseClient databaseClient;
18+
19+
public DeferredQueryHandler(DatabaseClient databaseClient) {
20+
this.databaseClient = databaseClient;
21+
}
22+
23+
public Object runDeferredQuery() {
24+
String category = source();
25+
return databaseClient
26+
.sql(() -> "SELECT * FROM items WHERE category = '" + category + "'")
27+
.fetch()
28+
.all(); // sink 2: sql(Supplier<String>)
29+
}
30+
}
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
package com.example;
2+
3+
import io.r2dbc.spi.Batch;
4+
import io.r2dbc.spi.Connection;
5+
import io.r2dbc.spi.Result;
6+
import io.r2dbc.spi.Statement;
7+
8+
/**
9+
* Test source for the low-level `io.r2dbc.spi` SPI that Spring's
10+
* `DatabaseClient` (and every other reactive relational driver) is built on
11+
* top of. Code that drops down to this API to build/execute SQL text bypasses
12+
* `DatabaseClient` entirely, so it needs its own sink models.
13+
*/
14+
public class RawR2dbcHandler {
15+
16+
// Fake remote flow source, recognized by DatabaseClientSqlInjection.ql.
17+
public static String source() {
18+
return null;
19+
}
20+
21+
private final Connection connection;
22+
23+
public RawR2dbcHandler(Connection connection) {
24+
this.connection = connection;
25+
}
26+
27+
public Result runStatement() {
28+
String category = source();
29+
Statement statement =
30+
connection.createStatement( // sink 3: Connection.createStatement(String)
31+
"SELECT * FROM items WHERE category = '" + category + "'");
32+
return statement.execute();
33+
}
34+
35+
public Result runBatch() {
36+
String category = source();
37+
Batch batch = connection.createBatch();
38+
batch.add("SELECT * FROM items WHERE category = '" + category + "'"); // sink 4: Batch.add(String)
39+
return batch.execute();
40+
}
41+
42+
public Result runStatementSafe() {
43+
// Parameter binding, not string concatenation - should NOT be flagged.
44+
String category = source();
45+
Statement statement =
46+
connection.createStatement("SELECT * FROM items WHERE category = $1");
47+
statement.bind(0, category);
48+
return statement.execute();
49+
}
50+
}
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
package com.example;
2+
3+
import org.springframework.r2dbc.core.DatabaseClient;
4+
5+
/**
6+
* Test source for `DatabaseClient.sql(String)`: request-derived values are
7+
* concatenated into a SQL string via a `StringBuilder`, then executed
8+
* directly.
9+
*/
10+
public class StringConcatQueryHandler {
11+
12+
// Fake remote flow source, recognized by DatabaseClientSqlInjection.ql.
13+
public static String source() {
14+
return null;
15+
}
16+
17+
private final DatabaseClient databaseClient;
18+
19+
public StringConcatQueryHandler(DatabaseClient databaseClient) {
20+
this.databaseClient = databaseClient;
21+
}
22+
23+
private void appendFilters(StringBuilder sql, String category, String sortBy, String sortOrder) {
24+
if (category != null) {
25+
sql.append(" AND category = '").append(category).append("'");
26+
}
27+
if (sortBy != null && sortOrder != null) {
28+
sql.append(" ORDER BY ").append(sortBy).append(" ").append(sortOrder);
29+
}
30+
}
31+
32+
private String buildQuery(String category, String sortBy, String sortOrder) {
33+
StringBuilder sql = new StringBuilder("SELECT * FROM items WHERE 1=1");
34+
appendFilters(sql, category, sortBy, sortOrder);
35+
return sql.toString();
36+
}
37+
38+
public Object runQuery() {
39+
String category = source();
40+
String sortBy = source();
41+
String sortOrder = source();
42+
String query = buildQuery(category, sortBy, sortOrder);
43+
return databaseClient.sql(query).fetch().all(); // sink 1: sql(String)
44+
}
45+
46+
public Object runQuerySafe() {
47+
// Parameter binding, not string concatenation - should NOT be flagged.
48+
String category = source();
49+
return databaseClient
50+
.sql("SELECT * FROM items WHERE category = :category")
51+
.bind("category", category)
52+
.fetch()
53+
.all();
54+
}
55+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/*
2+
* Minimal stub of the `io.r2dbc.spi` SPI, used only to compile the test
3+
* source in this directory. Not the real implementation.
4+
*/
5+
package io.r2dbc.spi;
6+
7+
public interface Batch {
8+
9+
Batch add(String sql);
10+
11+
Result execute();
12+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/*
2+
* Minimal stub of the `io.r2dbc.spi` SPI, used only to compile the test
3+
* source in this directory. Not the real implementation.
4+
*/
5+
package io.r2dbc.spi;
6+
7+
public interface Connection {
8+
9+
Statement createStatement(String sql);
10+
11+
Batch createBatch();
12+
}

0 commit comments

Comments
 (0)