From 240c668d120065534b1d298d6facc86839fcbab9 Mon Sep 17 00:00:00 2001
From: Tatsuo Ishii <ishii@sraoss.co.jp>
Date: Mon, 18 Mar 2024 10:33:16 +0900
Subject: [PATCH] Guard against inappropriate protocol data.

If a simple query message arrives before a sequence of extended query
messages ends (that is, no sync message arrives or some ready for
query messages corresponding the sync message do not arrive yet),
pgpool could hang.  This is because the query context in the session
context for the simple query is overwritten by the query contexts of
the extended query messages.

This commit implements a guard in SimpleQuery() by checking whether
extended query protocol messages ended. If they do not end, raise a
FATAL error. A known example detected by this checking is JDBC
driver's "autosave=always" option. This means pgpool will not accept
the option after this commit until the issue (sending a simple
protocol message before ending extended query message protocol) is
fixed by the JDBC driver side.

Discussion:
[pgpool-hackers: 4427] Guard against ill mannered frontend
https://www.pgpool.net/pipermail/pgpool-hackers/2024-February/004428.html
---
 src/protocol/pool_proto_modules.c             | 14 +++++++
 .../pgproto.data                              |  7 ++++
 .../082.guard_against_bad_protocol/test.sh    | 42 +++++++++++++++++++
 3 files changed, 63 insertions(+)
 create mode 100644 src/test/regression/tests/082.guard_against_bad_protocol/pgproto.data
 create mode 100755 src/test/regression/tests/082.guard_against_bad_protocol/test.sh

diff --git a/src/protocol/pool_proto_modules.c b/src/protocol/pool_proto_modules.c
index 553aebe55..439bcf83c 100644
--- a/src/protocol/pool_proto_modules.c
+++ b/src/protocol/pool_proto_modules.c
@@ -220,6 +220,20 @@ SimpleQuery(POOL_CONNECTION * frontend,
 	/* save last query string for logging purpose */
 	strlcpy(query_string_buffer, contents, sizeof(query_string_buffer));
 
+	/*
+	 * Check if extended query protocol message ended.  If not, reject the
+	 * query and raise an error to terminate the session to avoid hanging up.
+	 */
+	if (SL_MODE)
+	{
+		if (pool_is_doing_extended_query_message() ||
+			pool_pending_message_head_message())
+
+			ereport(FATAL,
+					(errmsg("simple query \"%s\" arrived before ending an extended query message",
+							query_string_buffer)));
+	}
+
 	/* show ps status */
 	query_ps_status(contents, backend);
 
diff --git a/src/test/regression/tests/082.guard_against_bad_protocol/pgproto.data b/src/test/regression/tests/082.guard_against_bad_protocol/pgproto.data
new file mode 100644
index 000000000..4b5bae6d0
--- /dev/null
+++ b/src/test/regression/tests/082.guard_against_bad_protocol/pgproto.data
@@ -0,0 +1,7 @@
+'P'	""	"BEGIN"	0
+'B'	""	""	0	0	0
+'E'	""	0
+'Q'	"SAVEPOINT PGJDBC_AUTOSAVE"
+'Y'
+'P'	""	"SELECT 1"	0
+'X'
diff --git a/src/test/regression/tests/082.guard_against_bad_protocol/test.sh b/src/test/regression/tests/082.guard_against_bad_protocol/test.sh
new file mode 100755
index 000000000..7b9529c90
--- /dev/null
+++ b/src/test/regression/tests/082.guard_against_bad_protocol/test.sh
@@ -0,0 +1,42 @@
+#!/usr/bin/env bash
+#-------------------------------------------------------------------
+# Test script for pgpool hang due to protocol violation.
+# If frontend sends simple query before extended protocol ended, pgpool hangs.
+# 
+# Discussion:
+# [pgpool-general: 8990] autosave=always jdbc option & it only sends query SAVEPOINT PGJDBC_AUTOSAVE and hangs
+# [pgpool-hackers: 4427] Guard against ill mannered frontend
+#
+source $TESTLIBS
+TESTDIR=testdir
+PSQL=$PGBIN/psql
+PG_CTL=$PGBIN/pg_ctl
+PGPROTO=$PGPOOL_INSTALL_DIR/bin/pgproto
+export PGDATABASE=test
+
+rm -fr $TESTDIR
+mkdir $TESTDIR
+cd $TESTDIR
+
+echo -n "creating test environment..."
+$PGPOOL_SETUP || exit 1
+echo "done."
+echo "backend_weight1=0" >> etc/pgpool.conf
+source ./bashrc.ports
+./startall
+wait_for_pgpool_startup
+
+# Wait for 1 seconds before pgproto ended.
+# Usually 1 seconds should be enough to finish pgproto.
+# If test suceeded, pgpool emits an error message:
+# "FATAL:  simple query "SAVEPOINT PGJDBC_AUTOSAVE" arrived before ending an extended query message"
+# grep command below should catch the message.
+timeout 1 $PGPROTO -d $PGDATABASE -p $PGPOOL_PORT -f ../pgproto.data |& grep 'simple query "SAVEPOINT PGJDBC_AUTOSAVE" arrived '
+
+if [ $? != 0 ];then
+    echo "test failed."
+    ./shutdownall
+    exit 1
+fi
+./shutdownall
+exit 0
-- 
2.39.5