agora inbox for pgsql-bugs@postgresql.org  
help / color / mirror / Atom feed
From: Grigorev Jurij <ju.grigorev@ftdata.ru>
To: pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Cc: jacob.champion@enterprisedb.com <jacob.champion@enterprisedb.com>
Cc: daniel@yesql.se <daniel@yesql.se>
Subject: Postmaster crashes on SIGHUP when oauth_validator_libraries holds only whitespace
Date: Mon, 14 Sep 2026 11:22:42 +0000
Message-ID: <04fa84f6ebbe400f940e179ebe1070e9@localhost.localdomain> (raw)

Hi,

I ran into a postmaster crash while investigating a static analyzer
report against 18.6.  I reproduced it on master (92aaf50e230,
--enable-cassert, Debian 13/aarch64).

To reproduce, set the following in postgresql.conf:

	oauth_validator_libraries = '   '

and add an OAuth line without validator= to pg_hba.conf:

	host all all 127.0.0.1/32 oauth issuer="https://example.com"; scope="openid"

On reload the server log ends here:

	LOG:  received SIGHUP, reloading configuration files
	LOG:  parameter "oauth_validator_libraries" changed to "   "

gdb then reports SIGSEGV in check_oauth_validator(), via load_hba().
The running instance goes down with every session on it.  The same
configuration also prevents startup.  pg_hba_file_rules() parses the
file from a regular backend, so the same NULL dereference there kills
the backend and the postmaster restarts the cluster.

check_oauth_validator() treats an unset list as an error by looking at
the raw GUC string, but SplitDirectoriesString() accepts a string of
spaces and returns NIL.  The subsequent elemlist->length dereferences
that.  This is a misconfiguration rather than a security issue -- the
GUC has PGC_SIGHUP context and is marked GUC_SUPERUSER_ONLY.

I've attached a patch against master that checks the parsed list
instead.  It also adds a TAP test for reload.  The patched master built
without warnings, and "make check" in src/test/modules/oauth_validator
passed with PG_TEST_EXTRA=oauth.

This appears to affect v18 onward, where OAuth support was added, so it
may need back-patching.  The v18 error message has different wording
and would need a small adjustment.

Could someone take a look at the attached patch and let me know
if this is the right fix?

Thanks,
  Yuriy=

Attachments:

  [application/octet-stream] v1-0001-Fix-postmaster-crash-on-whitespace-only-oauth_valida.patch (4.1K, ../04fa84f6ebbe400f940e179ebe1070e9@localhost.localdomain/2-v1-0001-Fix-postmaster-crash-on-whitespace-only-oauth_valida.patch)
  download | inline diff:
From cf766ea59614e3c72a698e4f6dc4cd919da1b0d5 Mon Sep 17 00:00:00 2001
From: Yuriy Grigoryev <ju.grigorev@ftdata.ru>
Date: Mon, 14 Sep 2026 15:31:22 +0700
Subject: [PATCH] Fix postmaster crash on whitespace-only
 oauth_validator_libraries

check_oauth_validator() rejects an unset validator list by looking at the
raw GUC string, which does not cover a value made up of whitespace only.
SplitDirectoriesString() accepts such a value and hands back an empty
list, so when the HBA line carries no validator= option the code went on
to read elemlist->length and dereferenced NIL.

Every entry point into HBA parsing is affected.  load_hba() runs in the
postmaster at startup and again on SIGHUP, so reloading a running server
brings the whole instance down along with every session on it, and
pg_hba_file_rules() parses the file from a regular backend, where the
same crash forces a cluster-wide restart.

Check the parse result instead of the raw string, keeping the message the
empty setting already produced.  Add a test that reloads a whitespace-only
setting and verifies that the server reports the error and stays up.
---
 src/backend/libpq/auth-oauth.c                | 31 +++++++++++--------
 .../modules/oauth_validator/t/001_server.pl   | 16 ++++++++++
 2 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c
index b769931..8dfe5f9 100644
--- a/src/backend/libpq/auth-oauth.c
+++ b/src/backend/libpq/auth-oauth.c
@@ -863,19 +863,6 @@ check_oauth_validator(HbaLine *hbaline, int elevel, char **err_msg)
 
 	*err_msg = NULL;
 
-	if (oauth_validator_libraries_string[0] == '\0')
-	{
-		ereport(elevel,
-				errcode(ERRCODE_CONFIG_FILE_ERROR),
-				errmsg("parameter \"%s\" must be set for authentication method \"%s\"",
-					   "oauth_validator_libraries", "oauth"),
-				errcontext("line %d of configuration file \"%s\"",
-						   line_num, file_name));
-		*err_msg = psprintf("parameter \"%s\" must be set for authentication method \"%s\"",
-							"oauth_validator_libraries", "oauth");
-		return false;
-	}
-
 	/* SplitDirectoriesString needs a modifiable copy */
 	rawstring = pstrdup(oauth_validator_libraries_string);
 
@@ -891,6 +878,24 @@ check_oauth_validator(HbaLine *hbaline, int elevel, char **err_msg)
 		goto done;
 	}
 
+	/*
+	 * An empty or all-whitespace setting is accepted by
+	 * SplitDirectoriesString(), which returns an empty list for it, so the
+	 * parse result has to be checked rather than the raw string.
+	 */
+	if (elemlist == NIL)
+	{
+		ereport(elevel,
+				errcode(ERRCODE_CONFIG_FILE_ERROR),
+				errmsg("parameter \"%s\" must be set for authentication method \"%s\"",
+					   "oauth_validator_libraries", "oauth"),
+				errcontext("line %d of configuration file \"%s\"",
+						   line_num, file_name));
+		*err_msg = psprintf("parameter \"%s\" must be set for authentication method \"%s\"",
+							"oauth_validator_libraries", "oauth");
+		goto done;
+	}
+
 	if (!hbaline->oauth_validator)
 	{
 		if (elemlist->length == 1)
diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl
index 8941a35..86307be 100644
--- a/src/test/modules/oauth_validator/t/001_server.pl
+++ b/src/test/modules/oauth_validator/t/001_server.pl
@@ -120,6 +120,22 @@ local all testparam oauth issuer="$issuer/param" scope="openid postgres"
 });
 $node->reload;
 
+$log_start =
+  $node->wait_for_log(qr/reloading configuration files/, $log_start);
+
+# An all-whitespace library list parses as an empty list.  Reject it without
+# crashing the postmaster during HBA reload.
+$node->append_conf('postgresql.conf',
+	"oauth_validator_libraries = '   '\n");
+$node->reload;
+$log_start = $node->wait_for_log(
+	qr/parameter "oauth_validator_libraries" must be set/, $log_start);
+is($bgconn->query_safe('SELECT 1'), '1',
+	'postmaster survives an empty OAuth validator list on reload');
+
+$node->append_conf('postgresql.conf',
+	"oauth_validator_libraries = 'validator'\n");
+$node->reload;
 $log_start =
   $node->wait_for_log(qr/reloading configuration files/, $log_start);
 
-- 
2.39.5 (Apple Git-154)



view thread (10+ messages)  latest in thread

Message-ID: <04fa84f6ebbe400f940e179ebe1070e9@localhost.localdomain>
Permalink:  ../04fa84f6ebbe400f940e179ebe1070e9@localhost.localdomain/
Also on:    postgresql.org/message-id/04fa84f6ebbe400f940e179ebe1070e9@localhost.localdomain

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-bugs@postgresql.org
  Cc: ju.grigorev@ftdata.ru, pgsql-bugs@lists.postgresql.org, jacob.champion@enterprisedb.com, daniel@yesql.se
  Subject: Re: Postmaster crashes on SIGHUP when oauth_validator_libraries holds only whitespace
  In-Reply-To: <04fa84f6ebbe400f940e179ebe1070e9@localhost.localdomain>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox