fix(tools/clickhouse,tools/bigquery): validate identifier parameters to prevent injection by evilgensec · Pull Request #3219 · googleapis/mcp-toolbox

evilgensec · 2026-05-12T18:41:04Z

Summary

This PR fixes identifier-injection vulnerabilities in three built-in tools that interpolate caller-supplied parameters directly into SQL without sanitisation.

1. `clickhouse-list-tables` — database identifier injection (original fix)

Tool.Invoke interpolates the caller-supplied database parameter directly into a SHOW TABLES FROM %s statement:

query := fmt.Sprintf("SHOW TABLES FROM %s", database)

The existing comment acknowledges the risk. The fix adds a validIdentifier regex (^[A-Za-z_][A-Za-z0-9_]*$) that must match before interpolation.

Concrete payloads on a default ClickHouse deployment:

`database` parameter	rendered query	effect
`default LIKE '%secret%'`	`SHOW TABLES FROM default LIKE '%secret%'`	filters listing by attacker-chosen pattern
`system FORMAT JSONEachRow`	`SHOW TABLES FROM system FORMAT JSONEachRow`	enumerates `system.*` (system.users, etc.)
`default INTO OUTFILE '/tmp/x.tsv'`	`SHOW TABLES FROM default INTO OUTFILE '/tmp/x.tsv'`	writes to disk on ClickHouse host

2. `bigquery-forecast` — backtick + column-name injection

Two injection classes:

a. Backtick injection in history_data (table identifier path)

historyDataSource = fmt.Sprintf("TABLE `%s`", historyData)

When allowed_datasets is not configured (the default), no validation runs before this line. An attacker-supplied history_data containing a backtick closes the opening delimiter and appends arbitrary SQL:

history_data = "ds.tbl` UNION ALL SELECT secret FROM private.pii --"
→  TABLE `ds.tbl` UNION ALL SELECT secret FROM private.pii --`

b. Column-name injection in data_col, timestamp_col, id_cols

sql := fmt.Sprintf(`... data_col => '%s', timestamp_col => '%s' ...`, dataCol, timestampCol, ...)

Column names are interpolated as single-quoted strings with no validation. A value containing ' breaks out of the string literal context.

Fix: ValidTableID (restricts history_data to [a-zA-Z0-9_]+ components with 1–2 dots) applied in the table-ID else-branch; ValidColumnName (^[a-zA-Z_][a-zA-Z0-9_]*$) applied to data_col, timestamp_col, and each element of id_cols.

3. `bigquery-analyze-contribution` — backtick + OPTIONS injection

a. Backtick injection in input_data (table identifier path)

inputDataSource = fmt.Sprintf("SELECT * FROM `%s`", inputData)

Same root cause as bigquery-forecast: when allowed_datasets is absent, no validation before interpolation.

b. Column-name and OPTIONS injection

options = append(options, fmt.Sprintf("IS_TEST_COL = '%s'", paramsMap["is_test_col"]))
// and each element of dimension_id_cols:
strCols = append(strCols, fmt.Sprintf("'%s'", c))

is_test_col and dimension_id_cols are column names interpolated into BigQuery ML OPTIONS() string literals without validation.

contribution_metric (e.g. SUM(col)/COUNT(DISTINCT col2)) goes into OPTIONS(CONTRIBUTION_METRIC = '%s'). A single-quote in this value breaks the OPTIONS string literal.

Fix: ValidTableID for input_data in the table-ID else-branch; ValidColumnName for is_test_col and each dimension_id_cols element; single-quote check for contribution_metric.

Shared validators in `bigquerycommon`

ValidTableID and ValidColumnName are exported from the bigquerycommon package so both tools share the same rules. Tests are in validators_test.go.

Test plan

go build ./... clean
go test ./internal/tools/bigquery/... -v passes (all existing tests + new TestValidTableID, TestValidColumnName)
go test ./internal/tools/clickhouse/clickhouselisttables/... -v passes

Not a dupe of #2811 / #779

Those cover the templateParameter flow (internal/util/parameters/parameters.go:applyEscape). The three tools fixed here use hard-coded fmt.Sprintf interpolation in their own Invoke methods — none of the templateParameter mitigations apply.

gemini-code-assist

Code Review

This pull request implements input validation for the database parameter in the ClickHouse list-tables tool to prevent SQL injection. It introduces a regular expression to restrict the parameter to plain identifiers before it is interpolated into the SQL statement and includes comprehensive unit tests for this validation logic. I have no feedback to provide.

`Tool.Invoke` interpolates the caller-supplied `database` parameter directly into a `SHOW TABLES FROM %s` statement: query := fmt.Sprintf("SHOW TABLES FROM %s", database) The existing comment acknowledges the risk ("formatting identifier directly is risky if input is untrusted"). The parameter framework binds `?` placeholders for values but does not support binding an identifier in the FROM position, so the only safe options are a strict identifier check or a refactor onto `system.tables`. This change adds the identifier check. Without it, any MCP client (or a prompt-injected LLM acting on a client's behalf) can supply a value that escapes the intended scope of the tool. Concrete payloads on a default ClickHouse deployment: database = "default LIKE '%secret%'" -> `SHOW TABLES FROM default LIKE '%secret%'`, scans the default database for table names matching a secret pattern. database = "default LIMIT 0 FORMAT JSON" -> rewrites the output format the calling layer is parsing. database = "system FORMAT JSONEachRow" -> enumerates the `system` database (system.users, system.parts, etc.), which the tool's documented contract does not permit and which typically holds cluster-wide metadata. database = "default INTO OUTFILE '/tmp/x.tsv'" -> on a server that accepts INTO OUTFILE for SHOW, writes to disk on the ClickHouse host. The fix is a one-line regex that constrains `database` to the ClickHouse unquoted identifier grammar `[A-Za-z_][A-Za-z0-9_]*`. Values that need quoting (which ClickHouse allows for unusual names) are out of scope of this tool and can be handled by a follow-up using `system.tables WHERE database = ?` with parameter binding. Tests cover the identifier regex against plain identifiers and against the injection payloads above.

…nalyze-contribution Both tools interpolate caller-supplied parameters directly into backtick-quoted BigQuery SQL identifiers and single-quoted column-name string arguments, with no character-level sanitisation. bigqueryforecast.go:243 historyDataSource = fmt.Sprintf("TABLE `%s`", historyData) bigqueryanalyzecontribution.go:262 inputDataSource = fmt.Sprintf("SELECT * FROM `%s`", inputData) When the allowed_datasets restriction is not configured (the default), neither path performs any validation before interpolation. An MCP client or a prompt-injected LLM can supply a value containing a backtick to close the opening identifier delimiter and append arbitrary SQL: history_data = "ds.tbl` UNION ALL SELECT secret FROM private.pii --" → TABLE `ds.tbl` UNION ALL SELECT secret FROM private.pii --` The result is a UNION-based injection that reads from tables outside the tool's intended scope. Column-name parameters (data_col, timestamp_col, id_cols in bigquery-forecast; is_test_col, dimension_id_cols in bigquery-analyze-contribution) are similarly unvalidated and are interpolated as single-quoted strings in AI.FORECAST and CREATE MODEL OPTIONS arguments. Fix: - Add ValidTableID and ValidColumnName to bigquerycommon. ValidTableID restricts input_data / history_data (in the table-ID branch) to the character set that BigQuery allows for dataset and table IDs ([a-zA-Z0-9_]) with one or two dots. ValidColumnName restricts column-name parameters to [a-zA-Z_][a-zA-Z0-9_]*. - Validate historyData (bigquery-forecast) and inputData (bigquery-analyze-contribution) against ValidTableID before interpolation. - Validate dataCol, timestampCol, and each element of idCols in bigquery-forecast against ValidColumnName. - Validate isTestCol and each element of dimension_id_cols in bigquery-analyze-contribution against ValidColumnName. - Tighten contribution_metric to reject values containing single quotes, which is the character required to break out of the OPTIONS string literal. - Add validators_test.go covering allowed identifiers and injection payloads.

Yuan325 · 2026-05-28T22:58:36Z

/gcbrun

Yuan325 · 2026-05-29T16:53:16Z

/gcbrun

Yuan325 · 2026-05-29T16:55:54Z

/gcbrun

Yuan325 · 2026-05-29T16:56:17Z

Hi @evilgensec Thank you for fixing this! :)

evilgensec · 2026-05-29T16:58:45Z

Hi @evilgensec Thank you for fixing this! :)

Happy to contribute to the security of such Open Source Projects. Have a good day ahead. ^^

Yuan325 · 2026-05-29T17:42:55Z

/gcbrun

🤖 I have created a release *beep* *boop* --- ## [1.4.0](v1.3.0...v1.4.0) (2026-06-04) ### Features * **ci:** Add support for windows/arm64 binary distribution ([#3231](#3231)) ([10abf3b](10abf3b)) * **datalineage:** Add Data Lineage integration ([#3285](#3285)) ([19353c3](19353c3)) * **server:** Ignore unknown tools at startup with `--ignore-unknown-tools` flag ([#3353](#3353)) ([5f0304f](5f0304f)) * **tools/cloudsqlpg:** Add remaining vector assist tools for Cloud SQL Postgres ([#3203](#3203)) ([b514cbd](b514cbd)) * **tools/spanner-search-catalog:** Implement search_catalog tool ([#3140](#3140)) ([defc086](defc086)) ### Bug Fixes * **auth/generic:** Enforce issuer presence in opaque token validation ([#3360](#3360)) ([1d8df0d](1d8df0d)) * **auth:** Separate Google and Generic MCP OAuth verification ([#3341](#3341)) ([dfd66ee](dfd66ee)) * **mcp:** Support annotations and metadata within Tools to earlier MCP schemas ([#3300](#3300)) ([9a88c72](9a88c72)) * **oracle:** Remove trailing semicolons from prebuilt tools ([#3215](#3215)) ([fcad02d](fcad02d)) * **server/auth:** Centralize tool scopes validation ([#3335](#3335)) ([adce4ab](adce4ab)) * **server:** Return null id for batch request rejection ([#3333](#3333)) ([0b18d58](0b18d58)) * **source/dataplex:** Limit search results to pageSize ([#3323](#3323)) ([905c1f6](905c1f6)), closes [#3308](#3308) * **telemetry:** Allow GCP project override ([#2960](#2960)) ([3c83ba5](3c83ba5)) * **tool/bigquery:** Prevent `allowedDatasets` bypass in forecast query ([#3324](#3324)) ([45df461](45df461)) * **tool/clickhouse:** Handle ignored ProcessParameters error ([#3340](#3340)) ([ddfd887](ddfd887)) * **tools/clickhouse,tools/bigquery:** Validate identifier parameters to prevent injection ([#3219](#3219)) ([2f45f75](2f45f75)) * **tools/looker:** Escape filter values for unquoted parameters ([#3289](#3289)) ([1711156](1711156)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com>

🤖 I have created a release *beep* *boop* --- ## [1.4.0](v1.3.0...v1.4.0) (2026-06-04) ### Features * **ci:** Add support for windows/arm64 binary distribution ([#3231](#3231)) ([10abf3b](10abf3b)) * **datalineage:** Add Data Lineage integration ([#3285](#3285)) ([19353c3](19353c3)) * **server:** Ignore unknown tools at startup with `--ignore-unknown-tools` flag ([#3353](#3353)) ([5f0304f](5f0304f)) * **tools/cloudsqlpg:** Add remaining vector assist tools for Cloud SQL Postgres ([#3203](#3203)) ([b514cbd](b514cbd)) * **tools/spanner-search-catalog:** Implement search_catalog tool ([#3140](#3140)) ([defc086](defc086)) ### Bug Fixes * **auth/generic:** Enforce issuer presence in opaque token validation ([#3360](#3360)) ([1d8df0d](1d8df0d)) * **auth:** Separate Google and Generic MCP OAuth verification ([#3341](#3341)) ([dfd66ee](dfd66ee)) * **mcp:** Support annotations and metadata within Tools to earlier MCP schemas ([#3300](#3300)) ([9a88c72](9a88c72)) * **oracle:** Remove trailing semicolons from prebuilt tools ([#3215](#3215)) ([fcad02d](fcad02d)) * **server/auth:** Centralize tool scopes validation ([#3335](#3335)) ([adce4ab](adce4ab)) * **server:** Return null id for batch request rejection ([#3333](#3333)) ([0b18d58](0b18d58)) * **source/dataplex:** Limit search results to pageSize ([#3323](#3323)) ([905c1f6](905c1f6)), closes [#3308](#3308) * **telemetry:** Allow GCP project override ([#2960](#2960)) ([3c83ba5](3c83ba5)) * **tool/bigquery:** Prevent `allowedDatasets` bypass in forecast query ([#3324](#3324)) ([45df461](45df461)) * **tool/clickhouse:** Handle ignored ProcessParameters error ([#3340](#3340)) ([ddfd887](ddfd887)) * **tools/clickhouse,tools/bigquery:** Validate identifier parameters to prevent injection ([#3219](#3219)) ([2f45f75](2f45f75)) * **tools/looker:** Escape filter values for unquoted parameters ([#3289](#3289)) ([1711156](1711156)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com> d67cfbe

🤖 I have created a release *beep* *boop* --- ## [1.4.0](googleapis/mcp-toolbox@v1.3.0...v1.4.0) (2026-06-04) ### Features * **ci:** Add support for windows/arm64 binary distribution ([googleapis#3231](googleapis#3231)) ([10abf3b](googleapis@10abf3b)) * **datalineage:** Add Data Lineage integration ([googleapis#3285](googleapis#3285)) ([19353c3](googleapis@19353c3)) * **server:** Ignore unknown tools at startup with `--ignore-unknown-tools` flag ([googleapis#3353](googleapis#3353)) ([5f0304f](googleapis@5f0304f)) * **tools/cloudsqlpg:** Add remaining vector assist tools for Cloud SQL Postgres ([googleapis#3203](googleapis#3203)) ([b514cbd](googleapis@b514cbd)) * **tools/spanner-search-catalog:** Implement search_catalog tool ([googleapis#3140](googleapis#3140)) ([defc086](googleapis@defc086)) ### Bug Fixes * **auth/generic:** Enforce issuer presence in opaque token validation ([googleapis#3360](googleapis#3360)) ([1d8df0d](googleapis@1d8df0d)) * **auth:** Separate Google and Generic MCP OAuth verification ([googleapis#3341](googleapis#3341)) ([dfd66ee](googleapis@dfd66ee)) * **mcp:** Support annotations and metadata within Tools to earlier MCP schemas ([googleapis#3300](googleapis#3300)) ([9a88c72](googleapis@9a88c72)) * **oracle:** Remove trailing semicolons from prebuilt tools ([googleapis#3215](googleapis#3215)) ([fcad02d](googleapis@fcad02d)) * **server/auth:** Centralize tool scopes validation ([googleapis#3335](googleapis#3335)) ([adce4ab](googleapis@adce4ab)) * **server:** Return null id for batch request rejection ([googleapis#3333](googleapis#3333)) ([0b18d58](googleapis@0b18d58)) * **source/dataplex:** Limit search results to pageSize ([googleapis#3323](googleapis#3323)) ([905c1f6](googleapis@905c1f6)), closes [googleapis#3308](googleapis#3308) * **telemetry:** Allow GCP project override ([googleapis#2960](googleapis#2960)) ([3c83ba5](googleapis@3c83ba5)) * **tool/bigquery:** Prevent `allowedDatasets` bypass in forecast query ([googleapis#3324](googleapis#3324)) ([45df461](googleapis@45df461)) * **tool/clickhouse:** Handle ignored ProcessParameters error ([googleapis#3340](googleapis#3340)) ([ddfd887](googleapis@ddfd887)) * **tools/clickhouse,tools/bigquery:** Validate identifier parameters to prevent injection ([googleapis#3219](googleapis#3219)) ([2f45f75](googleapis@2f45f75)) * **tools/looker:** Escape filter values for unquoted parameters ([googleapis#3289](googleapis#3289)) ([1711156](googleapis@1711156)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com> d67cfbe

evilgensec requested a review from a team as a code owner May 12, 2026 18:41

blunderbuss-gcf Bot assigned Yuan325 May 12, 2026

gemini-code-assist Bot reviewed May 12, 2026

View reviewed changes

evilgensec requested a review from a team as a code owner May 14, 2026 18:14

evilgensec force-pushed the fix/clickhouse-list-tables-identifier-injection branch from 9b5a323 to d9813fd Compare May 14, 2026 18:17

evilgensec added 2 commits May 21, 2026 00:23

evilgensec force-pushed the fix/clickhouse-list-tables-identifier-injection branch from 75ddc38 to b732d08 Compare May 20, 2026 18:39

evilgensec changed the title ~~clickhouse-list-tables: validate database identifier to prevent injection~~ fix(tools/clickhouse,tools/bigquery): validate identifier parameters to prevent injection May 20, 2026

Yuan325 approved these changes May 28, 2026

View reviewed changes

Merge branch 'main' into fix/clickhouse-list-tables-identifier-injection

f37d0ef

shobsi approved these changes May 29, 2026

View reviewed changes

Genesis929 approved these changes May 29, 2026

View reviewed changes

Merge branch 'main' into fix/clickhouse-list-tables-identifier-injection

6626f96

Yuan325 enabled auto-merge (squash) May 29, 2026 16:55

Yuan325 added 2 commits May 29, 2026 10:41

Update util.go

ac4acbf

allow hyphen in project id

ddb3778

Yuan325 merged commit 2f45f75 into googleapis:main May 29, 2026
19 checks passed

release-please Bot mentioned this pull request May 29, 2026

chore(main): release 1.4.0 #3301

Merged

BrewTestBot mentioned this pull request Jun 4, 2026

mcp-toolbox 1.4.0 Homebrew/homebrew-core#286359

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(tools/clickhouse,tools/bigquery): validate identifier parameters to prevent injection#3219

fix(tools/clickhouse,tools/bigquery): validate identifier parameters to prevent injection#3219
Yuan325 merged 6 commits into
googleapis:mainfrom
evilgensec:fix/clickhouse-list-tables-identifier-injection

evilgensec commented May 12, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Yuan325 commented May 28, 2026

Uh oh!

Yuan325 commented May 29, 2026

Uh oh!

Yuan325 commented May 29, 2026

Uh oh!

Yuan325 commented May 29, 2026

Uh oh!

evilgensec commented May 29, 2026

Uh oh!

Yuan325 commented May 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

evilgensec commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

1. clickhouse-list-tables — database identifier injection (original fix)

2. bigquery-forecast — backtick + column-name injection

3. bigquery-analyze-contribution — backtick + OPTIONS injection

Shared validators in bigquerycommon

Test plan

Not a dupe of #2811 / #779

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Yuan325 commented May 28, 2026

Uh oh!

Yuan325 commented May 29, 2026

Uh oh!

Yuan325 commented May 29, 2026

Uh oh!

Yuan325 commented May 29, 2026

Uh oh!

evilgensec commented May 29, 2026

Uh oh!

Yuan325 commented May 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

evilgensec commented May 12, 2026 •

edited

Loading

1. `clickhouse-list-tables` — database identifier injection (original fix)

2. `bigquery-forecast` — backtick + column-name injection

3. `bigquery-analyze-contribution` — backtick + OPTIONS injection

Shared validators in `bigquerycommon`