From 789bb26dfbf1aaf85163e5ddfc70fa6dae0894fb Mon Sep 17 00:00:00 2001 From: Sven Klemm Date: Tue, 31 Jan 2023 13:59:48 +0100 Subject: [PATCH] Lock down search_path in SPI calls --- CHANGELOG.md | 1 + src/hypertable.c | 10 ++++++++-- src/telemetry/replication.c | 5 +++++ src/telemetry/telemetry.c | 10 ++++------ tsl/src/continuous_aggs/materialize.c | 5 +++++ tsl/src/continuous_aggs/refresh.c | 5 +++++ tsl/src/reorder.c | 15 +++++++++++++++ 7 files changed, 43 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1bfe75b682..b589fd5d768 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ accidentally triggering the load of a previous DB version.** * #4926 Fix corruption when inserting into compressed chunks * #5218 Add role-level security to job error log * #5214 Fix use of prepared statement in async module +* #5259 Lock down search_path in SPI calls ## 2.9.2 (2023-01-26) diff --git a/src/hypertable.c b/src/hypertable.c index 3dd3f58f3f2..422424dc5a2 100644 --- a/src/hypertable.c +++ b/src/hypertable.c @@ -2901,10 +2901,16 @@ ts_hypertable_get_open_dim_max_value(const Hypertable *ht, int dimension_index, if (NULL == dim) elog(ERROR, "invalid open dimension index %d", dimension_index); - /* Query for the last bucket in the materialized hypertable */ + /* + * Query for the last bucket in the materialized hypertable. + * Since this might be run as part of a parallel operation + * we cannot use SET search_path here to lock down the + * search_path and instead have to fully schema-qualify + * everything. + */ command = makeStringInfo(); appendStringInfo(command, - "SELECT max(%s) FROM %s.%s", + "SELECT pg_catalog.max(%s) FROM %s.%s", quote_identifier(NameStr(dim->fd.column_name)), quote_identifier(NameStr(ht->fd.schema_name)), quote_identifier(NameStr(ht->fd.table_name))); diff --git a/src/telemetry/replication.c b/src/telemetry/replication.c index 311907fb229..ca23a5804f1 100644 --- a/src/telemetry/replication.c +++ b/src/telemetry/replication.c @@ -23,6 +23,11 @@ ts_telemetry_replication_info_gather(void) if (SPI_connect() != SPI_OK_CONNECT) return info; + /* Lock down search_path */ + res = SPI_exec("SET LOCAL search_path TO pg_catalog, pg_temp", 0); + if (res < 0) + ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR), (errmsg("could not set search_path")))); + res = SPI_execute("SELECT cast(count(pid) as int) from pg_catalog.pg_stat_get_wal_senders() " "WHERE pid is not null", true, /* read_only */ diff --git a/src/telemetry/telemetry.c b/src/telemetry/telemetry.c index 1a98f0a20b9..e09677e9aba 100644 --- a/src/telemetry/telemetry.c +++ b/src/telemetry/telemetry.c @@ -359,8 +359,8 @@ add_errors_by_sqlerrcode(JsonbParseState *parse_state) if (SPI_connect() != SPI_OK_CONNECT) elog(ERROR, "could not connect to SPI"); - /* SPI calls must be qualified otherwise they are unsafe */ - res = SPI_exec("SET search_path TO pg_catalog, pg_temp", 0); + /* Lock down search_path */ + res = SPI_exec("SET LOCAL search_path TO pg_catalog, pg_temp", 0); if (res < 0) ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR), (errmsg("could not set search_path")))); @@ -398,7 +398,6 @@ add_errors_by_sqlerrcode(JsonbParseState *parse_state) old_context = MemoryContextSwitchTo(spi_context); } - res = SPI_exec("RESET search_path", 0); res = SPI_finish(); Assert(res == SPI_OK_FINISH); @@ -462,8 +461,8 @@ add_job_stats_by_job_type(JsonbParseState *parse_state) if (SPI_connect() != SPI_OK_CONNECT) elog(ERROR, "could not connect to SPI"); - /* SPI calls must be qualified otherwise they are unsafe */ - res = SPI_exec("SET search_path TO pg_catalog, pg_temp", 0); + /* Lock down search_path */ + res = SPI_exec("SET LOCAL search_path TO pg_catalog, pg_temp", 0); if (res < 0) ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR), (errmsg("could not set search_path")))); @@ -525,7 +524,6 @@ add_job_stats_by_job_type(JsonbParseState *parse_state) add_job_stats_internal(parse_state, TextDatumGetCString(jobtype_datum), &stats); old_context = MemoryContextSwitchTo(spi_context); } - res = SPI_exec("RESET search_path", 0); res = SPI_finish(); Assert(res == SPI_OK_FINISH); } diff --git a/tsl/src/continuous_aggs/materialize.c b/tsl/src/continuous_aggs/materialize.c index 4dc63b08e19..551c5c4c94c 100644 --- a/tsl/src/continuous_aggs/materialize.c +++ b/tsl/src/continuous_aggs/materialize.c @@ -59,6 +59,11 @@ continuous_agg_update_materialization(SchemaAndName partial_view, if (res != SPI_OK_CONNECT) elog(ERROR, "could not connect to SPI in materializer"); + /* Lock down search_path */ + res = SPI_exec("SET LOCAL search_path TO pg_catalog, pg_temp", 0); + if (res < 0) + ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR), (errmsg("could not set search_path")))); + /* pin the start of new_materialization to the end of new_materialization, * we are not allowed to materialize beyond that point */ diff --git a/tsl/src/continuous_aggs/refresh.c b/tsl/src/continuous_aggs/refresh.c index 5ba20ebdb3b..7cec56f377a 100644 --- a/tsl/src/continuous_aggs/refresh.c +++ b/tsl/src/continuous_aggs/refresh.c @@ -717,6 +717,11 @@ continuous_agg_refresh_internal(const ContinuousAgg *cagg, if ((rc = SPI_connect_ext(SPI_OPT_NONATOMIC) != SPI_OK_CONNECT)) elog(ERROR, "SPI_connect failed: %s", SPI_result_code_string(rc)); + /* Lock down search_path */ + rc = SPI_exec("SET LOCAL search_path TO pg_catalog, pg_temp", 0); + if (rc < 0) + ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR), (errmsg("could not set search_path")))); + /* Like regular materialized views, require owner to refresh. */ if (!pg_class_ownercheck(cagg->relid, GetUserId())) aclcheck_error(ACLCHECK_NOT_OWNER, diff --git a/tsl/src/reorder.c b/tsl/src/reorder.c index 19ea75e37a3..a2973101e69 100644 --- a/tsl/src/reorder.c +++ b/tsl/src/reorder.c @@ -244,6 +244,11 @@ tsl_copy_or_move_chunk_proc(FunctionCallInfo fcinfo, bool delete_on_src_node) if ((rc = SPI_connect_ext(nonatomic ? SPI_OPT_NONATOMIC : 0)) != SPI_OK_CONNECT) elog(ERROR, "SPI_connect failed: %s", SPI_result_code_string(rc)); + /* Lock down search_path */ + rc = SPI_exec("SET LOCAL search_path TO pg_catalog, pg_temp", 0); + if (rc < 0) + ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR), (errmsg("could not set search_path")))); + /* perform the actual distributed chunk move after a few sanity checks */ chunk_copy(chunk_id, src_node_name, dst_node_name, op_id, delete_on_src_node); @@ -328,6 +333,11 @@ tsl_subscription_exec(PG_FUNCTION_ARGS) if (SPI_connect() != SPI_OK_CONNECT) elog(ERROR, "could not connect to SPI"); + /* Lock down search_path */ + res = SPI_exec("SET LOCAL search_path TO pg_catalog, pg_temp", 0); + if (res < 0) + ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR), (errmsg("could not set search_path")))); + res = SPI_execute(subscription_cmd, false /* read_only */, 0 /*count*/); if (res < 0) @@ -365,6 +375,11 @@ tsl_copy_chunk_cleanup_proc(PG_FUNCTION_ARGS) if ((rc = SPI_connect_ext(nonatomic ? SPI_OPT_NONATOMIC : 0)) != SPI_OK_CONNECT) elog(ERROR, "SPI_connect failed: %s", SPI_result_code_string(rc)); + /* Lock down search_path */ + rc = SPI_exec("SET LOCAL search_path TO pg_catalog, pg_temp", 0); + if (rc < 0) + ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR), (errmsg("could not set search_path")))); + /* perform the cleanup/repair depending on the stage */ chunk_copy_cleanup(operation_id);