On Fri, Aug 9, 2024 at 10:18 AM Dmitry Koval <d.ko...@postgrespro.ru> wrote:
> > This breaks from the CVE-2014-0062 (commit 5f17304) principle of not
> > repeating
> > name lookups. The attached demo uses this defect to make one partition have
> > two parents.
>
> Thank you very much for information (especially for the demo)!
>
> I'm not sure that we can get the identifier of the newly created
> partition from the ProcessUtility() function...
> Maybe it would be enough to check that the new partition is located in
> the namespace in which we created it (see attachment)?
The new partition doesn't necessarily get created in the same
namespace as parent partition. I think it would be better to somehow
open partition by its oid.
It would be quite unfortunate to replicate significant part of
ProcessUtilitySlow(). So, the question is how to get the oid of newly
created relation from ProcessUtility(). I don't like to change the
signature of ProcessUtility() especially as a part of backpatch. So,
I tried to fit this into existing parameters. Probably
QueryCompletion struct fits this purpose best from the existing
parameters. Attached draft patch implements returning oid of newly
created relation as part of QueryCompletion. Thoughts?
------
Regards,
Alexander Korotkov
Supabase
From 83b6d8e38d680daa952542f6ae4a41ae48491a62 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Sat, 10 Aug 2024 16:19:28 +0300
Subject: [PATCH v1] Fix createPartitionTable() security issue
---
src/backend/commands/tablecmds.c | 6 ++++--
src/backend/tcop/utility.c | 2 ++
src/include/tcop/cmdtag.h | 3 +++
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1f94f4fdbbc..c64b4953da8 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20329,6 +20329,7 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
TableLikeClause *tlc;
PlannedStmt *wrapper;
Relation newRel;
+ QueryCompletion qc;
/* If existing rel is temp, it must belong to this session */
if (modelRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP &&
@@ -20373,6 +20374,7 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
wrapper->stmt_location = context->pstmt->stmt_location;
wrapper->stmt_len = context->pstmt->stmt_len;
+ qc.commandTag = CMDTAG_UNKNOWN;
ProcessUtility(wrapper,
context->queryString,
false,
@@ -20380,13 +20382,13 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
NULL,
NULL,
None_Receiver,
- NULL);
+ &qc);
/*
* Open the new partition with no lock, because we already have
* AccessExclusiveLock placed there after creation.
*/
- newRel = table_openrv(newPartName, NoLock);
+ newRel = table_open(qc.tableAddress.objectId, NoLock);
/*
* We intended to create the partition with the same persistence as the
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index b2ea8125c92..e8a83e0c569 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1165,6 +1165,8 @@ ProcessUtilitySlow(ParseState *pstate,
RELKIND_RELATION,
InvalidOid, NULL,
queryString);
+ if (qc)
+ qc->tableAddress = address;
EventTriggerCollectSimpleCommand(address,
secondaryObject,
stmt);
diff --git a/src/include/tcop/cmdtag.h b/src/include/tcop/cmdtag.h
index 23c99d7eca6..a964a8c0cb7 100644
--- a/src/include/tcop/cmdtag.h
+++ b/src/include/tcop/cmdtag.h
@@ -13,6 +13,8 @@
#ifndef CMDTAG_H
#define CMDTAG_H
+#include "catalog/objectaddress.h"
+
/* buffer size required for command completion tags */
#define COMPLETION_TAG_BUFSIZE 64
@@ -29,6 +31,7 @@ typedef enum CommandTag
typedef struct QueryCompletion
{
CommandTag commandTag;
+ ObjectAddress tableAddress;
uint64 nprocessed;
} QueryCompletion;
--
2.39.3 (Apple Git-146)
