Re: Unprivileged user can induce crash by using an SUSET param in PGOPTIONS

Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> writes:
> At Fri, 22 Jul 2022 10:00:34 +0900, Michael Paquier <michael(at)paquier(dot)xyz> wrote in
>> On Thu, Jul 21, 2022 at 05:39:35PM -0700, Gurjeet Singh wrote:
>>> The patch also adds an assertion in pg_parameter_aclcheck() to ensure
>>> that there's a transaction is in progress before it's called.

>> This can involve extension code, I think that this should be at least
>> an elog(ERROR) so as we have higher chances of knowing if something
>> still goes wrong in the wild.

That assert strikes me as having been inserted with the advice of a
dartboard. Why pg_parameter_aclcheck, and not any other aclchk.c
functions? Why in the callers at all, rather than somewhere down
inside the syscache code? And why isn't the existing Assert that
you started the thread with plenty sufficient for that already?

> This patch makes process_session_preload_libraries called in
> autovacuum worker/launcher and background worker in addition to client
> backends. It seems to me we also need to prevent that.

Yeah. I think the definition of session/local_preload_libraries
is that it loads libraries into *interactive* sessions. The
existing coding seems already buggy in that regard, because it
will load such libraries into walsenders as well; isn't that
a POLA violation?

So I propose the attached. I tested this to the extent of checking
that all our contrib modules can be loaded via session_preload_libraries.
That doesn't prove a whole lot about what outside extensions might do,
but it's some comfort.

regards, tom lane