From 50d6e533e4d9a0f70d798c534254007c83c0d428 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Fri, 2 Oct 2026 10:43:17 +0900 Subject: [PATCH] Fix crash when describing a FETCH statement whose cursor is gone FetchStatementTargetList() dereferenced the portal named by a FetchStmt without checking that it still exists. With the extended query protocol, a client can sent a Parse "FETCH n FROM c" while cursor "c" is open (which caches a result's TupleDesc), issue a CLOSE on cursor "c", and then Describe the prepared statement or a portal bound to it. Describe would re-resolve the target list through the named cursor, and crash. In a non-assert builds, this causes a NULL pointer dereference. An assertion was triggered in assert builds slightly before the pointer dereference. This commit changes the describe of a FetchStmt to match with the ExecuteStmt case: if the named cursor's portal is invalid, fail rather than assume that the portal should always exist. A test case is added to libpq_pipeline, backpatchable all the way down. Author: Dirkjan Bussink Reviewed-by: Tom Lane Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/7DE248EB-7509-4F8A-9F37-4694A40BBAB7@gmail.com Backpatch-through: 14 --- src/backend/tcop/pquery.c | 6 ++- .../modules/libpq_pipeline/libpq_pipeline.c | 49 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c index 9f3ea1c9a75..fa2ef20f178 100644 --- a/src/backend/tcop/pquery.c +++ b/src/backend/tcop/pquery.c @@ -389,7 +389,11 @@ FetchStatementTargetList(Node *stmt) Assert(!fstmt->ismove); subportal = GetPortalByName(fstmt->portalname); - Assert(PortalIsValid(subportal)); + if (!PortalIsValid(subportal)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_CURSOR), + errmsg("cursor \"%s\" does not exist", + fstmt->portalname))); return FetchPortalTargetList(subportal); } if (IsA(stmt, ExecuteStmt)) diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/test/modules/libpq_pipeline/libpq_pipeline.c index ad007038dc9..66125d16eac 100644 --- a/src/test/modules/libpq_pipeline/libpq_pipeline.c +++ b/src/test/modules/libpq_pipeline/libpq_pipeline.c @@ -456,6 +456,52 @@ test_cancel(PGconn *conn) fprintf(stderr, "ok\n"); } +/* + * Test Describe of a prepared FETCH statement after the cursor it + * references has been closed. + */ +static void +test_describe_fetch(PGconn *conn) +{ + PGresult *res; + + fprintf(stderr, "test cursor describe and fetch... "); + + res = PQexec(conn, "BEGIN"); + if (PQresultStatus(res) != PGRES_COMMAND_OK) + pg_fatal("BEGIN failed: %s", PQerrorMessage(conn)); + PQclear(res); + + res = PQexec(conn, "DECLARE fetch_cursor CURSOR FOR SELECT 1"); + if (PQresultStatus(res) != PGRES_COMMAND_OK) + pg_fatal("DECLARE CURSOR failed: %s", PQerrorMessage(conn)); + PQclear(res); + + /* prepare while the cursor exists, so that it caches a result desc */ + res = PQprepare(conn, "fetch_one", "FETCH 1 FROM fetch_cursor", 0, NULL); + if (PQresultStatus(res) != PGRES_COMMAND_OK) + pg_fatal("PQprepare failed: %s", PQerrorMessage(conn)); + PQclear(res); + + res = PQexec(conn, "CLOSE fetch_cursor"); + if (PQresultStatus(res) != PGRES_COMMAND_OK) + pg_fatal("CLOSE failed: %s", PQerrorMessage(conn)); + PQclear(res); + + /* describe fails after the cursor has been closed */ + res = PQdescribePrepared(conn, "fetch_one"); + if (PQresultStatus(res) != PGRES_FATAL_ERROR) + pg_fatal("expected FATAL_ERROR, got %s", PQresStatus(PQresultStatus(res))); + PQclear(res); + + res = PQexec(conn, "ROLLBACK"); + if (PQresultStatus(res) != PGRES_COMMAND_OK) + pg_fatal("ROLLBACK failed: %s", PQerrorMessage(conn)); + PQclear(res); + + fprintf(stderr, "ok\n"); +} + static void test_disallowed_in_pipeline(PGconn *conn) { @@ -2117,6 +2163,7 @@ static void print_test_list(void) { printf("cancel\n"); + printf("describe_fetch\n"); printf("disallowed_in_pipeline\n"); printf("multi_pipelines\n"); printf("nosync\n"); @@ -2223,6 +2270,8 @@ main(int argc, char **argv) if (strcmp(testname, "cancel") == 0) test_cancel(conn); + else if (strcmp(testname, "describe_fetch") == 0) + test_describe_fetch(conn); else if (strcmp(testname, "disallowed_in_pipeline") == 0) test_disallowed_in_pipeline(conn); else if (strcmp(testname, "multi_pipelines") == 0)