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

Moving the report from security to -hackers on Noah's advice. Since
the function(s) involved in the crash are not present in any of the
released versions, it is not considered a security issue.

I can confirm that this is reproducible on the latest commit on
master, 3c0bcdbc66. Below is the original analysis, followed by Noah's
analysis.

To be able to reproduce it, please note that perl support is required;
hence `./configure --with-perl`.

The note about 'security concerns around on_plperl_init parameter',
below, refers to now-fixed issue, at commit 13d8388151.

Best regards,
Gurjeet
http://Gurje.et

Forwarded Conversation
Subject: Unprivileged user can induce crash by using an SUSET param in PGOPTIONS
------------------------

From: Gurjeet Singh <gurjeet(at)singh(dot)im>
Date: Mon, Jul 4, 2022 at 10:24 AM
To: Postgres Security <security(at)postgresql(dot)org>
Cc: Bossart, Nathan <bossartn(at)amazon(dot)com>

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.

The condition:
User specifies PGOPTIONS"-c custom.param"
"custom.param" is used by an extension which is specified in
session_preload_libraries
The extension uses DefineCustom*Variable("custom.param", PGC_SUSET)
inside set_config_option()
record->context == PGC_SUSET
context == PGC_BACKEND
calls pg_parameter_aclcheck() -> eventually leads to
assertion-failure (or crash, when assertions are disabled)

See below for 1. How to reproduce, 2. Assertion failure stack, and 3.
Crash stack

When the user does not specify PGOPTIONS, the code in
define_custom_variable() returns prematurely, after a failed bsearch()
lookup, and hence avoids this bug.

I think similar crash can be induced when the custom parameter is of
kind PGC_SU_BACKEND, because the code to handle that also invokes
pg_parameter_aclcheck(). Also, I believe the same condition would
arise if the extension is specified local_preload_libraries.

I haven't been able to think of an attack vector using this bug, but
it can be used to cause a denial-of-service by an authenticated user.
I'm sending this report to security list, instead of -hackers, out of
abundance of caution; please feel free to move it to -hackers, if it's
not considered a security concern.

I discovered this bug a couple of days ago, just before leaving on a
trip. But because of shortage of time over the weekend, I haven't been
able to dig deeper into it. Since I don't think I'll be able to spend
any significant time on it for at least another couple of days, I'm
sending this report without a patch or a proposed fix.

CC: Nathan, whose security concerns around on_plperl_init parameter
lead to this discovery.

[1]: How to reproduce

$ psql -c 'create user test'
CREATE ROLE

$ psql -c "alter system set session_preload_libraries='plperl'"
ALTER SYSTEM

$ # restart server

$ psql -c 'show session_preload_libraries'
session_preload_libraries
---------------------------
plperl
(1 row)

$ PGOPTIONS="-c plperl.on_plperl_init=" psql -U test
psql: error: connection to server on socket "/tmp/.s.psql.5432"
failed: server closed the connection unexpectedly
┆ This probably means the server terminated abnormally
before or while processing the request.

[2]: Assertion failure stack

LOG: database system is ready to accept connections
TRAP: FailedAssertion("IsTransactionState()", File:
"../../../../../../POSTGRES/src/backend/utils/cache/catcache.c", Line:
1209, P
ID: 199868)
postgres: test postgres [local]
startup(ExceptionalCondition+0xd0)[0x55e503a4e6c9]
postgres: test postgres [local] startup(+0x7e069b)[0x55e503a2a69b]
postgres: test postgres [local] startup(SearchCatCache1+0x3a)[0x55e503a2a56b]
postgres: test postgres [local] startup(SearchSysCache1+0xc1)[0x55e503a46fe4]
postgres: test postgres [local]
startup(pg_parameter_aclmask+0x6f)[0x55e50345f098]
postgres: test postgres [local]
startup(pg_parameter_aclcheck+0x2d)[0x55e50346039c]
postgres: test postgres [local] startup(set_config_option+0x450)[0x55e503a70727]
postgres: test postgres [local] startup(+0x829ce8)[0x55e503a73ce8]
postgres: test postgres [local]
startup(DefineCustomStringVariable+0xa4)[0x55e503a74306]
/home/vagrant/dev/POSTGRES_builds/add_tz_param/db/lib/postgresql/plperl.so(_PG_init+0xd7)[0x7fed3d845425]
postgres: test postgres [local] startup(+0x80cc50)[0x55e503a56c50]
postgres: test postgres [local] startup(load_file+0x43)[0x55e503a566d9]
postgres: test postgres [local] startup(+0x81ba89)[0x55e503a65a89]
postgres: test postgres [local]
startup(process_session_preload_libraries+0x23)[0x55e503a65bc6]
postgres: test postgres [local] startup(PostgresMain+0x23b)[0x55e50388a52a]
postgres: test postgres [local] startup(+0x564c5d)[0x55e5037aec5d]
postgres: test postgres [local] startup(+0x564542)[0x55e5037ae542]
postgres: test postgres [local] startup(+0x560777)[0x55e5037aa777]
postgres: test postgres [local] startup(PostmasterMain+0x1374)[0x55e5037a9f10]
postgres: test postgres [local] startup(+0x451550)[0x55e50369b550]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3)[0x7fed46dac083]
postgres: test postgres [local] startup(_start+0x2e)[0x55e503317eae]
LOG: server process (PID 199868) was terminated by signal 6: Aborted
LOG: terminating any other active server processes

[3]: Crash stack

(gdb) bt
#0 0x0000560937b35206 in ResourceArrayEnlarge (resarr=0x80)
at ../../../../../../POSTGRES/src/backend/utils/resowner/resowner.c:222
#1 0x0000560937b36693 in ResourceOwnerEnlargeRelationRefs (owner=0x0)
at ../../../../../../POSTGRES/src/backend/utils/resowner/resowner.c:1106
#2 0x0000560937ae32ca in RelationIncrementReferenceCount (rel=0x7fb697a0b860)
at ../../../../../../POSTGRES/src/backend/utils/cache/relcache.c:2128
#3 0x0000560937ae322b in RelationIdGetRelation (relationId=6243)
at ../../../../../../POSTGRES/src/backend/utils/cache/relcache.c:2074
#4 0x00005609374758a3 in relation_open (relationId=6243, lockmode=1)
at ../../../../../../POSTGRES/src/backend/access/common/relation.c:59
#5 0x00005609375181ca in table_open (relationId=6243, lockmode=1)
at ../../../../../../POSTGRES/src/backend/access/table/table.c:43
#6 0x0000560937ad41dc in SearchCatCacheMiss (cache=0x560938b60500,
nkeys=1, hashValue=658344123, hashIndex=3,
v1=94597605943240, v2=0, v3=0, v4=0) at
../../../../../../POSTGRES/src/backend/utils/cache/catcache.c:1353
#7 0x0000560937ad40cc in SearchCatCacheInternal
(cache=0x560938b60500, nkeys=1, v1=94597605943240, v2=0, v3=0, v4=0)
at ../../../../../../POSTGRES/src/backend/utils/cache/catcache.c:1295
#8 0x0000560937ad3de7 in SearchCatCache1 (cache=0x560938b60500,
v1=94597605943240)
at ../../../../../../POSTGRES/src/backend/utils/cache/catcache.c:1163
#9 0x0000560937aedba7 in SearchSysCache1 (cacheId=41, key1=94597605943240)
at ../../../../../../POSTGRES/src/backend/utils/cache/syscache.c:1180
#10 0x00005609375658b0 in pg_parameter_aclmask (name=0x560938b26670
"plperl.on_plperl_init", roleid=16384, mask=4096,
how=ACLMASK_ANY) at
../../../../../POSTGRES/src/backend/catalog/aclchk.c:4234
#11 0x0000560937566b82 in pg_parameter_aclcheck (name=0x560938b26670
"plperl.on_plperl_init", roleid=16384, mode=4096)
at ../../../../../POSTGRES/src/backend/catalog/aclchk.c:5048
#12 0x0000560937b14fbc in set_config_option (name=0x560938b26670
"plperl.on_plperl_init", value=0x560938ba13a0 "",
context=PGC_BACKEND, source=PGC_S_CLIENT, action=GUC_ACTION_SET,
changeVal=true, elevel=19, is_reload=false)
at ../../../../../../POSTGRES/src/backend/utils/misc/guc.c:7735
#13 0x0000560937b18408 in define_custom_variable (variable=0x560938b265c0)
at ../../../../../../POSTGRES/src/backend/utils/misc/guc.c:9361
#14 0x0000560937b189fa in DefineCustomStringVariable
(name=0x7fb697963114 "plperl.on_plperl_init",
short_desc=0x7fb6979630d0 "Perl initialization code to execute
once when plperl is first used.", long_desc=0x0,
valueAddr=0x7fb697968730 <plperl_on_plperl_init>, bootValue=0x0,
context=PGC_SUSET, flags=0, check_hook=0x0,
assign_hook=0x0, show_hook=0x0) at
../../../../../../POSTGRES/src/backend/utils/misc/guc.c:9589
#15 0x00007fb69795234d in _PG_init () at
../../../../../POSTGRES/src/pl/plperl/plperl.c:443
#16 0x0000560937afcc8c in internal_load_library (
--Type <RET> for more, q to quit, c to continue without paging--
libname=0x560938b2e188
"/home/vagrant/dev/POSTGRES_builds/add_tz_param/db/lib/postgresql/plperl.so")
at ../../../../../../POSTGRES/src/backend/utils/fmgr/dfmgr.c:289
#17 0x0000560937afc729 in load_file (filename=0x560938b2e748 "plperl",
restricted=false)
at ../../../../../../POSTGRES/src/backend/utils/fmgr/dfmgr.c:156
#18 0x0000560937b0ab02 in load_libraries (libraries=0x560938b1b5c0
"plperl", gucname=0x560937cd3706 "session_preload_libraries",
restricted=false) at
../../../../../../POSTGRES/src/backend/utils/init/miscinit.c:1668
#19 0x0000560937b0ac3f in process_session_preload_libraries ()
at ../../../../../../POSTGRES/src/backend/utils/init/miscinit.c:1699
#20 0x0000560937945908 in PostgresMain (dbname=0x560938af9ca8
"postgres", username=0x560938b2b0d8 "test")
at ../../../../../POSTGRES/src/backend/tcop/postgres.c:4170
#21 0x00005609378827ce in BackendRun (port=0x560938b240e0) at
../../../../../POSTGRES/src/backend/postmaster/postmaster.c:4504
#22 0x00005609378820b3 in BackendStartup (port=0x560938b240e0)
at ../../../../../POSTGRES/src/backend/postmaster/postmaster.c:4232
#23 0x000056093787e5e3 in ServerLoop () at
../../../../../POSTGRES/src/backend/postmaster/postmaster.c:1806
#24 0x000056093787dd86 in PostmasterMain (argc=3, argv=0x560938af7be0)
at ../../../../../POSTGRES/src/backend/postmaster/postmaster.c:1478
#25 0x000056093778078f in main (argc=3, argv=0x560938af7be0) at
../../../../../POSTGRES/src/backend/main/main.c:202
(gdb)

Best regards,
Gurjeet
http://Gurje.et

----------
From: Noah Misch <noah(at)leadboat(dot)com>
Date: Tue, Jul 5, 2022 at 10:50 AM
To: Gurjeet Singh <gurjeet(at)singh(dot)im>
Cc: Postgres Security <security(at)postgresql(dot)org>, Bossart, Nathan
<bossartn(at)amazon(dot)com>

On Mon, Jul 04, 2022 at 10:24:13AM -0700, Gurjeet Singh wrote:
> calls pg_parameter_aclcheck() -> eventually leads to
> assertion-failure (or crash, when assertions are disabled)

> I'm sending this report to security list, instead of -hackers, out of
> abundance of caution; please feel free to move it to -hackers, if it's
> not considered a security concern.

Thanks for the report. v14 doesn't have pg_parameter_aclcheck(). If this is
specific to unreleased and beta versions, do use -hackers.