From 5bd087eb5d770624ea5ea903af7a7ec6c460d060 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Mon, 27 Jul 2020 15:58:59 +0900
Subject: [PATCH] Fix corner case with 16kB-long decompression in pgcrypto,
 take 2

A compressed stream may end with an empty packet.  In this case
decompression finishes before reading the empty packet and the
remaining stream packet causes a failure in reading the following
data.  This commit makes sure to consume such extra data, avoiding a
failure when decompression the data.  This corner case was reproducible
easily with a data length of 16kB, and existed since e94dd6a.  A cheap
regression test is added to cover this case based on a random,
incompressible string.

The first attempt of this patch has allowed to find an older failure
within the compression logic of pgcrypto, fixed by b9b6105.  This
involved SLES 15 with z390 where a custom flavor of libz gets used.
Bonus thanks to Mark Wong for providing access to the specific
environment.

Reported-by: Frank Gagnepain
Author: Kyotaro Horiguchi, Michael Paquier
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/16476-692ef7b84e5fb893@postgresql.org
Backpatch-through: 9.5
---
 contrib/pgcrypto/expected/pgp-compression.out | 30 +++++++++++++++++++
 contrib/pgcrypto/pgp-compress.c               | 21 +++++++++++++
 contrib/pgcrypto/sql/pgp-compression.sql      | 21 +++++++++++++
 3 files changed, 72 insertions(+)

diff --git a/contrib/pgcrypto/expected/pgp-compression.out b/contrib/pgcrypto/expected/pgp-compression.out
index 32b350b8fe..d4c57feba3 100644
--- a/contrib/pgcrypto/expected/pgp-compression.out
+++ b/contrib/pgcrypto/expected/pgp-compression.out
@@ -48,3 +48,33 @@ select pgp_sym_decrypt(
  Secret message
 (1 row)
 
+-- check corner case involving an input string of 16kB, as per bug #16476.
+SELECT setseed(0);
+ setseed 
+---------
+ 
+(1 row)
+
+WITH random_string AS
+(
+  -- This generates a random string of 16366 bytes.  This is chosen
+  -- as random so that it does not get compressed, and the decompression
+  -- would work on a string with the same length as the origin, making the
+  -- test behavior more predictible.  lpad() ensures that the generated
+  -- hexadecimal value is completed by extra zero characters if random()
+  -- has generated a value strictly lower than 16.
+  SELECT string_agg(decode(lpad(to_hex((random()*256)::int), 2, '0'), 'hex'), '') as bytes
+    FROM generate_series(0, 16365)
+)
+SELECT bytes =
+    pgp_sym_decrypt_bytea(
+      pgp_sym_encrypt_bytea(bytes, 'key',
+                            'compress-algo=1,compress-level=1'),
+                            'key', 'expect-compress-algo=1')
+    AS is_same
+  FROM random_string;
+ is_same 
+---------
+ t
+(1 row)
+
diff --git a/contrib/pgcrypto/pgp-compress.c b/contrib/pgcrypto/pgp-compress.c
index 7e5708dfb8..0dca665246 100644
--- a/contrib/pgcrypto/pgp-compress.c
+++ b/contrib/pgcrypto/pgp-compress.c
@@ -287,7 +287,28 @@ restart:
 
 	dec->buf_data = dec->buf_len - dec->stream.avail_out;
 	if (res == Z_STREAM_END)
+	{
+		uint8	   *tmp;
+
+		/*
+		 * A stream must be terminated by a normal packet.  If the last stream
+		 * packet in the source stream is a full packet, a normal empty packet
+		 * must follow.  Since the underlying packet reader doesn't know that
+		 * the compressed stream has been ended, we need to to consume the
+		 * terminating packet here.  This read does not harm even if the
+		 * stream has already ended.
+		 */
+		res = pullf_read(src, 1, &tmp);
+
+		if (res < 0)
+			return res;
+		else if (res > 0)
+		{
+			px_debug("decompress_read: extra bytes after end of stream");
+			return PXE_PGP_CORRUPT_DATA;
+		}
 		dec->eof = 1;
+	}
 	goto restart;
 }
 
diff --git a/contrib/pgcrypto/sql/pgp-compression.sql b/contrib/pgcrypto/sql/pgp-compression.sql
index ca9ee1fc00..87c59c6cab 100644
--- a/contrib/pgcrypto/sql/pgp-compression.sql
+++ b/contrib/pgcrypto/sql/pgp-compression.sql
@@ -28,3 +28,24 @@ select pgp_sym_decrypt(
 	pgp_sym_encrypt('Secret message', 'key',
 			'compress-algo=2, compress-level=0'),
 	'key', 'expect-compress-algo=0');
+
+-- check corner case involving an input string of 16kB, as per bug #16476.
+SELECT setseed(0);
+WITH random_string AS
+(
+  -- This generates a random string of 16366 bytes.  This is chosen
+  -- as random so that it does not get compressed, and the decompression
+  -- would work on a string with the same length as the origin, making the
+  -- test behavior more predictible.  lpad() ensures that the generated
+  -- hexadecimal value is completed by extra zero characters if random()
+  -- has generated a value strictly lower than 16.
+  SELECT string_agg(decode(lpad(to_hex((random()*256)::int), 2, '0'), 'hex'), '') as bytes
+    FROM generate_series(0, 16365)
+)
+SELECT bytes =
+    pgp_sym_decrypt_bytea(
+      pgp_sym_encrypt_bytea(bytes, 'key',
+                            'compress-algo=1,compress-level=1'),
+                            'key', 'expect-compress-algo=1')
+    AS is_same
+  FROM random_string;