On Tue, Jul 09, 2024 at 05:47:36PM -0700, Jeff Davis wrote:
> On Tue, 2024-07-09 at 15:20 +0900, Michael Paquier wrote:
> > On Sun, Jun 30, 2024 at 03:23:44PM -0700, Noah Misch wrote:
> > Hmm. Is RestrictSearchPath() something that we should advertise more
> > strongly, thinking here about extensions that call NewGUCNestLevel()?
> > That would be really easy to miss, and it could have bad
> > consequences.
> > I know that this is not something that's published in the release
> > notes, but it looks like something sensible to have, though.
>
> The pattern also involves SetUserIdAndSecContext(). Perhaps we could
> come up with a wrapper function to better encapsulate the general
> pattern?
Worth a look. usercontext.c has an existing wrapper for a superuser process
switching to an untrusted user. It could become the home for another wrapper
targeting MAINTAIN-relevant callers.
> > > While "not necessary for security", ExecCreateTableAs() should do
> > > it for the
> > > same reason it calls NewGUCNestLevel().
> >
> > +1.
>
> Do you have a suggestion about how that should be done?
>
> It's not trivial, because the both creates the table and populates it
> in ExecutorRun. For table creation, we need to use the original
> search_path, but we need to use the restricted search_path when
> populating it.
>
> I could try to refactor it into two statements and execute them
> separately, or I could try to rewrite the statement to use a fully-
> qualified destination table before execution. Thoughts?
Those sound fine. Also fine: just adding a comment on why creation namespace
considerations led to not doing it there.
