From 878a9b09c729a6ec6bc0b5e3a66b7715a18d0d8e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 4 Nov 2019 09:54:47 +0100
Subject: [PATCH] Catch invalid typlens in a couple of places

Rearrange the logic in record_image_cmp() and record_image_eq() to
error out on unexpected typlens (either not supported there or
completely invalid due to corruption).  Barring corruption, this is
not possible today but it seems more future-proof and robust to fix
this.

Reported-by: Peter Geoghegan <pg@bowt.ie>
---
 src/backend/utils/adt/rowtypes.c | 117 ++++++++++++++++---------------
 1 file changed, 61 insertions(+), 56 deletions(-)

diff --git a/src/backend/utils/adt/rowtypes.c b/src/backend/utils/adt/rowtypes.c
index e1c72a12324..4f316f7e49a 100644
--- a/src/backend/utils/adt/rowtypes.c
+++ b/src/backend/utils/adt/rowtypes.c
@@ -1426,30 +1426,7 @@ record_image_cmp(FunctionCallInfo fcinfo)
 			}
 
 			/* Compare the pair of elements */
-			if (tupdesc1->attrs[i1]->attlen == -1)
-			{
-				Size		len1,
-							len2;
-				struct varlena *arg1val;
-				struct varlena *arg2val;
-
-				len1 = toast_raw_datum_size(values1[i1]);
-				len2 = toast_raw_datum_size(values2[i2]);
-				arg1val = PG_DETOAST_DATUM_PACKED(values1[i1]);
-				arg2val = PG_DETOAST_DATUM_PACKED(values2[i2]);
-
-				cmpresult = memcmp(VARDATA_ANY(arg1val),
-								   VARDATA_ANY(arg2val),
-								   Min(len1, len2) - VARHDRSZ);
-				if ((cmpresult == 0) && (len1 != len2))
-					cmpresult = (len1 < len2) ? -1 : 1;
-
-				if ((Pointer) arg1val != (Pointer) values1[i1])
-					pfree(arg1val);
-				if ((Pointer) arg2val != (Pointer) values2[i2])
-					pfree(arg2val);
-			}
-			else if (tupdesc1->attrs[i1]->attbyval)
+			if (tupdesc1->attrs[i1]->attbyval)
 			{
 				switch (tupdesc1->attrs[i1]->attlen)
 				{
@@ -1491,12 +1468,37 @@ record_image_cmp(FunctionCallInfo fcinfo)
 						Assert(false);	/* cannot happen */
 				}
 			}
-			else
+			else if (tupdesc1->attrs[i1]->attlen > 0)
 			{
 				cmpresult = memcmp(DatumGetPointer(values1[i1]),
 								   DatumGetPointer(values2[i2]),
 								   tupdesc1->attrs[i1]->attlen);
 			}
+			else if (tupdesc1->attrs[i1]->attlen == -1)
+			{
+				Size		len1,
+							len2;
+				struct varlena *arg1val;
+				struct varlena *arg2val;
+
+				len1 = toast_raw_datum_size(values1[i1]);
+				len2 = toast_raw_datum_size(values2[i2]);
+				arg1val = PG_DETOAST_DATUM_PACKED(values1[i1]);
+				arg2val = PG_DETOAST_DATUM_PACKED(values2[i2]);
+
+				cmpresult = memcmp(VARDATA_ANY(arg1val),
+								   VARDATA_ANY(arg2val),
+								   Min(len1, len2) - VARHDRSZ);
+				if ((cmpresult == 0) && (len1 != len2))
+					cmpresult = (len1 < len2) ? -1 : 1;
+
+				if ((Pointer) arg1val != (Pointer) values1[i1])
+					pfree(arg1val);
+				if ((Pointer) arg2val != (Pointer) values2[i2])
+					pfree(arg2val);
+			}
+			else
+				elog(ERROR, "unexpected attlen: %d", tupdesc1->attrs[i1]->attlen);
 
 			if (cmpresult < 0)
 			{
@@ -1687,36 +1689,7 @@ record_image_eq(PG_FUNCTION_ARGS)
 			}
 
 			/* Compare the pair of elements */
-			if (tupdesc1->attrs[i1]->attlen == -1)
-			{
-				Size		len1,
-							len2;
-
-				len1 = toast_raw_datum_size(values1[i1]);
-				len2 = toast_raw_datum_size(values2[i2]);
-				/* No need to de-toast if lengths don't match. */
-				if (len1 != len2)
-					result = false;
-				else
-				{
-					struct varlena *arg1val;
-					struct varlena *arg2val;
-
-					arg1val = PG_DETOAST_DATUM_PACKED(values1[i1]);
-					arg2val = PG_DETOAST_DATUM_PACKED(values2[i2]);
-
-					result = (memcmp(VARDATA_ANY(arg1val),
-									 VARDATA_ANY(arg2val),
-									 len1 - VARHDRSZ) == 0);
-
-					/* Only free memory if it's a copy made here. */
-					if ((Pointer) arg1val != (Pointer) values1[i1])
-						pfree(arg1val);
-					if ((Pointer) arg2val != (Pointer) values2[i2])
-						pfree(arg2val);
-				}
-			}
-			else if (tupdesc1->attrs[i1]->attbyval)
+			if (tupdesc1->attrs[i1]->attbyval)
 			{
 				switch (tupdesc1->attrs[i1]->attlen)
 				{
@@ -1742,12 +1715,44 @@ record_image_eq(PG_FUNCTION_ARGS)
 						Assert(false);	/* cannot happen */
 				}
 			}
-			else
+			else if (tupdesc1->attrs[i1]->attlen > 0)
 			{
 				result = (memcmp(DatumGetPointer(values1[i1]),
 								 DatumGetPointer(values2[i2]),
 								 tupdesc1->attrs[i1]->attlen) == 0);
 			}
+			else if (tupdesc1->attrs[i1]->attlen == -1)
+			{
+				Size		len1,
+							len2;
+
+				len1 = toast_raw_datum_size(values1[i1]);
+				len2 = toast_raw_datum_size(values2[i2]);
+				/* No need to de-toast if lengths don't match. */
+				if (len1 != len2)
+					result = false;
+				else
+				{
+					struct varlena *arg1val;
+					struct varlena *arg2val;
+
+					arg1val = PG_DETOAST_DATUM_PACKED(values1[i1]);
+					arg2val = PG_DETOAST_DATUM_PACKED(values2[i2]);
+
+					result = (memcmp(VARDATA_ANY(arg1val),
+									 VARDATA_ANY(arg2val),
+									 len1 - VARHDRSZ) == 0);
+
+					/* Only free memory if it's a copy made here. */
+					if ((Pointer) arg1val != (Pointer) values1[i1])
+						pfree(arg1val);
+					if ((Pointer) arg2val != (Pointer) values2[i2])
+						pfree(arg2val);
+				}
+			}
+			else
+				elog(ERROR, "unexpected attlen: %d", tupdesc1->attrs[i1]->attlen);
+
 			if (!result)
 				break;
 		}
-- 
GitLab