From e142f43833fa4a135123ca44a642fa27cbe324f4 Mon Sep 17 00:00:00 2001 From: drrtuy Date: Fri, 7 Aug 2026 15:43:13 +0100 Subject: [PATCH 1/2] fix: resolve SQL injection imposed by lexer semantics difference b/w MariaDB and DuckDB. --- storage/duckdb/build.sh | 19 +- storage/duckdb/common/duckdb_types.cc | 15 ++ storage/duckdb/common/duckdb_types.h | 16 ++ storage/duckdb/convertor/ddl_convertor.cc | 64 +++--- storage/duckdb/convertor/dml_convertor.cc | 42 ++-- .../docs/mariadb-duckdb-incompatibilities.md | 8 +- storage/duckdb/ha_duckdb.cc | 75 ++++--- storage/duckdb/ha_duckdb_pushdown.cc | 47 ++--- .../r/duckdb_identifier_escaping.result | 102 ++++++++++ .../duckdb/t/duckdb_identifier_escaping.test | 88 +++++++++ storage/duckdb/runtime/delta_appender.cc | 53 ++--- storage/duckdb/runtime/duckdb_context.cc | 5 +- storage/duckdb/runtime/duckdb_query.cc | 183 +++++++++++++++++- storage/duckdb/runtime/duckdb_query.h | 15 ++ 14 files changed, 599 insertions(+), 133 deletions(-) create mode 100644 storage/duckdb/mysql-test/duckdb/r/duckdb_identifier_escaping.result create mode 100644 storage/duckdb/mysql-test/duckdb/t/duckdb_identifier_escaping.test diff --git a/storage/duckdb/build.sh b/storage/duckdb/build.sh index c7d179a2c6464..c9a9956913c5f 100755 --- a/storage/duckdb/build.sh +++ b/storage/duckdb/build.sh @@ -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 \ @@ -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:*) diff --git a/storage/duckdb/common/duckdb_types.cc b/storage/duckdb/common/duckdb_types.cc index 625e1c0b168dc..e95fe198667aa 100644 --- a/storage/duckdb/common/duckdb_types.cc +++ b/storage/duckdb/common/duckdb_types.cc @@ -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]; diff --git a/storage/duckdb/common/duckdb_types.h b/storage/duckdb/common/duckdb_types.h index 2e39c1aeb1150..6642467ccdebb 100644 --- a/storage/duckdb/common/duckdb_types.h +++ b/storage/duckdb/common/duckdb_types.h @@ -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/". */ diff --git a/storage/duckdb/convertor/ddl_convertor.cc b/storage/duckdb/convertor/ddl_convertor.cc index fe87a77c7d9d2..2ce9cab6b8f26 100644 --- a/storage/duckdb/convertor/ddl_convertor.cc +++ b/storage/duckdb/convertor/ddl_convertor.cc @@ -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 + "')"; } /** @@ -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, @@ -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; @@ -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, @@ -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 << ";"; } @@ -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, @@ -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, @@ -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 << ";"; } @@ -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 << ";"; } @@ -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 << ";"; } @@ -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 ----- */ @@ -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) @@ -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 @@ -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 << ";"; } @@ -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); diff --git a/storage/duckdb/convertor/dml_convertor.cc b/storage/duckdb/convertor/dml_convertor.cc index 76108e00d53aa..4685ed5f04d46 100644 --- a/storage/duckdb/convertor/dml_convertor.cc +++ b/storage/duckdb/convertor/dml_convertor.cc @@ -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()) @@ -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 &fields) @@ -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); @@ -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); @@ -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); diff --git a/storage/duckdb/docs/mariadb-duckdb-incompatibilities.md b/storage/duckdb/docs/mariadb-duckdb-incompatibilities.md index d391478119c5a..8288215bf56ac 100644 --- a/storage/duckdb/docs/mariadb-duckdb-incompatibilities.md +++ b/storage/duckdb/docs/mariadb-duckdb-incompatibilities.md @@ -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`): @@ -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. --- diff --git a/storage/duckdb/ha_duckdb.cc b/storage/duckdb/ha_duckdb.cc index 34400e647de91..e5b3b989e78df 100644 --- a/storage/duckdb/ha_duckdb.cc +++ b/storage/duckdb/ha_duckdb.cc @@ -37,7 +37,6 @@ #include "duckdb_select.h" #include "ddl_convertor.h" #include "dml_convertor.h" -#include "delta_appender.h" #include "row_helpers.h" #include "ha_duckdb_pushdown.h" #include "duckdb_log.h" @@ -212,9 +211,7 @@ static void duckdb_drop_database(handlerton *hton, char *path) Databasename db(path); - std::string query= "DROP SCHEMA IF EXISTS \""; - query.append(db.name); - query.append("\""); + std::string query= "DROP SCHEMA IF EXISTS " + quote_duckdb_identifier(db.name); if (duckdb_register_trx(thd)) DBUG_VOID_RETURN; @@ -544,10 +541,27 @@ int ha_duckdb::write_row(const uchar *buf) static ulonglong reserve_autoinc_block(THD *thd, const char *db_name, const char *table_name, ulonglong want) { + /* + 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 result for the enclosing + literal (doubling ') so a crafted schema/table name cannot inject SQL + (MDEV-40653). + */ + std::string qualified= + quote_duckdb_identifier(std::string(db_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); + } std::string query= - "SELECT min(x), max(x) FROM (SELECT nextval('\"" + - std::string(db_name) + "\".\"" + autoinc_sequence_name(table_name) + - "\"') AS x FROM range(" + std::to_string(want) + ")) t"; + "SELECT min(x), max(x) FROM (SELECT nextval('" + escaped + + "') AS x FROM range(" + std::to_string(want) + ")) t"; auto *ctx= get_duckdb_context(thd); for (int attempt= 0; attempt < 3; attempt++) @@ -775,8 +789,8 @@ int ha_duckdb::rnd_init(bool) else DBUG_RETURN(HA_ERR_INTERNAL_ERROR); - std::string query= - "SELECT * FROM \"" + schema_name + "\".\"" + table_name + "\""; + std::string query= "SELECT * FROM " + quote_duckdb_identifier(schema_name) + + "." + quote_duckdb_identifier(table_name); auto *ctx= get_duckdb_context(thd); query_result= myduck::duckdb_query(ctx->get_connection(), query); @@ -938,8 +952,8 @@ int ha_duckdb::delete_all_rows() ctx->delete_appender(dt.db_name, dt.table_name); /* Execute DELETE FROM "schema"."table" */ - std::string query= - "DELETE FROM \"" + dt.db_name + "\".\"" + dt.table_name + "\""; + std::string query= "DELETE FROM " + quote_duckdb_identifier(dt.db_name) + + "." + quote_duckdb_identifier(dt.table_name); auto query_result= myduck::duckdb_query(ctx->get_connection(), query); if (query_result->HasError()) @@ -951,13 +965,9 @@ int ha_duckdb::delete_all_rows() DBUG_RETURN(0); } -const COND *ha_duckdb::cond_push(const COND *cond) +const COND *ha_duckdb::cond_push(const COND *) { DBUG_ENTER("ha_duckdb::cond_push"); - /* - Accept all conditions — DuckDB will evaluate the WHERE clause - from the original SQL query in direct_delete_rows(). - */ DBUG_RETURN(NULL); } @@ -972,6 +982,15 @@ int ha_duckdb::direct_delete_rows(ha_rows *delete_rows) DBUG_ENTER("ha_duckdb::direct_delete_rows"); int ret= 0; THD *thd= ha_thd(); + LEX_STRING *source_query= thd_query_string(thd); + if (myduck::mariadb_query_has_unsafe_quote_escape( + thd, source_query->str, source_query->length)) + { + my_error(ER_GET_ERRMSG, MYF(0), HA_DUCKDB_DML_ERROR, + "Unsafe MariaDB backslash quote escape in forwarded SQL", + "DuckDB"); + DBUG_RETURN(HA_DUCKDB_DML_ERROR); + } ret= duckdb_register_trx(thd); if (ret) @@ -1021,6 +1040,15 @@ int ha_duckdb::direct_update_rows(ha_rows *update_rows, ha_rows *found_rows) DBUG_ENTER("ha_duckdb::direct_update_rows"); int ret= 0; THD *thd= ha_thd(); + LEX_STRING *source_query= thd_query_string(thd); + if (myduck::mariadb_query_has_unsafe_quote_escape( + thd, source_query->str, source_query->length)) + { + my_error(ER_GET_ERRMSG, MYF(0), HA_DUCKDB_DML_ERROR, + "Unsafe MariaDB backslash quote escape in forwarded SQL", + "DuckDB"); + DBUG_RETURN(HA_DUCKDB_DML_ERROR); + } ret= duckdb_register_trx(thd); if (ret) @@ -1135,8 +1163,9 @@ int ha_duckdb::delete_table(const char *name) DatabaseTableNames dt(name); - std::string query= - "DROP TABLE IF EXISTS \"" + dt.db_name + "\".\"" + dt.table_name + "\""; + std::string query= "DROP TABLE IF EXISTS " + + quote_duckdb_identifier(dt.db_name) + "." + + quote_duckdb_identifier(dt.table_name); auto *ctx= get_duckdb_context(thd); auto query_result= myduck::duckdb_query(ctx->get_connection(), query); @@ -1150,8 +1179,10 @@ int ha_duckdb::delete_table(const char *name) The name is derived from the table alone, so no lookup is needed and the statement is harmless for tables without AUTO_INCREMENT. */ - std::string seq_query= "DROP SEQUENCE IF EXISTS \"" + dt.db_name + "\".\"" + - autoinc_sequence_name(dt.table_name) + "\""; + std::string seq_query= "DROP SEQUENCE IF EXISTS " + + quote_duckdb_identifier(dt.db_name) + "." + + quote_duckdb_identifier( + autoinc_sequence_name(dt.table_name)); auto seq_result= myduck::duckdb_query(ctx->get_connection(), seq_query); if (seq_result == nullptr || seq_result->HasError()) @@ -1208,8 +1239,8 @@ int ha_duckdb::truncate() table->s->table_name.length); std::ostringstream query; - query << "USE \"" << schema_name << "\";"; - query << "TRUNCATE TABLE \"" << table_name << "\";"; + query << "USE " << quote_duckdb_identifier(schema_name) << ";"; + query << "TRUNCATE TABLE " << quote_duckdb_identifier(table_name) << ";"; auto *ctx= get_duckdb_context(thd); auto query_result= myduck::duckdb_query(ctx->get_connection(), query.str()); diff --git a/storage/duckdb/ha_duckdb_pushdown.cc b/storage/duckdb/ha_duckdb_pushdown.cc index c865162243ca3..74e00f823ad1b 100644 --- a/storage/duckdb/ha_duckdb_pushdown.cc +++ b/storage/duckdb/ha_duckdb_pushdown.cc @@ -106,6 +106,8 @@ static bool is_query_token(const std::string &sql, size_t start, size_t length) static bool extract_source_query(THD *thd, std::string &source) { const std::string sql(thd->query(), thd->query_length()); + if (myduck::mariadb_query_has_unsafe_quote_escape(thd, sql.data(), sql.size())) + return false; const bool strip_prefix= thd->lex->sql_command == SQLCOM_INSERT_SELECT; const bool backslash_escapes= thd->backslash_escapes(); size_t i= 0; @@ -119,48 +121,21 @@ static bool extract_source_query(THD *thd, std::string &source) i++; continue; } - if (c == '/' && i + 1 < sql.size() && sql[i + 1] == '*') - { - size_t end= sql.find("*/", i + 2); - if (end == std::string::npos) - return false; - i= end + 2; - continue; - } - if (c == '#' || - (c == '-' && i + 1 < sql.size() && sql[i + 1] == '-' && - (i + 2 == sql.size() || isspace((unsigned char) sql[i + 2])))) + size_t end; + myduck::SqlRegionType region= + myduck::scan_sql_region(sql, i, backslash_escapes, end); + if (region == myduck::SqlRegionType::UNTERMINATED) + return false; + if (region == myduck::SqlRegionType::COMMENT) { - size_t end= sql.find('\n', i + (c == '#' ? 1 : 2)); - i= end == std::string::npos ? sql.size() : end + 1; + i= end; continue; } - if (c == '\'' || c == '"' || c == '`') + if (region == myduck::SqlRegionType::QUOTED) { if (!strip_prefix && depth == 0) return false; - const char quote= (char) c; - bool closed= false; - for (i++; i < sql.size(); i++) - { - if (sql[i] == '\\' && backslash_escapes && i + 1 < sql.size()) - { - i++; - continue; - } - if (sql[i] != quote) - continue; - if (i + 1 < sql.size() && sql[i + 1] == quote) - { - i++; - continue; - } - i++; - closed= true; - break; - } - if (!closed) - return false; + i= end; continue; } if (c == '(') diff --git a/storage/duckdb/mysql-test/duckdb/r/duckdb_identifier_escaping.result b/storage/duckdb/mysql-test/duckdb/r/duckdb_identifier_escaping.result new file mode 100644 index 0000000000000..7e7a441c402ca --- /dev/null +++ b/storage/duckdb/mysql-test/duckdb/r/duckdb_identifier_escaping.result @@ -0,0 +1,102 @@ +CREATE DATABASE db_ident_esc; +USE db_ident_esc; +# +# 1) Column names containing a double quote (CREATE/INSERT/SELECT) +# +CREATE TABLE t1 (`a"b` INT PRIMARY KEY, `c"d` INT) ENGINE=DuckDB; +INSERT INTO t1 VALUES (1, 10), (2, 20); +SELECT * FROM t1 ORDER BY `a"b`; +a"b c"d +1 10 +2 20 +SELECT /* user's ' column */ * FROM t1 ORDER BY `a"b`; +a"b c"d +1 10 +2 20 +SELECT * -- user's ' columns +FROM t1 ORDER BY `a"b`; +a"b c"d +1 10 +2 20 +# +# 2) UPDATE/DELETE route the quoted names through the WHERE/SET builders +# +UPDATE t1 SET `c"d` = 99 WHERE `a"b` = 1; +SELECT * FROM t1 ORDER BY `a"b`; +a"b c"d +1 99 +2 20 +DELETE FROM t1 WHERE `a"b` = 2; +SELECT * FROM t1 ORDER BY `a"b`; +a"b c"d +1 99 +# +# 3) ALTER RENAME/ADD/DROP COLUMN with quoted names +# +ALTER TABLE t1 RENAME COLUMN `c"d` TO `e""f`; +SHOW CREATE TABLE t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a"b` int(11) NOT NULL, + `e""f` int(11) DEFAULT NULL, + PRIMARY KEY (`a"b`) +) ENGINE=DUCKDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_uca1400_ai_ci +SELECT `e""f` FROM t1; +e""f +99 +ALTER TABLE t1 ADD COLUMN `g"h` INT; +SELECT * FROM t1; +a"b e""f g"h +1 99 NULL +ALTER TABLE t1 DROP COLUMN `g"h`; +# +# 4) Table name containing a double quote +# +CREATE TABLE `x"y` (`id` INT PRIMARY KEY) ENGINE=DuckDB; +INSERT INTO `x"y` VALUES (1), (2); +SELECT * FROM `x"y` ORDER BY `id`; +id +1 +2 +TRUNCATE TABLE `x"y`; +SELECT COUNT(*) FROM `x"y`; +COUNT(*) +0 +DROP TABLE `x"y`; +# +# 5) Backslash quote-boundary mismatches disable raw SQL forwarding +# +CREATE TABLE lexical_guard (id INT PRIMARY KEY, v VARCHAR(100)) ENGINE=DuckDB; +INSERT INTO lexical_guard VALUES (1, 'safe'); +SELECT COUNT(*) FROM lexical_guard +WHERE v = 'missing\' OR true -- '; +COUNT(*) +0 +UPDATE lexical_guard SET v = 'changed\' WHERE true -- ' WHERE id = 999; +ERROR HY000: Got error 168 'Unsafe MariaDB backslash quote escape in forwarded SQL' from DuckDB +SELECT * FROM lexical_guard; +id v +1 safe +DELETE FROM lexical_guard WHERE v = 'missing\' OR true -- '; +ERROR HY000: Got error 168 'Unsafe MariaDB backslash quote escape in forwarded SQL' from DuckDB +SELECT COUNT(*) FROM lexical_guard; +COUNT(*) +1 +UPDATE lexical_guard SET v = 'safe\''; +ERROR HY000: Got error 168 'Unsafe MariaDB backslash quote escape in forwarded SQL' from DuckDB +SELECT * FROM lexical_guard; +id v +1 safe +UPDATE /* user's \' comment */ lexical_guard SET v = 'unchanged' WHERE id = 999; +UPDATE lexical_guard SET v = 'unchanged\n' WHERE id = 999; +UPDATE lexical_guard SET v = "changed\" WHERE true -- " WHERE id = 999; +ERROR HY000: Got error 168 'Unsafe MariaDB backslash quote escape in forwarded SQL' from DuckDB +SELECT * FROM lexical_guard; +id v +1 safe +DROP TABLE lexical_guard; +# +# 6) Cleanup +# +DROP TABLE t1; +DROP DATABASE db_ident_esc; diff --git a/storage/duckdb/mysql-test/duckdb/t/duckdb_identifier_escaping.test b/storage/duckdb/mysql-test/duckdb/t/duckdb_identifier_escaping.test new file mode 100644 index 0000000000000..3ffeb7adef4d8 --- /dev/null +++ b/storage/duckdb/mysql-test/duckdb/t/duckdb_identifier_escaping.test @@ -0,0 +1,88 @@ +# MDEV-40653: identifiers (schema/table/column names) must be escaped when +# building DuckDB SQL. DuckDB delimits identifiers with double quotes and +# escapes an embedded double quote by doubling it. A MariaDB identifier may +# contain a literal double quote, so without escaping a crafted name breaks +# out of the quoted identifier (parser error / SQL injection). + +--disable_query_log +SET @saved_duckdb_require_primary_key = @@GLOBAL.duckdb_require_primary_key; +SET @saved_sql_mode = @@SESSION.sql_mode; +SET GLOBAL duckdb_require_primary_key = OFF; +SET SESSION sql_mode = ''; +--enable_query_log + +CREATE DATABASE db_ident_esc; +USE db_ident_esc; + +--echo # +--echo # 1) Column names containing a double quote (CREATE/INSERT/SELECT) +--echo # +CREATE TABLE t1 (`a"b` INT PRIMARY KEY, `c"d` INT) ENGINE=DuckDB; +INSERT INTO t1 VALUES (1, 10), (2, 20); +SELECT * FROM t1 ORDER BY `a"b`; +SELECT /* user's ' column */ * FROM t1 ORDER BY `a"b`; +SELECT * -- user's ' columns +FROM t1 ORDER BY `a"b`; + +--echo # +--echo # 2) UPDATE/DELETE route the quoted names through the WHERE/SET builders +--echo # +UPDATE t1 SET `c"d` = 99 WHERE `a"b` = 1; +SELECT * FROM t1 ORDER BY `a"b`; +DELETE FROM t1 WHERE `a"b` = 2; +SELECT * FROM t1 ORDER BY `a"b`; + +--echo # +--echo # 3) ALTER RENAME/ADD/DROP COLUMN with quoted names +--echo # +ALTER TABLE t1 RENAME COLUMN `c"d` TO `e""f`; +SHOW CREATE TABLE t1; +SELECT `e""f` FROM t1; +ALTER TABLE t1 ADD COLUMN `g"h` INT; +SELECT * FROM t1; +ALTER TABLE t1 DROP COLUMN `g"h`; + +--echo # +--echo # 4) Table name containing a double quote +--echo # +CREATE TABLE `x"y` (`id` INT PRIMARY KEY) ENGINE=DuckDB; +INSERT INTO `x"y` VALUES (1), (2); +SELECT * FROM `x"y` ORDER BY `id`; +TRUNCATE TABLE `x"y`; +SELECT COUNT(*) FROM `x"y`; +DROP TABLE `x"y`; + +--echo # +--echo # 5) Backslash quote-boundary mismatches disable raw SQL forwarding +--echo # +CREATE TABLE lexical_guard (id INT PRIMARY KEY, v VARCHAR(100)) ENGINE=DuckDB; +INSERT INTO lexical_guard VALUES (1, 'safe'); +SELECT COUNT(*) FROM lexical_guard +WHERE v = 'missing\' OR true -- '; +--error ER_GET_ERRMSG +UPDATE lexical_guard SET v = 'changed\' WHERE true -- ' WHERE id = 999; +SELECT * FROM lexical_guard; +--error ER_GET_ERRMSG +DELETE FROM lexical_guard WHERE v = 'missing\' OR true -- '; +SELECT COUNT(*) FROM lexical_guard; +--error ER_GET_ERRMSG +UPDATE lexical_guard SET v = 'safe\''; +SELECT * FROM lexical_guard; +UPDATE /* user's \' comment */ lexical_guard SET v = 'unchanged' WHERE id = 999; +UPDATE lexical_guard SET v = 'unchanged\n' WHERE id = 999; +--error ER_GET_ERRMSG +UPDATE lexical_guard SET v = "changed\" WHERE true -- " WHERE id = 999; +SELECT * FROM lexical_guard; +DROP TABLE lexical_guard; + +--echo # +--echo # 6) Cleanup +--echo # +DROP TABLE t1; +DROP DATABASE db_ident_esc; + +--disable_query_log +SET GLOBAL duckdb_require_primary_key = @saved_duckdb_require_primary_key; +SET SESSION sql_mode = @saved_sql_mode; +--enable_query_log +--source ../include/cleanup_duckdb.inc diff --git a/storage/duckdb/runtime/delta_appender.cc b/storage/duckdb/runtime/delta_appender.cc index 3d6b91623b0b3..46c4259b64915 100644 --- a/storage/duckdb/runtime/delta_appender.cc +++ b/storage/duckdb/runtime/delta_appender.cc @@ -17,8 +17,9 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1335 USA */ +#define MYSQL_SERVER 1 + #include -#include "sql_class.h" #include "log.h" #undef UNKNOWN @@ -29,7 +30,6 @@ #include "ddl_convertor.h" #include "duckdb_timezone.h" #include "duckdb_handler_errors.h" -#include "tztime.h" #include "my_decimal.h" #include "duckdb/common/hugeint.hpp" @@ -214,15 +214,16 @@ bool DeltaAppender::Initialize(TABLE *table) m_tmp_table_name= buf_table_name(m_schema_name, m_table_name); std::stringstream ss; - ss << "CREATE TEMPORARY TABLE IF NOT EXISTS main.\"" << m_tmp_table_name - << "\" AS FROM \"" << m_schema_name << "\".\"" << m_table_name - << "\" LIMIT 0;"; - ss << "ALTER TABLE main.\"" << m_tmp_table_name - << "\" ADD COLUMN \"#mdb_delete_flag\" BOOL;"; - ss << "ALTER TABLE main.\"" << m_tmp_table_name - << "\" ADD COLUMN \"#mdb_row_no\" INT;"; - ss << "ALTER TABLE main.\"" << m_tmp_table_name - << "\" ADD COLUMN \"#mdb_trx_no\" INT;"; + ss << "CREATE TEMPORARY TABLE IF NOT EXISTS main." + << quote_duckdb_identifier(m_tmp_table_name) << " AS FROM " + << quote_duckdb_identifier(m_schema_name) << "." + << quote_duckdb_identifier(m_table_name) << " LIMIT 0;"; + ss << "ALTER TABLE main." << quote_duckdb_identifier(m_tmp_table_name) + << " ADD COLUMN \"#mdb_delete_flag\" BOOL;"; + ss << "ALTER TABLE main." << quote_duckdb_identifier(m_tmp_table_name) + << " ADD COLUMN \"#mdb_row_no\" INT;"; + ss << "ALTER TABLE main." << quote_duckdb_identifier(m_tmp_table_name) + << " ADD COLUMN \"#mdb_trx_no\" INT;"; auto ret= myduck::duckdb_query(*m_con, ss.str()); if (ret->HasError()) @@ -249,9 +250,8 @@ bool DeltaAppender::Initialize(TABLE *table) { if (i) m_pk_list+= ", "; - m_pk_list+= "\""; - m_pk_list+= key_part->field->field_name.str; - m_pk_list+= "\""; + m_pk_list+= quote_duckdb_identifier(key_part->field->field_name.str, + key_part->field->field_name.length); bitmap_set_bit(&m_pk_bitmap, key_part->field->field_index); } @@ -259,9 +259,8 @@ bool DeltaAppender::Initialize(TABLE *table) { if (i) m_col_list+= ", "; - m_col_list+= "\""; - m_col_list+= table->field[i]->field_name.str; - m_col_list+= "\""; + m_col_list+= quote_duckdb_identifier(table->field[i]->field_name.str, + table->field[i]->field_name.length); } } else @@ -479,8 +478,8 @@ static void appendSelectQuery(std::stringstream &ss, ss << "SELECT UNNEST(r) FROM (SELECT LAST(ROW(" << select_list << ") ORDER BY \"#mdb_row_no\") AS r, " "LAST(\"#mdb_delete_flag\" ORDER BY \"#mdb_row_no\") AS " - "\"#mdb_delete_flag\" FROM main.\"" - << table_name << "\" GROUP BY " << pk_list << ")"; + "\"#mdb_delete_flag\" FROM main." + << quote_duckdb_identifier(table_name) << " GROUP BY " << pk_list << ")"; if (!delete_flag) ss << " WHERE \"#mdb_delete_flag\" = " << delete_flag; } @@ -488,20 +487,21 @@ static void appendSelectQuery(std::stringstream &ss, void DeltaAppender::generateQuery(std::stringstream &ss, bool delete_flag) { ss.str(""); - ss << "USE \"" << m_schema_name << "\"; "; + ss << "USE " << quote_duckdb_identifier(m_schema_name) << "; "; if (!delete_flag) { - ss << "INSERT INTO \"" << m_schema_name << "\".\"" << m_table_name - << "\" "; + ss << "INSERT INTO " << quote_duckdb_identifier(m_schema_name) << "." + << quote_duckdb_identifier(m_table_name) << " "; appendSelectQuery(ss, m_col_list, m_pk_list, m_tmp_table_name, delete_flag); ss << ";"; } else { - ss << "DELETE FROM \"" << m_schema_name << "\".\"" << m_table_name - << "\" WHERE (" << m_pk_list << ") IN ("; + ss << "DELETE FROM " << quote_duckdb_identifier(m_schema_name) << "." + << quote_duckdb_identifier(m_table_name) << " WHERE (" << m_pk_list + << ") IN ("; appendSelectQuery(ss, m_pk_list, m_pk_list, m_tmp_table_name, delete_flag); ss << ");"; } @@ -532,7 +532,7 @@ bool DeltaAppender::flush(bool idempotent_flag) } ss.str(""); - ss << "DROP TABLE main.\"" << m_tmp_table_name << "\""; + ss << "DROP TABLE main." << quote_duckdb_identifier(m_tmp_table_name); auto ret= myduck::duckdb_query(*m_con, ss.str()); if (ret->HasError()) return true; @@ -553,7 +553,8 @@ void DeltaAppender::cleanup() { my_bitmap_free(&m_pk_bitmap); std::stringstream ss; - ss << "DROP TABLE IF EXISTS main.\"" << m_tmp_table_name << "\";"; + ss << "DROP TABLE IF EXISTS main." << quote_duckdb_identifier(m_tmp_table_name) + << ";"; myduck::duckdb_query(*m_con, ss.str()); } } diff --git a/storage/duckdb/runtime/duckdb_context.cc b/storage/duckdb/runtime/duckdb_context.cc index 9d1f05557c216..03d8da01ddc49 100644 --- a/storage/duckdb/runtime/duckdb_context.cc +++ b/storage/duckdb/runtime/duckdb_context.cc @@ -65,8 +65,9 @@ void DuckdbThdContext::config_duckdb_env(const std::string &schema) if (schema.empty() || schema == m_current_schema) return; - std::string sql1= "CREATE SCHEMA IF NOT EXISTS \"" + schema + "\""; - std::string sql2= "USE \"" + schema + "\""; + std::string sql1= "CREATE SCHEMA IF NOT EXISTS " + + quote_duckdb_identifier(schema); + std::string sql2= "USE " + quote_duckdb_identifier(schema); m_current_schema= schema; for (auto &sql : {sql1, sql2}) diff --git a/storage/duckdb/runtime/duckdb_query.cc b/storage/duckdb/runtime/duckdb_query.cc index 9c177679a01f9..2b1236b52d559 100644 --- a/storage/duckdb/runtime/duckdb_query.cc +++ b/storage/duckdb/runtime/duckdb_query.cc @@ -30,17 +30,184 @@ #include "duckdb_manager.h" #include "duckdb_log.h" +#include + extern handlerton *duckdb_hton; namespace myduck { +SqlRegionType scan_sql_region(const std::string &sql, size_t start, + bool backslash_escapes, size_t &end) +{ + end= start; + if (start >= sql.size()) + return SqlRegionType::NONE; + + char c= sql[start]; + if (c == '/' && start + 1 < sql.size() && sql[start + 1] == '*') + { + size_t close= sql.find("*/", start + 2); + if (close == std::string::npos) + { + end= sql.size(); + return SqlRegionType::UNTERMINATED; + } + end= close + 2; + return SqlRegionType::COMMENT; + } + + if (c == '#' || + (c == '-' && start + 1 < sql.size() && sql[start + 1] == '-' && + (start + 2 == sql.size() || + isspace(static_cast(sql[start + 2]))))) + { + size_t newline= sql.find('\n', start + (c == '#' ? 1 : 2)); + end= newline == std::string::npos ? sql.size() : newline + 1; + return SqlRegionType::COMMENT; + } + + if (c != '\'' && c != '"' && c != '`') + return SqlRegionType::NONE; + + for (size_t i= start + 1; i < sql.size(); i++) + { + if (sql[i] == '\\' && backslash_escapes && i + 1 < sql.size()) + { + i++; + continue; + } + if (sql[i] != c) + continue; + if (i + 1 < sql.size() && sql[i + 1] == c) + { + i++; + continue; + } + end= i + 1; + return SqlRegionType::QUOTED; + } + + end= sql.size(); + return SqlRegionType::UNTERMINATED; +} + +bool mariadb_query_has_unsafe_quote_escape(THD *thd, const char *query, + size_t length) +{ + if (!thd->backslash_escapes() || length == 0) + return false; + + const std::string sql(query, length); + const bool ansi_quotes= thd->variables.sql_mode & MODE_ANSI_QUOTES; + for (size_t i= 0; i < sql.size();) + { + size_t duckdb_end; + SqlRegionType duckdb_region= scan_sql_region(sql, i, false, duckdb_end); + if (duckdb_region == SqlRegionType::COMMENT) + { + i= duckdb_end; + continue; + } + + const bool string_literal= + sql[i] == '\'' || (sql[i] == '"' && !ansi_quotes); + if (string_literal) + { + size_t mariadb_end; + SqlRegionType mariadb_region= + scan_sql_region(sql, i, true, mariadb_end); + if (mariadb_region != duckdb_region || mariadb_end != duckdb_end) + return true; + i= mariadb_end; + continue; + } + + if (duckdb_region == SqlRegionType::QUOTED) + i= duckdb_end; + else if (duckdb_region == SqlRegionType::UNTERMINATED) + return true; + else + i++; + } + return false; +} + +/* + Convert MariaDB's printed SQL (backtick-quoted identifiers) into DuckDB SQL + (double-quoted identifiers). + + MariaDB delimits identifiers with backticks and doubles an embedded backtick; + DuckDB delimits with double quotes and doubles an embedded double quote. A + naive character-by-character swap breaks identifiers that contain a double + quote (MDEV-40653) and also corrupts backticks that appear inside string + literals. Walk the string instead: copy string literals and already + double-quoted identifiers verbatim, and rewrite only backtick-delimited + identifiers, escaping any embedded double quote. +*/ static std::string backticks_to_double_quotes(const std::string &sql) { - std::string out(sql); - for (auto &ch : out) - if (ch == '`') - ch= '"'; + std::string out; + out.reserve(sql.size()); + const size_t n= sql.size(); + size_t i= 0; + + while (i < n) + { + char c= sql[i]; + size_t end; + SqlRegionType region= scan_sql_region(sql, i, false, end); + + if (region == SqlRegionType::COMMENT || + region == SqlRegionType::UNTERMINATED) + { + out.append(sql, i, end - i); + i= end; + continue; + } + + /* Single-quoted string literal: copy verbatim ('' and \' escapes). */ + if (region == SqlRegionType::QUOTED && c == '\'') + { + out.append(sql, i, end - i); + i= end; + continue; + } + + /* Already double-quoted identifier: copy verbatim ("" escape). */ + if (region == SqlRegionType::QUOTED && c == '"') + { + out.append(sql, i, end - i); + i= end; + continue; + } + + /* Backtick identifier: rewrite as a double-quoted identifier. */ + if (region == SqlRegionType::QUOTED && c == '`') + { + out.push_back('"'); + for (i++; i + 1 < end; i++) + { + char d= sql[i]; + if (d == '`' && i + 2 < end && sql[i + 1] == '`') + { + out.push_back('`'); + i++; + continue; + } + if (d == '"') + out.push_back('"'); /* escape " inside a DuckDB identifier */ + out.push_back(d); + } + out.push_back('"'); + i= end; + continue; + } + + out.push_back(c); + i++; + } + return out; } @@ -116,6 +283,10 @@ static std::string get_thd_schema(THD *thd) duckdb::unique_ptr duckdb_query(THD *thd, const std::string &query, bool need_config) { + if (mariadb_query_has_unsafe_quote_escape(thd, query.data(), query.size())) + return duckdb::make_uniq( + duckdb::ErrorData("Unsafe MariaDB backslash quote escape in forwarded SQL")); + auto *ctx= static_cast(thd_get_ha_data(thd, duckdb_hton)); if (!ctx) @@ -136,6 +307,10 @@ duckdb_query(THD *thd, const std::string &query, bool need_config) duckdb::unique_ptr duckdb_stream_query(THD *thd, const std::string &query, bool need_config) { + if (mariadb_query_has_unsafe_quote_escape(thd, query.data(), query.size())) + return duckdb::make_uniq( + duckdb::ErrorData("Unsafe MariaDB backslash quote escape in forwarded SQL")); + auto *ctx= static_cast(thd_get_ha_data(thd, duckdb_hton)); if (!ctx) diff --git a/storage/duckdb/runtime/duckdb_query.h b/storage/duckdb/runtime/duckdb_query.h index 21f8cd12f9491..6ec9db6da7897 100644 --- a/storage/duckdb/runtime/duckdb_query.h +++ b/storage/duckdb/runtime/duckdb_query.h @@ -19,6 +19,7 @@ #pragma once +#include #include #include #include @@ -31,6 +32,20 @@ class THD; namespace myduck { +enum class SqlRegionType +{ + NONE, + COMMENT, + QUOTED, + UNTERMINATED +}; + +SqlRegionType scan_sql_region(const std::string &sql, size_t start, + bool backslash_escapes, size_t &end); + +bool mariadb_query_has_unsafe_quote_escape(THD *thd, const char *query, + size_t length); + duckdb::unique_ptr duckdb_query(duckdb::Connection &connection, const std::string &query); From 09b0996e5f848430007f2551db221ee30da5192b Mon Sep 17 00:00:00 2001 From: drrtuy Date: Fri, 7 Aug 2026 17:43:00 +0100 Subject: [PATCH 2/2] fix: hardening against unknown SQL injection vectors running DuckDB. --- storage/duckdb/runtime/duckdb_query.cc | 34 +++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/storage/duckdb/runtime/duckdb_query.cc b/storage/duckdb/runtime/duckdb_query.cc index 2b1236b52d559..a3d9030e3fe76 100644 --- a/storage/duckdb/runtime/duckdb_query.cc +++ b/storage/duckdb/runtime/duckdb_query.cc @@ -26,6 +26,7 @@ #include "duckdb_query.h" #include "duckdb/common/exception.hpp" +#include "duckdb/main/pending_query_result.hpp" #include "duckdb_context.h" #include "duckdb_manager.h" #include "duckdb_log.h" @@ -244,8 +245,9 @@ duckdb_query(duckdb::Connection &connection, const std::string &query) } } -duckdb::unique_ptr -duckdb_stream_query(duckdb::Connection &connection, const std::string &query) +static duckdb::unique_ptr +duckdb_pending_query(duckdb::Connection &connection, const std::string &query, + duckdb::QueryResultOutputType output_type) { const std::string q= backticks_to_double_quotes(query); @@ -254,7 +256,13 @@ duckdb_stream_query(duckdb::Connection &connection, const std::string &query) try { - auto res= connection.SendQuery(q, duckdb::QueryResultOutputType::ALLOW_STREAMING); + auto pending= connection.PendingQuery(q, output_type); + duckdb::unique_ptr res; + if (pending->HasError()) + res= duckdb::make_uniq( + pending->GetErrorObject()); + else + res= pending->Execute(); if ((myduck::duckdb_log_options & LOG_DUCKDB_QUERY_RESULT) && res->HasError()) @@ -273,6 +281,24 @@ duckdb_stream_query(duckdb::Connection &connection, const std::string &query) } } +static duckdb::unique_ptr +duckdb_query_single(duckdb::Connection &connection, const std::string &query) +{ + auto res= duckdb_pending_query( + connection, query, duckdb::QueryResultOutputType::FORCE_MATERIALIZED); + DBUG_ASSERT(res->type == duckdb::QueryResultType::MATERIALIZED_RESULT); + return duckdb::unique_ptr_cast( + std::move(res)); +} + +duckdb::unique_ptr +duckdb_stream_query(duckdb::Connection &connection, const std::string &query) +{ + return duckdb_pending_query( + connection, query, duckdb::QueryResultOutputType::ALLOW_STREAMING); +} + static std::string get_thd_schema(THD *thd) { if (thd->db.str && thd->db.length > 0) @@ -301,7 +327,7 @@ duckdb_query(THD *thd, const std::string &query, bool need_config) ctx->config_duckdb_session(thd); } - return duckdb_query(ctx->get_connection(), query); + return duckdb_query_single(ctx->get_connection(), query); } duckdb::unique_ptr