Rework refactoring of hex and encoding routines

This commit addresses some issues with c3826f83 that moved the hex
decoding routine to src/common/:
- The decoding function lacked overflow checks, so when used for
security-related features it was an open door to out-of-bound writes if
not carefully used that could remain undetected.  Like the base64
routines already in src/common/ used by SCRAM, this routine is reworked
to check for overflows by having the size of the destination buffer
passed as argument, with overflows checked before doing any writes.
- The encoding routine was missing.  This is moved to src/common/ and
it gains the same overflow checks as the decoding part.

On failure, the hex routines of src/common/ issue an error as per the
discussion done to make them usable by frontend tools, but not by shared
libraries.  Note that this is why ECPG is left out of this commit, and
it still includes a duplicated logic doing hex encoding and decoding.

While on it, this commit uses better variable names for the source and
destination buffers in the existing escape and base64 routines in
encode.c and it makes them more robust to overflow detection.  The
previous core code issued a FATAL after doing out-of-bound writes if
going through the SQL functions, which would be enough to detect
problems when working on changes that impacted this area of the
code.  Instead, an error is issued before doing an out-of-bound write.
The hex routines were being directly called for bytea conversions and
backup manifests without such sanity checks.  The current calls happen
to not have any problems, but careless uses of such APIs could easily
lead to CVE-class bugs.

Author: Bruce Momjian, Michael Paquier
Reviewed-by: Sehrope Sarkuni
Discussion: https://postgr.es/m/[email protected]

1 files changed, 192 insertions, 0 deletions

diff --git a/src/common/hex.c b/src/common/hex.c
new file mode 100644
index 00000000000..e4878ba253d
--- /dev/null
+++ b/src/common/hex.c
@@ -0,0 +1,192 @@
+/*-------------------------------------------------------------------------
+ *
+ * hex.c
+ *	  Encoding and decoding routines for hex.
+ *
+ * Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * IDENTIFICATION
+ *	  src/common/hex.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
+
+#ifndef FRONTEND
+#include "postgres.h"
+#else
+#include "postgres_fe.h"
+#endif
+
+#include "common/hex.h"
+#ifdef FRONTEND
+#include "common/logging.h"
+#endif
+#include "mb/pg_wchar.h"
+
+
+static const int8 hexlookup[128] = {
+	-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
+	-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
+	-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
+	0, 1, 2, 3, 4, 5, 6, 7, 8, 9, -1, -1, -1, -1, -1, -1,
+	-1, 10, 11, 12, 13, 14, 15, -1, -1, -1, -1, -1, -1, -1, -1, -1,
+	-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
+	-1, 10, 11, 12, 13, 14, 15, -1, -1, -1, -1, -1, -1, -1, -1, -1,
+	-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
+};
+
+static const char hextbl[] = "0123456789abcdef";
+
+static inline char
+get_hex(const char *cp)
+{
+	unsigned char c = (unsigned char) *cp;
+	int			res = -1;
+
+	if (c < 127)
+		res = hexlookup[c];
+
+	if (res < 0)
+	{
+#ifdef FRONTEND
+		pg_log_fatal("invalid hexadecimal digit");
+		exit(EXIT_FAILURE);
+#else
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("invalid hexadecimal digit: \"%.*s\"",
+						pg_mblen(cp), cp)));
+#endif
+	}
+
+	return (char) res;
+}
+
+/*
+ * pg_hex_encode
+ *
+ * Encode into hex the given string.  Returns the length of the encoded
+ * string.
+ */
+uint64
+pg_hex_encode(const char *src, size_t srclen, char *dst, size_t dstlen)
+{
+	const char *end = src + srclen;
+	char	   *p;
+
+	p = dst;
+
+	while (src < end)
+	{
+		/*
+		 * Leave if there is an overflow in the area allocated for the encoded
+		 * string.
+		 */
+		if ((p - dst + 2) > dstlen)
+		{
+#ifdef FRONTEND
+			pg_log_fatal("overflow of destination buffer in hex encoding");
+			exit(EXIT_FAILURE);
+#else
+			elog(ERROR, "overflow of destination buffer in hex encoding");
+#endif
+		}
+
+		*p++ = hextbl[(*src >> 4) & 0xF];
+		*p++ = hextbl[*src & 0xF];
+		src++;
+	}
+
+	Assert((p - dst) <= dstlen);
+	return p - dst;
+}
+
+/*
+ * pg_hex_decode
+ *
+ * Decode the given hex string.  Returns the length of the decoded string.
+ */
+uint64
+pg_hex_decode(const char *src, size_t srclen, char *dst, size_t dstlen)
+{
+	const char *s,
+			   *srcend;
+	char		v1,
+				v2,
+			   *p;
+
+	srcend = src + srclen;
+	s = src;
+	p = dst;
+	while (s < srcend)
+	{
+		if (*s == ' ' || *s == '\n' || *s == '\t' || *s == '\r')
+		{
+			s++;
+			continue;
+		}
+		v1 = get_hex(s) << 4;
+		s++;
+
+		if (s >= srcend)
+		{
+#ifdef FRONTEND
+			pg_log_fatal("invalid hexadecimal data: odd number of digits");
+			exit(EXIT_FAILURE);
+#else
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+					 errmsg("invalid hexadecimal data: odd number of digits")));
+#endif
+		}
+
+		v2 = get_hex(s);
+		s++;
+
+		/* overflow check */
+		if ((p - dst + 1) > dstlen)
+		{
+#ifdef FRONTEND
+			pg_log_fatal("overflow of destination buffer in hex decoding");
+			exit(EXIT_FAILURE);
+#else
+			elog(ERROR, "overflow of destination buffer in hex decoding");
+#endif
+		}
+
+		*p++ = v1 | v2;
+	}
+
+	Assert((p - dst) <= dstlen);
+	return p - dst;
+}
+
+/*
+ * pg_hex_enc_len
+ *
+ * Returns to caller the length of the string if it were encoded with
+ * hex based on the length provided by caller.  This is useful to estimate
+ * how large a buffer allocation needs to be done before doing the actual
+ * encoding.
+ */
+uint64
+pg_hex_enc_len(size_t srclen)
+{
+	return (uint64) srclen << 1;
+}
+
+/*
+ * pg_hex_dec_len
+ *
+ * Returns to caller the length of the string if it were to be decoded
+ * with hex, based on the length given by caller.  This is useful to
+ * estimate how large a buffer allocation needs to be done before doing
+ * the actual decoding.
+ */
+uint64
+pg_hex_dec_len(size_t srclen)
+{
+	return (uint64) srclen >> 1;
+}