Skip to content

MDEV-40653 Various SQL injections paths mitigation #5514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
drrtuy merged 2 commits into from
Aug 7, 2026
Merged

MDEV-40653 Various SQL injections paths mitigation #5514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e142f43
fix: resolve SQL injection imposed by lexer semantics difference b/w
drrtuy Aug 7, 2026
09b0996
fix: hardening against unknown SQL injection vectors running DuckDB.
drrtuy Aug 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions storage/duckdb/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,13 @@ install_deps() {
ncurses-devel readline-devel openssl-devel zlib-devel bzip2-devel \
libzstd-devel libcurl-devel libaio-devel libxml2-devel pcre2-devel \
libxcrypt-devel xz-devel pam-devel perl-DBI python3 python3-devel \
ccache rpm-build"
libatomic ccache"

# rpm-build is only needed to build RPM packages; on some bases (e.g. UBI 9)
# installing it forces an rpm upgrade that conflicts with pinned @System rpm.
if [[ $BUILD_PACKAGES = true ]]; then
RPM_DEPS="$RPM_DEPS rpm-build"
fi

local DEB_DEPS="build-essential git cmake ninja-build bison flex \
libncurses-dev libreadline-dev libssl-dev zlib1g-dev libbz2-dev \
Expand All @@ -313,9 +319,16 @@ install_deps() {
warn "Rocky 8 default gcc 8 lacks C++20 -- consider re-running with -R"
fi
;;
rockylinux:9|rocky:9|rocky:10)
rockylinux:9|rocky:9|rocky:10|rhel:9*|redhat:9*|red:9*)
# Enable EPEL and the CodeReady Builder / PowerTools-equivalent repo.
# Rocky/Alma expose it as the 'crb' alias; genuine RHEL enables it via
# subscription-manager. Enabling is best-effort: if neither mechanism
# is available we warn and continue so the dnf install below still runs.
command="$SUDO dnf install -y 'dnf-command(config-manager)' epel-release && \
$SUDO dnf config-manager --set-enabled crb && \
{ $SUDO dnf config-manager --set-enabled crb 2>/dev/null || \
$SUDO dnf config-manager --set-enabled ubi-9-codeready-builder-rpms 2>/dev/null || \
$SUDO subscription-manager repos --enable codeready-builder-for-rhel-9-\$(uname -m)-rpms 2>/dev/null || \
warn 'Could not enable CRB/CodeReady Builder repo; continuing without it'; } && \
$SUDO dnf install -y gcc gcc-c++ ${RPM_DEPS}"
;;
ubuntu:*|debian:*)
Expand Down
15 changes: 15 additions & 0 deletions storage/duckdb/common/duckdb_types.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,21 @@ DatabaseTableNames::DatabaseTableNames(const char *name)
db_name= std::string(ori_db_name, db_name_length);
}

std::string quote_duckdb_identifier(const char *name, size_t length)
{
std::string out;
out.reserve(length + 2);
out.push_back('"');
for (size_t i= 0; i < length; i++)
{
if (name[i] == '"')
out.push_back('"');
out.push_back(name[i]);
}
out.push_back('"');
return out;
}

Databasename::Databasename(const char *path_name)
{
char dbname[FN_REFLEN];
Expand Down
16 changes: 16 additions & 0 deletions storage/duckdb/common/duckdb_types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,22 @@ class DatabaseTableNames
std::string table_name;
};

/**
Quote an SQL identifier for DuckDB.

DuckDB (SQL standard) delimits identifiers with double quotes and escapes
an embedded double quote by doubling it. MariaDB identifiers (column names
in particular) may contain arbitrary characters, so failing to escape lets
a crafted name break out of the quoted identifier and inject DuckDB SQL
(MDEV-40653). Returns the name wrapped in double quotes with any embedded
double quote doubled.
*/
std::string quote_duckdb_identifier(const char *name, size_t length);
inline std::string quote_duckdb_identifier(const std::string &name)
{
return quote_duckdb_identifier(name.data(), name.size());
}

/**
Utility class to extract the database name from a path like "./db/".
*/
Expand Down
64 changes: 42 additions & 22 deletions storage/duckdb/convertor/ddl_convertor.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,8 +235,24 @@ Field *find_autoinc_field(const TABLE *table)
static std::string autoinc_nextval_expr(const std::string &schema_name,
const std::string &table_name)
{
return "nextval('\"" + schema_name + "\".\"" +
autoinc_sequence_name(table_name) + "\"')";
/*
The qualified sequence name is a quoted identifier nested inside a
single-quoted string literal argument to nextval(). Escape the inner
identifiers (doubling ") and then escape the resulting string for the
enclosing literal (doubling ').
*/
std::string qualified=
quote_duckdb_identifier(schema_name) + "." +
quote_duckdb_identifier(autoinc_sequence_name(table_name));
std::string escaped;
escaped.reserve(qualified.size());
for (char c : qualified)
{
if (c == '\'')
escaped.push_back('\'');
escaped.push_back(c);
}
return "nextval('" + escaped + "')";
}

/**
Expand Down Expand Up @@ -334,8 +350,8 @@ static void append_stmt_alter_table(std::ostringstream &output,
const std::string &schema_name,
const std::string &table_name)
{
output << "USE \"" << schema_name << "\";";
output << ALTER_TABLE_OP_STR << '"' << table_name << '"';
output << "USE " << quote_duckdb_identifier(schema_name) << ";";
output << ALTER_TABLE_OP_STR << quote_duckdb_identifier(table_name);
}

static void append_stmt_column_add(std::ostringstream &output,
Expand All @@ -349,7 +365,7 @@ static void append_stmt_column_add(std::ostringstream &output,
assert(!schema_name.empty() && !table_name.empty() && !column_name.empty() &&
!column_type.empty());
append_stmt_alter_table(output, schema_name, table_name);
output << ADD_COLUMN_OP_STR << '"' << column_name << '"' << " "
output << ADD_COLUMN_OP_STR << quote_duckdb_identifier(column_name) << " "
<< column_type;
if (has_default)
output << DEFINE_DEFAULT_STR << default_value;
Expand All @@ -363,7 +379,7 @@ static void append_stmt_column_drop(std::ostringstream &output,
{
assert(!schema_name.empty() && !table_name.empty() && !column_name.empty());
append_stmt_alter_table(output, schema_name, table_name);
output << DROP_COLUMN_OP_STR << '"' << column_name << '"' << ";";
output << DROP_COLUMN_OP_STR << quote_duckdb_identifier(column_name) << ";";
}

static void append_stmt_column_change_type(std::ostringstream &output,
Expand All @@ -375,7 +391,7 @@ static void append_stmt_column_change_type(std::ostringstream &output,
assert(!schema_name.empty() && !table_name.empty() && !column_name.empty() &&
!column_type.empty());
append_stmt_alter_table(output, schema_name, table_name);
output << ALTER_COLUMN_OP_STR << '"' << column_name << '"'
output << ALTER_COLUMN_OP_STR << quote_duckdb_identifier(column_name)
<< SET_DATA_TYPE_STR << column_type << ";";
}

Expand All @@ -388,8 +404,8 @@ static void append_stmt_column_rename(std::ostringstream &output,
assert(!schema_name.empty() && !table_name.empty() &&
!old_column_name.empty() && !new_column_name.empty());
append_stmt_alter_table(output, schema_name, table_name);
output << RENAME_COLUMN_OP_STR << '"' << old_column_name << '"' << " TO "
<< '"' << new_column_name << '"' << ";";
output << RENAME_COLUMN_OP_STR << quote_duckdb_identifier(old_column_name)
<< " TO " << quote_duckdb_identifier(new_column_name) << ";";
}

static void append_stmt_column_set_default(std::ostringstream &output,
Expand All @@ -401,8 +417,8 @@ static void append_stmt_column_set_default(std::ostringstream &output,
assert(!schema_name.empty() && !table_name.empty() && !column_name.empty() &&
!default_value.empty());
append_stmt_alter_table(output, schema_name, table_name);
output << ALTER_COLUMN_OP_STR << '"' << column_name << '"' << SET_DEFAULT_STR
<< default_value << ";";
output << ALTER_COLUMN_OP_STR << quote_duckdb_identifier(column_name)
<< SET_DEFAULT_STR << default_value << ";";
}

static void append_stmt_column_drop_default(std::ostringstream &output,
Expand All @@ -412,7 +428,7 @@ static void append_stmt_column_drop_default(std::ostringstream &output,
{
assert(!schema_name.empty() && !table_name.empty() && !column_name.empty());
append_stmt_alter_table(output, schema_name, table_name);
output << ALTER_COLUMN_OP_STR << '"' << column_name << '"'
output << ALTER_COLUMN_OP_STR << quote_duckdb_identifier(column_name)
<< DROP_DEFAULT_STR << ";";
}

Expand All @@ -423,7 +439,7 @@ static void append_stmt_column_set_not_null(std::ostringstream &output,
{
assert(!schema_name.empty() && !table_name.empty() && !column_name.empty());
append_stmt_alter_table(output, schema_name, table_name);
output << ALTER_COLUMN_OP_STR << '"' << column_name << '"'
output << ALTER_COLUMN_OP_STR << quote_duckdb_identifier(column_name)
<< SET_NOT_NULL_STR << ";";
}

Expand All @@ -434,7 +450,7 @@ static void append_stmt_column_drop_not_null(std::ostringstream &output,
{
assert(!schema_name.empty() && !table_name.empty() && !column_name.empty());
append_stmt_alter_table(output, schema_name, table_name);
output << ALTER_COLUMN_OP_STR << '"' << column_name << '"'
output << ALTER_COLUMN_OP_STR << quote_duckdb_identifier(column_name)
<< DROP_NOT_NULL_STR << ";";
}

Expand All @@ -449,7 +465,8 @@ static void append_stmt_table_rename(std::ostringstream &output,
!new_schema_name.empty() && !new_table_name.empty());
assert(old_schema_name == new_schema_name);
append_stmt_alter_table(output, old_schema_name, old_table_name);
output << RENAME_TABLE_OP_STR << '"' << new_table_name << '"' << ";";
output << RENAME_TABLE_OP_STR << quote_duckdb_identifier(new_table_name)
<< ";";
}

/* ----- FieldConvertor ----- */
Expand Down Expand Up @@ -508,7 +525,9 @@ std::string FieldConvertor::translate()

std::ostringstream result;

result << '"' << field->field_name.str << '"' << " ";
result << quote_duckdb_identifier(field->field_name.str,
field->field_name.length)
<< " ";
result << convert_type(m_field);

if (field->flags & NOT_NULL_FLAG)
Expand Down Expand Up @@ -733,10 +752,10 @@ std::string CreateTableConvertor::translate()
std::ostringstream result;
assert((m_create_info->options & HA_LEX_CREATE_TMP_TABLE) == 0);

result << "CREATE SCHEMA IF NOT EXISTS " << '"' << m_schema_name << '"'
<< ";";
result << "CREATE SCHEMA IF NOT EXISTS "
<< quote_duckdb_identifier(m_schema_name) << ";";

result << "USE " << '"' << m_schema_name << '"' << ";";
result << "USE " << quote_duckdb_identifier(m_schema_name) << ";";

/*
The sequence must exist before the table, because the AUTO_INCREMENT
Expand All @@ -751,8 +770,9 @@ std::string CreateTableConvertor::translate()
if (start > (ulonglong) INT64_MAX)
start= (ulonglong) INT64_MAX;

result << "CREATE SEQUENCE IF NOT EXISTS " << '"' << m_schema_name << '"'
<< "." << '"' << autoinc_sequence_name(m_table_name) << '"'
result << "CREATE SEQUENCE IF NOT EXISTS "
<< quote_duckdb_identifier(m_schema_name) << "."
<< quote_duckdb_identifier(autoinc_sequence_name(m_table_name))
<< " START WITH " << start << ";";
}

Expand All @@ -761,7 +781,7 @@ std::string CreateTableConvertor::translate()
Always use IF NOT EXISTS for safety in DuckDB. */
result << IF_NOT_EXISTS_STR;

result << '"' << m_table_name << '"';
result << quote_duckdb_identifier(m_table_name);
result << " (";

append_column_definition(result);
Expand Down
42 changes: 27 additions & 15 deletions storage/duckdb/convertor/dml_convertor.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,24 @@ namespace myduck { extern my_bool use_double_for_decimal; }
static const uint sizeof_trailing_comma= sizeof(", ") - 1;
static const uint sizeof_trailing_and= sizeof(" AND ") - 1;

/*
Append an SQL identifier to a String, quoted for DuckDB. DuckDB escapes an
embedded double quote by doubling it; failing to escape lets a crafted
identifier break out of the quoted name and inject SQL (MDEV-40653).
*/
static void append_quoted_identifier(String &target, const char *name,
size_t length)
{
target.append(STRING_WITH_LEN("\""));
for (size_t i= 0; i < length; i++)
{
if (name[i] == '"')
target.append(STRING_WITH_LEN("\""));
target.append(&name[i], 1);
}
target.append(STRING_WITH_LEN("\""));
}

void append_field_value_to_sql(String &target_str, Field *field)
{
if (field->is_null())
Expand Down Expand Up @@ -165,13 +183,10 @@ static inline void append_table_name(TABLE *table, String &query)
the temp name.
*/
DatabaseTableNames dt(table->s->normalized_path.str);
query.append(STRING_WITH_LEN("\""));
query.append(dt.db_name.c_str(), dt.db_name.length());
query.append(STRING_WITH_LEN("\""));
append_quoted_identifier(query, dt.db_name.c_str(), dt.db_name.length());
query.append(STRING_WITH_LEN("."));
query.append(STRING_WITH_LEN("\""));
query.append(dt.table_name.c_str(), dt.table_name.length());
query.append(STRING_WITH_LEN("\""));
append_quoted_identifier(query, dt.table_name.c_str(),
dt.table_name.length());
}

static inline void get_write_fields(TABLE *table, std::vector<Field *> &fields)
Expand Down Expand Up @@ -232,9 +247,8 @@ void DMLConvertor::generate_where_clause(String &query)

for (auto field : fields)
{
query.append(STRING_WITH_LEN("\""));
query.append(field->field_name.str, field->field_name.length);
query.append(STRING_WITH_LEN("\""));
append_quoted_identifier(query, field->field_name.str,
field->field_name.length);
query.append(STRING_WITH_LEN(" = "));

append_where_value(query, field);
Expand All @@ -260,9 +274,8 @@ void InsertConvertor::generate_fields_and_values(String &query)
query.append(STRING_WITH_LEN(" ("));
for (auto field : fields)
{
query.append(STRING_WITH_LEN("\""));
query.append(field->field_name.str, field->field_name.length);
query.append(STRING_WITH_LEN("\""));
append_quoted_identifier(query, field->field_name.str,
field->field_name.length);
query.append(STRING_WITH_LEN(", "));
}
query.length(query.length() - sizeof_trailing_comma);
Expand Down Expand Up @@ -293,9 +306,8 @@ void UpdateConvertor::generate_fields_and_values(String &query)

for (auto field : fields)
{
query.append(STRING_WITH_LEN("\""));
query.append(field->field_name.str, field->field_name.length);
query.append(STRING_WITH_LEN("\""));
append_quoted_identifier(query, field->field_name.str,
field->field_name.length);
query.append(STRING_WITH_LEN(" = "));

append_field_value_to_sql(query, field);
Expand Down
8 changes: 5 additions & 3 deletions storage/duckdb/docs/mariadb-duckdb-incompatibilities.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,15 @@ SELECT pushdown uses the original SQL text from `THD::query()`. MariaDB-specific
| `HIGH_PRIORITY`, `SQL_NO_CACHE`, `SQL_CACHE`, `SQL_BUFFER_RESULT`, `SQL_SMALL_RESULT`, `SQL_BIG_RESULT`, `SQL_CALC_FOUND_ROWS` | -- | Stripped |
| `FORCE INDEX(...)`, `USE INDEX(...)`, `IGNORE INDEX(...)` | -- | Stripped |

### Known unhandled cases (currently cause query failures)
### Known unhandled cases (currently fail or change semantics)

These MariaDB constructs are **not yet rewritten** and fail when pushed down. Because pushdown forwards the original `THD::query()` text (only backticks are converted to double quotes), MariaDB-specific token semantics survive into DuckDB. Discovered while running an analytical query set (402 queries) against DuckDB-engine tables.
These MariaDB constructs are **not yet rewritten** and either fail or have different semantics when pushed down. Because pushdown forwards the original `THD::query()` text (only backticks are converted to double quotes), MariaDB-specific token semantics survive into DuckDB. Discovered while running an analytical query set (402 queries) against DuckDB-engine tables.

| MariaDB construct | Sent to DuckDB as | DuckDB result | Root cause |
|---|---|---|---|
| Double-quoted **string literal**, e.g. `JSON_OBJECT("month", ...)` | `"month"` (verbatim) | `Binder Error: Referenced column "month" not found` | MariaDB without `ANSI_QUOTES` treats `"x"` as a string literal; DuckDB treats `"x"` as an identifier. The forwarded literal is read as a column reference. |
| Unquoted column **alias equal to a DuckDB reserved keyword**, e.g. `SELECT expr name` / `SELECT expr year` | `... name` / `... year` (verbatim) | `Parser Error: syntax error at or near "name"` | DuckDB forbids reserved keywords as unquoted identifiers. `AS name` or `"name"` work; bare `name` / `year` / `month` do not. This is why most implicit aliases pass but keyword aliases fail. |
| MariaDB **executable/versioned comments**, e.g. `/*! + 1 */`, `/*!100000 + 1 */`, or `/*M! + 1 */` | Comment text (verbatim) | Contents are ignored as an ordinary block comment | MariaDB executes eligible `/*! ... */` and `/*M! ... */` contents as SQL, optionally gated by a version number; DuckDB treats the entire region as a comment. Forwarded queries can therefore silently use different predicates or expressions. |

Reproductions (against any DuckDB-engine table `t`):

Expand All @@ -104,9 +105,10 @@ SELECT JSON_OBJECT("k", 1) FROM t; -- Binder Error: column "k" not found
SELECT JSON_OBJECT('k', 1) FROM t; -- OK
SELECT col name FROM t; -- Parser Error at "name"
SELECT col AS name FROM t; -- OK
SELECT 1 /*! + 1 */ FROM t; -- MariaDB: 2; DuckDB pushdown: 1
```

**Fix direction**: in `ha_duckdb_pushdown.cc`, convert double-quoted string literals to single-quoted form and quote (or `AS`-prefix) aliases that are DuckDB reserved keywords. Both require lexer-aware handling of the query text, not naive replacement — `backticks_to_double_quotes()` already produces legitimate double-quoted identifiers that must not be altered.
**Fix direction**: in `ha_duckdb_pushdown.cc`, convert double-quoted string literals to single-quoted form and quote (or `AS`-prefix) aliases that are DuckDB reserved keywords. Both require lexer-aware handling of the query text, not naive replacement — `backticks_to_double_quotes()` already produces legitimate double-quoted identifiers that must not be altered. Executable/versioned comments must either be expanded according to MariaDB's version rules or make the query ineligible for raw SQL forwarding.

---

Expand Down
Loading