fix(sqlite): prevent SQL injection in describe_table and query tools#3663
Open
xr843 wants to merge 1 commit intomodelcontextprotocol:mainfrom
Open
fix(sqlite): prevent SQL injection in describe_table and query tools#3663xr843 wants to merge 1 commit intomodelcontextprotocol:mainfrom
xr843 wants to merge 1 commit intomodelcontextprotocol:mainfrom
Conversation
The describe_table tool constructed SQL using f-string interpolation without sanitizing the table_name parameter, allowing injection attacks like: `users); DROP TABLE users; --` Fix applies defense-in-depth with two layers: 1. Regex validation rejects identifiers with non-alphanumeric chars 2. Whitelist validation checks table_name against sqlite_master using a parameterized query before use Also wraps the table name in SQLite bracket-quoting as an extra guard. Note: The sqlite server was moved to servers-archived (which is now read-only), so this PR re-introduces the fixed source files. The maintainers may prefer to apply this patch differently. Fixes modelcontextprotocol#3314 Ref: OWASP A03:2021 (Injection) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.
Summary
describe_tabletool (OWASP A03:2021 — Injection)table_nameparameter was interpolated directly into a PRAGMA query via f-string without any sanitizationsqlite_master+ bracket quotingFixes #3314
Vulnerability
The
describe_tabletool atsrc/sqlite/src/mcp_server_sqlite/server.pyconstructed SQL using unsanitized string interpolation:An attacker could provide a crafted
table_namelikeusers); DROP TABLE users; --to execute arbitrary SQL.Fix — Defense-in-Depth
1. Regex identifier validation (
_validate_identifier)Rejects any identifier containing characters other than
[a-zA-Z0-9_.], blocking SQL metacharacters (;,),',--).2. Whitelist validation against
sqlite_master(_validate_table_name)Uses a parameterized query to verify the table actually exists before using it:
3. Bracket-quoted identifier
The validated table name is wrapped in SQLite bracket quoting (
[table_name]) as a final safeguard:Note on repo structure
The SQLite MCP server was previously archived to
servers-archived(which is now read-only and cannot accept PRs). This PR re-introduces the sqlite source with the security fix applied. Maintainers may prefer to apply this patch to the archived repo directly or handle it differently.Test plan
describe_tableworks correctly with valid table namesusers); DROP TABLE users; --are rejected with a clear error;,',",-) are rejected by the regex validator