Обсуждение: Fix bug with accessing to temporary tables of other sessions

Hi,

During previous commitfest this topic already has been discussed
within the "Forbid to DROP temp tables of other sessions" thread [1].
Unfortunately its name doesn't reflect the real problem, so I decided
to start a new thread, as David G. Johnston advised.

Here are the summary results of the discussion [1] :
The superuser is only allowed to DROP temporary relations of other
sessions. Other commands (like SELECT, INSERT, UPDATE, DELETE ...)
must be forbidden to him. Error message for this case will look like
this : `could not access temporary relations of other sessions`.
For now, superuser still can specify such operations because of a bug
in the code that mistakenly recognizes other session's temp table as
permanent (I've covered this topic in more detail in [2]). Attached
patch fixes this bug (targeted on
b51f86e49a7f119004c0ce5d0be89cdf98309141).

Opened issue:
Not everyone liked the idea that table's persistence can be assigned
to table during makeRangeVarXXX calls (inside gram.y).
My opinion - `As far as "pg_temp_" prefix is reserved by the postgres
kernel, we can definitely say that we have encountered a temporary
table when we see this prefix.`

I will be glad to hear your opinion.

--
Best regards,
Daniil Davydov

[1] https://www.postgresql.org/message-id/CAJDiXgj72Axj0d4ojKdRWG_rnkfs4uWY414NL%3D15sCvh7-9rwg%40mail.gmail.com
[2] https://www.postgresql.org/message-id/CAJDiXgj%2B5UKLWSUT5605rJhuw438NmEKecvhFAF2nnrMsgGK3w%40mail.gmail.com

Hi,

On Mon, Jul 28, 2025 at 10:43 AM Stepan Neretin <slpmcf@gmail.com> wrote:
>
>
> Your patch for securing cross-session temp table access is a great improvement. The RVR_OTHER_TEMP_OK flag elegantly
handlesthe 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
precisethan 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
safewould 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
therestrictions 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

Hi Daniil,

On 7/29/25 11:35, Daniil Davydov wrote:
> I attach a v7 patch to this letter. No changes yet, just rebased on the newest
> commit in master branch.

A few days ago I reviewed one patch[1] that has a significant overlap
with this one. Perhaps they should be merged?

Here my first tests and comments:

== session 1 ==

$ /usr/local/postgres-dev/bin/psql postgres
psql (19devel)
Type "help" for help.

postgres=# CREATE TEMPORARY TABLE tmp AS SELECT 42 AS val;
SELECT 1
postgres=# \d tmp
              Table "pg_temp_75.tmp"
 Column |  Type   | Collation | Nullable | Default
--------+---------+-----------+----------+---------
 val    | integer |           |          |


== session 2 ==

$ /usr/local/postgres-dev/bin/psql postgres
psql (19devel)
Type "help" for help.

-- fixed: previously accessed the table but returning 0 rows
postgres=# SELECT * FROM pg_temp_75.tmp;
ERROR:  could not access temporary relations of other sessions
LINE 1: SELECT * FROM pg_temp_75.tmp;
                      ^
-- fixed: previously returning DELETE 0
postgres=# DELETE FROM pg_temp_75.tmp;
ERROR:  could not access temporary relations of other sessions
LINE 1: DELETE FROM pg_temp_75.tmp;
                    ^
postgres=# TRUNCATE TABLE pg_temp_75.tmp;
ERROR:  could not access temporary relations of other sessions

-- fixed: previously returning UPDATE 0
postgres=# UPDATE pg_temp_75.tmp SET val = NULL;
ERROR:  could not access temporary relations of other sessions
LINE 1: UPDATE pg_temp_75.tmp SET val = NULL;
               ^
-- error message changed: previously "ERROR:  cannot access temporary
tables of other sessions"
postgres=# INSERT INTO pg_temp_75.tmp VALUES (73);
ERROR:  could not access temporary relations of other sessions
LINE 1: INSERT INTO pg_temp_75.tmp VALUES (73);
                    ^
-- fixed: previously returning COPY 0
postgres=# COPY pg_temp_75.tmp TO '/tmp/foo';
ERROR:  could not access temporary relations of other sessions

-- error message changed. previously "ERROR:  cannot alter temporary
tables of other sessions"
postgres=# ALTER TABLE pg_temp_75.tmp ADD COLUMN foo int;
ERROR:  could not access temporary relations of other sessions

-- fixed: previously[2] it was possible to rename the temp table.
postgres=# ALTER TABLE pg_temp_75.tmp RENAME TO bar;
ERROR:  could not access temporary relations of other sessions

-- fixed: previously[3] it was possible to LOCK the temp table.
postgres=# BEGIN;
BEGIN
postgres=*# LOCK TABLE pg_temp_75.tmp IN ACCESS EXCLUSIVE MODE;
ERROR:  could not access temporary relations of other sessions

DROP TABLE still works, but I guess it is the main motivation of
RVR_OTHER_TEMP_OK :)

Thanks for the patch. It's a great improvement!

Best regards, Jim


[1]
https://www.postgresql.org/message-id/flat/2736425.1758475979%40sss.pgh.pa.us
[2] ALTER TABLE ... RENAME TO tests in PostgreSQL 14.19:

  == session 1 ==
  psql (14.19 (Debian 14.19-1.pgdg13+1))
  Geben Sie »help« für Hilfe ein.

  postgres=# CREATE TEMPORARY TABLE tmp AS SELECT 42 AS val;
  SELECT 1
  postgres=# \d tmp
                      Tabelle »pg_temp_4.tmp«
   Spalte |   Typ   | Sortierfolge | NULL erlaubt? | Vorgabewert
  --------+---------+--------------+---------------+-------------
   val    | integer |              |               |


  == session 2 ==
  psql (14.19 (Debian 14.19-1.pgdg13+1))
  Geben Sie »help« für Hilfe ein.

  postgres=# ALTER TABLE pg_temp_4.tmp RENAME TO foo;
  ALTER TABLE

  == session 1 ==

  postgres=# \d tmp
  Keine Relation namens »tmp« gefunden
  postgres=# \d foo
                      Tabelle »pg_temp_4.foo«
   Spalte |   Typ   | Sortierfolge | NULL erlaubt? | Vorgabewert
  --------+---------+--------------+---------------+-------------
   val    | integer |              |               |

[3] LOCK TABLE tests in PostgreSQL 14.19
  == session 2 ==
  postgres=# BEGIN;
  BEGIN
  postgres=*# LOCK TABLE pg_temp_4.foo IN ACCESS EXCLUSIVE MODE;
  LOCK TABLE
  postgres=*#

  == session 1 ==
  -- * owner of the temp table
  postgres=# SELECT locktype, relation::regclass, mode, granted, pid
  FROM pg_locks
  WHERE relation = 'pg_temp_4.foo'::regclass::oid;
   locktype | relation |        mode         | granted |  pid
  ----------+----------+---------------------+---------+--------
   relation | foo      | AccessExclusiveLock | t       | 277699
  (1 Zeile)

Hi,

On Thu, Sep 25, 2025 at 5:45 PM Jim Jones <jim.jones@uni-muenster.de> wrote:
>
> A few days ago I reviewed one patch[1] that has a significant overlap
> with this one. Perhaps they should be merged?
>

Thanks for looking into it!
I don't know what exactly is meant by merging. Maybe we should just
apply a current
patch that fixes all problems ?..

> Here my first tests and comments:
> ....

OK, I'll replace "could not" with "cannot" in order to match previous comments.

>
> DROP TABLE still works, but I guess it is the main motivation of
> RVR_OTHER_TEMP_OK :)

Yep, motivation of this decision you can find here [1].


I'll attach a v8 patch that fixes error messages (could not -> cannot).

--
Best regards,
Daniil Davydov

[1] https://www.postgresql.org/message-id/Zx7oLCnqis3FjgCK%40paquier.xyz

On 9/25/25 15:15, Daniil Davydov wrote:
> I don't know what exactly is meant by merging. Maybe we should just
> apply a current
> patch that fixes all problems ?..

Here I just wanted to bring to your attention that we have duplicate
efforts with these two patches. This one covers much more ground though ;)

> OK, I'll replace "could not" with "cannot" in order to match previous comments.

Small typo (you forgot to remove one "not")

errmsg("cannot not access temporary relations of other sessions")

It should be something like:

errmsg("cannot access temporary relations from other sessions")

Best regards, Jim

Hi,

On Thu, Sep 25, 2025 at 9:04 PM Jim Jones <jim.jones@uni-muenster.de> wrote:
>
> Small typo (you forgot to remove one "not")
>
> errmsg("cannot not access temporary relations of other sessions")
>
> It should be something like:
>
> errmsg("cannot access temporary relations from other sessions")
>

Oh, my bad. Fixed. Thanks for noticing it.

--
Best regards,
Daniil Davydov

The code LGTM (commit message still missing though)

Given that the lack of tests allowed this bug to go undetected until
now, I'd suggest to include additional tests in this patch to prevent
similar issues in the future. Something like 0002 attached. What do you
think?

Best, Jim

Hi,

On Fri, Sep 26, 2025 at 6:19 PM Jim Jones <jim.jones@uni-muenster.de> wrote:
>
> Given that the lack of tests allowed this bug to go undetected until
> now, I'd suggest to include additional tests in this patch to prevent
> similar issues in the future. Something like 0002 attached. What do you
> think?
>

Thanks for the test! Some time ago I wrote an isolation test [1] for
this patch, but
it looked a bit ugly, so I decided to abandon it temporarily. Your
test looks much
better. I'd prefer to keep it.

[1] https://www.postgresql.org/message-id/CAJDiXgi9CWaZCVcHmvAT604RrAqDN5zpOYxZq92adqkPq5QbnQ@mail.gmail.com

--
Best regards,
Daniil Davydov