proxysql

Commit Graph

Author	SHA1	Message	Date
Rene Cannao	9d8c6b77b3	fix(security): close MCP run_sql_readonly multi-statement bypass (GHSA-7wh6-2vcc-gcm4) The MCP /mcp/query endpoint advertised a read-only contract for the run_sql_readonly tool, but enforcement was a substring-only validator on the first keyword while the backend MySQL connection was opened with CLIENT_MULTI_STATEMENTS. A payload like SELECT 1; RENAME TABLE testdb.x TO testdb.y passed validation (starts with SELECT) and executed both statements, renaming the backend table. The pre-fix substring blacklist also omitted several side-effecting MySQL keywords (RENAME, SET, RESET, LOCK, UNLOCK, KILL, FLUSH, OPTIMIZE, REPAIR, HANDLER, ...) so the bypass also worked as a single statement when those keywords appeared at the start. Fixes applied in defense-in-depth order: 1. Drop CLIENT_MULTI_STATEMENTS from the MCP backend mysql_real_connect calls in Query_Tool_Handler.cpp:init_connection_pool and backend_client.cpp:dial_mysql. This alone closes the demonstrated PoC end-to-end: the server-side parser will reject the trailing statement as a syntax error. 2. Add Query_Tool_Handler::contains_multi_statement() which tracks lexical state (single/double-quoted strings, backtick-quoted identifiers, '--' line comments, '/* */' block comments) and rejects any ';' followed by non-whitespace. Call it at the top of validate_readonly_query so multi-statement payloads are rejected before they ever reach the backend. 3. Expand the dangerous-keyword denylist to cover RENAME, RESET, LOCK, UNLOCK, KILL, FLUSH, GRANT, REVOKE, OPTIMIZE, REPAIR, HANDLER, INSTALL/UNINSTALL, CHANGE, PURGE, SAVEPOINT/RELEASE, transaction control (BEGIN, START, COMMIT, ROLLBACK, XA), SHUTDOWN, INTO OUTFILE/DUMPFILE. Reject 'SET ' and 'USE ' as standalone leading keywords (they appear legitimately as substrings inside SELECT, but never as the first token of a read-only query). 4. Reject 'EXPLAIN ANALYZE' explicitly: PostgreSQL EXPLAIN ANALYZE actually executes the wrapped statement, and the same applies to some MySQL configurations. 5. Apply the same validation in the explain_sql handler, which previously did no read-only validation at all. An MCP caller could send "SELECT 1; RENAME TABLE ..." through explain_sql, the handler would prepend "EXPLAIN " and execute the multi-statement payload. Operator note: this only closes the bypass. /mcp/query authentication defaults to mcp-query_endpoint_auth="" (no auth) when MCP is enabled, which makes any deployment exposing /mcp/query reachable to an unauthenticated network attacker. Hardening that default is tracked separately.	2 months ago
Rene Cannao	b501fa17dc	genai: harden real persistence and connection bugs Fix the remaining concrete defects from the GenAI review pass: - serialize MCP variable access with the handler rwlock instead of relying on external callers - make GenAI/MCP save-to-admin operations transactional and fail closed on SQLite errors - clamp genai_log() newline handling after truncation - use PQconnectdbParams() for libpq connections so credentials and schema values are passed safely - install and uninstall the GenAI plugin alongside mysqlx in the top-level Makefile - add a regression test for stale output handling in get_variable_string() These changes keep the runtime and persistence paths consistent with the branch's plugin lifecycle fixes.	2 months ago
Rene Cannao	4080606672	feat(plugins/genai): Step 4.B — add backend_client helper for local-proxy connections Per docs/superpowers/plans/2026-04-19-step4-mcp-subsystem-move.md "Sub-step 4.B", introduces the helper that 4.D will use to rewire Query_Tool_Handler and MySQL_Tool_Handler from direct-to-backend dialing to dialing 127.0.0.1:<local-proxy-port>. This commit is functionally a no-op: nothing in the plugin or core calls the new helper yet. The .so links it in for future use; only the unit test exercises it. New files: plugins/genai/include/backend_client.h BackendTarget, MySQL/PgSQLDialResult, dial_, dial__local, LocalProxyEndpoint, parse_interfaces_first_tcp, resolve__endpoint. plugins/genai/src/backend_client.cpp dial_mysql / dial_pgsql: thin wrappers around mysql_real_connect / PQconnectdb mirroring the dial sequence currently inlined in Query_Tool_Handler.cpp. No CLIENT_MULTI_STATEMENTS change of semantics relative to existing code. dial__local: resolve local proxy endpoint via admindb, then dial. plugins/genai/src/local_proxy_endpoint.cpp Reads mysql-interfaces / pgsql-interfaces from global_variables, parses the semicolon-separated host:port format (mirrors MySQL_Thread::listener_add's parser at lib/MySQL_Thread.cpp ~L240), canonicalises 0.0.0.0 -> 127.0.0.1 and :: -> ::1, skips Unix-socket tokens. No caching today; trivial to add when 4.D wires real callers. test/tap/tests/unit/genai_plugin_backend_client_unit-t.cpp 27 TAP assertions: parser variants (IPv4/IPv6/wildcards/multi/Unix- skip/whitespace/port-bounds), null-admindb guards on every public entry point, invalid-host/zero-port guards on direct-dial. Full MySQL handshake left for the 4.D integration coverage. Plumbing: - plugins/genai/Makefile: compile both new .cpp into the .so. - test/tap/tests/unit/Makefile: build genai_plugin_backend_client_unit-t using the same compile-twice trick as genai_plugin_anomaly_unit-t. - test/tap/groups/groups.json: register the new test in unit-tests-g1 with @proxysql_min_version:4.0. Resolves Q-svcuser and Q-localhost-port from the carve-out spec design, choosing existing mcp_auth_profiles credentials and admindb-driven endpoint discovery respectively (no new mechanisms).	2 months ago

Author

SHA1

Message

Date

Rene Cannao

9d8c6b77b3

fix(security): close MCP run_sql_readonly multi-statement bypass (GHSA-7wh6-2vcc-gcm4)

The MCP /mcp/query endpoint advertised a read-only contract for the
run_sql_readonly tool, but enforcement was a substring-only validator on
the first keyword while the backend MySQL connection was opened with
CLIENT_MULTI_STATEMENTS.  A payload like

    SELECT 1; RENAME TABLE testdb.x TO testdb.y

passed validation (starts with SELECT) and executed both statements,
renaming the backend table.  The pre-fix substring blacklist also
omitted several side-effecting MySQL keywords (RENAME, SET, RESET, LOCK,
UNLOCK, KILL, FLUSH, OPTIMIZE, REPAIR, HANDLER, ...) so the bypass also
worked as a single statement when those keywords appeared at the start.

Fixes applied in defense-in-depth order:

1. Drop CLIENT_MULTI_STATEMENTS from the MCP backend mysql_real_connect
   calls in Query_Tool_Handler.cpp:init_connection_pool and
   backend_client.cpp:dial_mysql.  This alone closes the demonstrated
   PoC end-to-end: the server-side parser will reject the trailing
   statement as a syntax error.

2. Add Query_Tool_Handler::contains_multi_statement() which tracks
   lexical state (single/double-quoted strings, backtick-quoted
   identifiers, '--' line comments, '/* */' block comments) and rejects
   any ';' followed by non-whitespace.  Call it at the top of
   validate_readonly_query so multi-statement payloads are rejected
   before they ever reach the backend.

3. Expand the dangerous-keyword denylist to cover RENAME, RESET, LOCK,
   UNLOCK, KILL, FLUSH, GRANT, REVOKE, OPTIMIZE, REPAIR, HANDLER,
   INSTALL/UNINSTALL, CHANGE, PURGE, SAVEPOINT/RELEASE, transaction
   control (BEGIN, START, COMMIT, ROLLBACK, XA), SHUTDOWN, INTO
   OUTFILE/DUMPFILE.  Reject 'SET ' and 'USE ' as standalone leading
   keywords (they appear legitimately as substrings inside SELECT, but
   never as the first token of a read-only query).

4. Reject 'EXPLAIN ANALYZE' explicitly: PostgreSQL EXPLAIN ANALYZE
   actually executes the wrapped statement, and the same applies to
   some MySQL configurations.

5. Apply the same validation in the explain_sql handler, which
   previously did no read-only validation at all.  An MCP caller could
   send "SELECT 1; RENAME TABLE ..." through explain_sql, the handler
   would prepend "EXPLAIN " and execute the multi-statement payload.

Operator note: this only closes the bypass.  /mcp/query authentication
defaults to mcp-query_endpoint_auth="" (no auth) when MCP is enabled,
which makes any deployment exposing /mcp/query reachable to an
unauthenticated network attacker.  Hardening that default is tracked
separately.

Rene Cannao

b501fa17dc

genai: harden real persistence and connection bugs

Fix the remaining concrete defects from the GenAI review pass:

- serialize MCP variable access with the handler rwlock instead of relying on external callers
- make GenAI/MCP save-to-admin operations transactional and fail closed on SQLite errors
- clamp genai_log() newline handling after truncation
- use PQconnectdbParams() for libpq connections so credentials and schema values are passed safely
- install and uninstall the GenAI plugin alongside mysqlx in the top-level Makefile
- add a regression test for stale output handling in get_variable_string()

These changes keep the runtime and persistence paths consistent with the branch's plugin lifecycle fixes.

Rene Cannao

4080606672

feat(plugins/genai): Step 4.B — add backend_client helper for local-proxy connections

Per docs/superpowers/plans/2026-04-19-step4-mcp-subsystem-move.md
"Sub-step 4.B", introduces the helper that 4.D will use to rewire
Query_Tool_Handler and MySQL_Tool_Handler from direct-to-backend dialing
to dialing 127.0.0.1:<local-proxy-port>.

This commit is functionally a no-op: nothing in the plugin or core
calls the new helper yet.  The .so links it in for future use; only
the unit test exercises it.

New files:

  plugins/genai/include/backend_client.h
    BackendTarget, MySQL/PgSQLDialResult, dial_*, dial_*_local,
    LocalProxyEndpoint, parse_interfaces_first_tcp,
    resolve_*_endpoint.

  plugins/genai/src/backend_client.cpp
    dial_mysql / dial_pgsql: thin wrappers around mysql_real_connect /
    PQconnectdb mirroring the dial sequence currently inlined in
    Query_Tool_Handler.cpp.  No CLIENT_MULTI_STATEMENTS change of
    semantics relative to existing code.
    dial_*_local: resolve local proxy endpoint via admindb, then dial.

  plugins/genai/src/local_proxy_endpoint.cpp
    Reads mysql-interfaces / pgsql-interfaces from global_variables,
    parses the semicolon-separated host:port format (mirrors
    MySQL_Thread::listener_add's parser at lib/MySQL_Thread.cpp ~L240),
    canonicalises 0.0.0.0 -> 127.0.0.1 and :: -> ::1, skips Unix-socket
    tokens.  No caching today; trivial to add when 4.D wires real
    callers.

  test/tap/tests/unit/genai_plugin_backend_client_unit-t.cpp
    27 TAP assertions: parser variants (IPv4/IPv6/wildcards/multi/Unix-
    skip/whitespace/port-bounds), null-admindb guards on every public
    entry point, invalid-host/zero-port guards on direct-dial.  Full
    MySQL handshake left for the 4.D integration coverage.

Plumbing:
  - plugins/genai/Makefile: compile both new .cpp into the .so.
  - test/tap/tests/unit/Makefile: build genai_plugin_backend_client_unit-t
    using the same compile-twice trick as genai_plugin_anomaly_unit-t.
  - test/tap/groups/groups.json: register the new test in unit-tests-g1
    with @proxysql_min_version:4.0.

Resolves Q-svcuser and Q-localhost-port from the carve-out spec design,
choosing existing mcp_auth_profiles credentials and admindb-driven
endpoint discovery respectively (no new mechanisms).

3 Commits (docs/passthrough-auth-spec)