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

Gurjeet Singh <gurjeet(at)singh(dot)im> writes:
> While poking at plperl's GUC in an internal discussion, I was able to
> induce a crash (or an assertion failure in assert-enabled builds) as
> an unprivileged user.
> My investigation so far has revealed that the code path for the
> following condition has never been tested, and because of this, when a
> user tries to override an SUSET param via PGOPTIONS, Postgres tries to
> perform a table lookup during process initialization. Because there's
> no transaction in progress, and because this table is not in the
> primed caches, we end up with code trying to dereference an
> uninitialized CurrentResourceOwner.

Right. So there are basically two things we could do about this:

1. set_config_option could decline to call pg_parameter_aclcheck
if not IsTransactionState(), instead failing the assignment.
This isn't a great answer because it would result in disallowing
GUC assignments that users might expect to work.

2. We could move process_session_preload_libraries() to someplace
where a transaction is in progress -- probably, relocate it to
inside InitPostgres().

I'm inclined to think that #2 is a better long-term solution,
because it'd allow you to session-preload libraries that expect
to be able to do database access during _PG_init. (Right now
that'd fail with the same sort of symptoms seen here.) But
there's no denying that this might have surprising side-effects
for extensions that weren't expecting such a change.

It could also be reasonable to do both #1 and #2, with the idea
that #1 might save us from crashing if there are any other
code paths where we can reach that pg_parameter_aclcheck call
outside a transaction.

Thoughts?

regards, tom lane