Re: Fix bug with accessing to temporary tables of other sessions

On Mon, Apr 14, 2025 at 12:36 PM Daniil Davydov <3danissimo(at)gmail(dot)com>
wrote:

> 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 Daniil,

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.

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 noticed the Assert assumes myTempNamespace is always valid. While
correct, a brief comment explaining why this is safe would help future
maintainers. The relpersistence logic could also be centralized in one
place for consistency.

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.

Thanks for working on this important security enhancement!

Best regards,
Stepan Neretin