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

Hi all,

On Wed, Mar 25, 2026 at 12:38 PM Daniil Davydov <3danissimo(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Wed, Mar 25, 2026 at 1:58 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > > Actually, v12 patch is not about a superuser rights restriction, but about
> > > forbidding such operations for everyone.
> >
> > ... including superusers, who bypass permissions restrictions
> > everywhere else. You are going to have to contort the ACL system
> > badly to make that happen at all, and I would not be surprised
> > if you introduce new bugs.
> >
>
> I've never dealt with the ACL system before, so it's hard for me to assess
> the scale of the problem. I am inclined to believe you on this issue.
>
> > >> The actual problem is that the buffer manager is incapable of dealing
> > >> with other sessions' temp tables, and we need to un-break the buffer
> > >> manager's defense for that implementation restriction. So I feel the
> > >> correct approach is something similar to what I described here:
> > >> https://www.postgresql.org/message-id/flat/2736425.1758475979%40sss.pgh.pa.us
> >
> > > Handling access to other-temp-tables on the buffer manager level seems to me
> > > like fighting the symptom, not the cause.
> >
> > No, it IS the cause. If someday someone were to reimplement buffer
> > management in a way that didn't have this implementation restriction,
> > we would surely not arbitrarily restrict superusers from looking at
> > tables that they then would physically be able to look at.
> >
>
> OK, now I fully understand your point (I hope so) :
> We want to restrict backend access to other-temp-tables just because it is
> physically impossible for them to read the pages of such tables. And if some
> users has enough privileges, it is OK for us to allow them to
> lock/vacuum/drop/... other-temp-tables. I.e. only operations with heap pages
> access must be forbidden, and the buffer manager layer is an appropriate place
> for it.
>
> > > Am I missing something?
> >
> > Mainly, that we had a setup that was working fine for decades,
> > until somebody made holes in it with careless refactoring.
> > We should fix that mistake, not introduce inconsistent-with-
> > decades-of-practice permissions behavior to hide the mistake
> > at an unrelated logical level.
> >
>
> Yeah, we have a few checks in the bufmgr (PrefetchBuffer, ReadBufferExtended),
> but they stopped coping with their task.
>
> > Also, we need a defense at the buffer manager level anyway, because
> > otherwise C code could try to access another session's temp table
> > and we'd not realize it was getting bogus answers. (Whether such
> > an attempt is a bug or not is a different discussion; but we at
> > least need some logic that detects that it won't work, and the ACL
> > system cannot be expected to stop C-level code from trying.)
> >
> > Also, we really need a patch that's simple and non-invasive enough
> > to be back-patched into v17 and v18. This proposal is not that.
> >
>
> OK
>
> >
> > > I don't actually want to use gram.y as a main solver of this issue. But
> > > gram.y is setting the "relpersistence" field for the RangeVar and all
> > > subsequent code is treating this value as truthful.
> >
> > I do kind of agree with this concern, but the v12 patch simply moves
> > the untruthfulness around. Reality is that we cannot know whether an
> > unqualified-name RangeVar references a temp table until we do a
> > catalog lookup, ...
>
> Yep, Jim's example shows us that we cannot always rely on the "relpersistence"
> field.
>
> > ...so IMO we should not have a relpersistence field there
> > at all. At best it means something quite different from what it means
> > elsewhere, and that's a recipe for confusion. But changing that would
> > not be a bug fix (AFAIK) but refactoring to reduce the probability of
> > future bugs.
> >
>
> I agree with the idea to get rid of this field. By now I cannot say for sure
> whether we can fix a bug without modifying the RangeVar structure. But I'll
> try to implement proposed logic only within the bufmgr.
>
>
> Thank you very much for your comments! I'll post a new patch in the near
> future.

I worked on the issue of accessing temporary tables belonging to other
sessions and tried implementing the fix at the buffer manager level,
as suggested. I added checks in ReadBuffer_common() and
PrefetchBuffer() to reject access when a relation is temporary
(relpersistence = TEMP) but does not use local buffers
(!RelationUsesLocalBuffers) so that it ensures only heap page access
is blocked, while catalog lookups and other metadata operations
continue to work as before. While testing, I observed that in many
cases the query does not reach the buffer manager because name
resolution fails earlier with “relation does not exist”. However, the
added checks ensure that even if execution reaches the buffer layer,
access to other sessions’ temporary tables is safely rejected. The
change is minimal, and did not modify parser/ACL behavior and all
regression tests got passed successfully too.
Kindly review the attached patch herewith. Please let me know if this
approach aligns with expectations or if further adjustments are
needed.

Regards,
Soumya