Hi,
On Mon, Jul 28, 2025 at 10:43 AM Stepan Neretin <slp...@gmail.com> wrote:
>
>
> Your patch for securing cross-session temp table access is a great
> improvement. The RVR_OTHER_TEMP_OK flag elegantly handles the DROP case while
> keeping the main restriction in place.
>
> For schema name validation, an exact strcmp for "pg_temp" and proper numeric
> parsing for "pg_temp_X" would be more precise than the current prefix check.
> This would avoid any accidental matches to similarly named schemas.
>
Thanks for looking into it!
> The error message could be adjusted to emphasize permissions, like
> "permission denied for cross-session temp table access". This would make the
> security intent clearer to users.
>
I don't think that such an error message will be more appropriate. We
want to forbid this operation not because of "permission" reasons, but
because of the danger of this operation.
Yes, some people insist that dropping other sessions' temp tables might
be useful in some cases, but it is a "last resort" solution.
Even with this patch, DROP of other session temp tables can lead to
an error. I wrote about it here [1].
> I noticed the Assert assumes myTempNamespace is always valid. While correct,
> a brief comment explaining why this is safe would help future maintainers.
Well, v5 patch already contains comment for this assert :
/*
* If this table was recognized as temporary, it means that we
* found it because the backend's temporary namespace was specified
* in search_path. Thus, MyTempNamespace must contain valid oid.
*/
> The relpersistence logic could also be centralized in one place for
> consistency.
I don't see a reason to separate this logic into a new function, because
there will be no more cases when it will be useful to us.
> I've added an isolation test to verify the behavior when trying to access
> another backend's temp tables. It confirms the restrictions work as intended
> while allowing permitted operations.
Some time ago I also created a test for this situation, see patch in this [2]
message. it worked in a similar way (but covered more test cases).
It caused a mixed reaction from people, so I decided to abandon this idea.
I guess it might be a discussion point in the future, but first I'd like to
settle the core logic of the patch.
I attach a v7 patch to this letter. No changes yet, just rebased on the newest
commit in master branch.
[1]
https://www.postgresql.org/message-id/flat/CAJDiXghoi-FM4d5XVZzUyiuhv8DDm9JdGOU8KC47emasqi1GUw%40mail.gmail.com
[2]
https://www.postgresql.org/message-id/CAJDiXgi9CWaZCVcHmvAT604RrAqDN5zpOYxZq92adqkPq5QbnQ%40mail.gmail.com
--
Best regards,
Daniil Davydov
From f849f7f75f3d79b586d49c74f172b1155560335f Mon Sep 17 00:00:00 2001
From: Daniil Davidov <d.davy...@postgrespro.ru>
Date: Tue, 29 Jul 2025 16:32:35 +0700
Subject: [PATCH v7] Fix accessing other sessions temp tables
---
src/backend/catalog/namespace.c | 56 ++++++++++++++++++++------------
src/backend/commands/tablecmds.c | 3 +-
src/backend/nodes/makefuncs.c | 6 +++-
src/backend/parser/gram.y | 11 ++++++-
src/include/catalog/namespace.h | 2 ++
5 files changed, 55 insertions(+), 23 deletions(-)
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index d97d632a7ef..f407efd9447 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -499,28 +499,44 @@ RangeVarGetRelidExtended(const RangeVar *relation, LOCKMODE lockmode,
*/
if (relation->relpersistence == RELPERSISTENCE_TEMP)
{
- if (!OidIsValid(myTempNamespace))
- relId = InvalidOid; /* this probably can't happen? */
- else
- {
- if (relation->schemaname)
- {
- Oid namespaceId;
+ Oid namespaceId;
- namespaceId = LookupExplicitNamespace(relation->schemaname, missing_ok);
+ if (relation->schemaname)
+ {
+ namespaceId = LookupExplicitNamespace(relation->schemaname, missing_ok);
+ /*
+ * If the user has specified an existing temporary schema
+ * owned by another user.
+ */
+ if (OidIsValid(namespaceId) && namespaceId != myTempNamespace)
+ {
/*
- * For missing_ok, allow a non-existent schema name to
- * return InvalidOid.
+ * We don't allow users to access temp tables of other
+ * sessions except for the case of dropping tables.
*/
- if (namespaceId != myTempNamespace)
+ if (!(flags & RVR_OTHER_TEMP_OK))
ereport(ERROR,
- (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
- errmsg("temporary tables cannot specify a schema name")));
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("could not access temporary relations of other sessions")));
}
+ }
+ else
+ {
+ namespaceId = myTempNamespace;
- relId = get_relname_relid(relation->relname, myTempNamespace);
+ /*
+ * If this table was recognized as temporary, it means that we
+ * found it because backend's temporary namespace was specified
+ * in search_path. Thus, MyTempNamespace must contain valid oid.
+ */
+ Assert(OidIsValid(namespaceId));
}
+
+ if (missing_ok && !OidIsValid(namespaceId))
+ relId = InvalidOid;
+ else
+ relId = get_relname_relid(relation->relname, namespaceId);
}
else if (relation->schemaname)
{
@@ -3553,21 +3569,19 @@ get_namespace_oid(const char *nspname, bool missing_ok)
RangeVar *
makeRangeVarFromNameList(const List *names)
{
- RangeVar *rel = makeRangeVar(NULL, NULL, -1);
+ RangeVar *rel;
switch (list_length(names))
{
case 1:
- rel->relname = strVal(linitial(names));
+ rel = makeRangeVar(NULL, strVal(linitial(names)), -1);
break;
case 2:
- rel->schemaname = strVal(linitial(names));
- rel->relname = strVal(lsecond(names));
+ rel = makeRangeVar(strVal(linitial(names)), strVal(lsecond(names)), -1);
break;
case 3:
+ rel = makeRangeVar(strVal(lsecond(names)), strVal(lthird(names)), -1);
rel->catalogname = strVal(linitial(names));
- rel->schemaname = strVal(lsecond(names));
- rel->relname = strVal(lthird(names));
break;
default:
ereport(ERROR,
@@ -3774,6 +3788,8 @@ GetTempNamespaceProcNumber(Oid namespaceId)
return INVALID_PROC_NUMBER; /* no such namespace? */
if (strncmp(nspname, "pg_temp_", 8) == 0)
result = atoi(nspname + 8);
+ else if (strcmp(nspname, "pg_temp") == 0)
+ result = MyProcNumber;
else if (strncmp(nspname, "pg_toast_temp_", 14) == 0)
result = atoi(nspname + 14);
else
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index cb811520c29..76406b5a9d9 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1623,7 +1623,8 @@ RemoveRelations(DropStmt *drop)
state.heapOid = InvalidOid;
state.partParentOid = InvalidOid;
- relOid = RangeVarGetRelidExtended(rel, lockmode, RVR_MISSING_OK,
+ relOid = RangeVarGetRelidExtended(rel, lockmode,
+ RVR_MISSING_OK | RVR_OTHER_TEMP_OK,
RangeVarCallbackForDropRelation,
&state);
diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index e2d9e9be41a..62edf24b5c2 100644
--- a/src/backend/nodes/makefuncs.c
+++ b/src/backend/nodes/makefuncs.c
@@ -478,10 +478,14 @@ makeRangeVar(char *schemaname, char *relname, int location)
r->schemaname = schemaname;
r->relname = relname;
r->inh = true;
- r->relpersistence = RELPERSISTENCE_PERMANENT;
r->alias = NULL;
r->location = location;
+ if (r->schemaname && strncmp(r->schemaname, "pg_temp", 7) == 0)
+ r->relpersistence = RELPERSISTENCE_TEMP;
+ else
+ r->relpersistence = RELPERSISTENCE_PERMANENT;
+
return r;
}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index db43034b9db..1b959e752c3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -19405,7 +19405,11 @@ makeRangeVarFromAnyName(List *names, int position, core_yyscan_t yyscanner)
break;
}
- r->relpersistence = RELPERSISTENCE_PERMANENT;
+ if (r->schemaname && strncmp(r->schemaname, "pg_temp", 7) == 0)
+ r->relpersistence = RELPERSISTENCE_TEMP;
+ else
+ r->relpersistence = RELPERSISTENCE_PERMANENT;
+
r->location = position;
return r;
@@ -19445,6 +19449,11 @@ makeRangeVarFromQualifiedName(char *name, List *namelist, int location,
break;
}
+ if (r->schemaname && strncmp(r->schemaname, "pg_temp", 7) == 0)
+ r->relpersistence = RELPERSISTENCE_TEMP;
+ else
+ r->relpersistence = RELPERSISTENCE_PERMANENT;
+
return r;
}
diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespace.h
index 8c7ccc69a3c..9c45a30516e 100644
--- a/src/include/catalog/namespace.h
+++ b/src/include/catalog/namespace.h
@@ -72,6 +72,8 @@ typedef enum RVROption
RVR_MISSING_OK = 1 << 0, /* don't error if relation doesn't exist */
RVR_NOWAIT = 1 << 1, /* error if relation cannot be locked */
RVR_SKIP_LOCKED = 1 << 2, /* skip if relation cannot be locked */
+ RVR_OTHER_TEMP_OK = 1 << 3 /* don't error if relation is temp relation of
+ other session (needed for DROP command) */
} RVROption;
typedef void (*RangeVarGetRelidCallback) (const RangeVar *relation, Oid relId,
--
2.43.0
