Fix possible buffer overrun in \d command: substr(..., 128) produces

tglsfdc · tglsfdc · commit f1f597860351 · 2002-12-21T01:07:07.000Z
a result of at most 128 chars, but that could be more than 128 bytes.
Also ensure we don't try to pfree uninitialized pointers during error
cleanup.
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
@@ -3,7 +3,7 @@
  *
  * Copyright 2000-2002 by PostgreSQL Global Development Group
  *
- * $Header: /cvsroot/pgsql/src/bin/psql/describe.c,v 1.72 2002/12/12 21:02:24 momjian Exp $
+ * $Header: /cvsroot/pgsql/src/bin/psql/describe.c,v 1.73 2002/12/21 01:07:07 tgl Exp $
  */
 #include "postgres_fe.h"
 #include "describe.h"
@@ -44,6 +44,16 @@ xmalloc(size_t size)
 	return tmp;
 }
 
+static void *
+xmalloczero(size_t size)
+{
+	void	   *tmp;
+
+	tmp = xmalloc(size);
+	memset(tmp, 0, size);
+	return tmp;
+}
+
 
 /*----------------
  * Handlers for various slash commands displaying some sort of list
@@ -635,7 +645,9 @@ describeOneTableDetails(const char *schemaname,
 	char	  **footers = NULL;
 	char	  **ptr;
 	PQExpBufferData title;
-	unsigned int cols = 0;
+	PQExpBufferData tmpbuf;
+	int			cols = 0;
+	int			numrows = 0;
 	struct
 	{
 		bool		hasindex;
@@ -644,12 +656,14 @@ describeOneTableDetails(const char *schemaname,
 		int16		triggers;
 		bool		hasrules;
 	}			tableinfo;
+	bool		show_modifiers = false;
 	bool		retval;
 
 	retval = false;
 
 	initPQExpBuffer(&buf);
 	initPQExpBuffer(&title);
+	initPQExpBuffer(&tmpbuf);
 
 	/* Get general table info */
 	printfPQExpBuffer(&buf,
@@ -683,6 +697,7 @@ describeOneTableDetails(const char *schemaname,
 
 	if (tableinfo.relkind == 'r' || tableinfo.relkind == 'v')
 	{
+		show_modifiers = true;
 		cols++;
 		headers[cols - 1] = _("Modifiers");
 	}
@@ -715,6 +730,7 @@ describeOneTableDetails(const char *schemaname,
 	res = PSQLexec(buf.data, false);
 	if (!res)
 		goto error_return;
+	numrows = PQntuples(res);
 
 	/* Check if table is a view */
 	if (tableinfo.relkind == 'v')
@@ -733,10 +749,10 @@ describeOneTableDetails(const char *schemaname,
 	}
 
 	/* Generate table cells to be printed */
-	cells = xmalloc((PQntuples(res) * cols + 1) * sizeof(*cells));
-	cells[PQntuples(res) * cols] = NULL;		/* end of list */
+	/* note: initialize all cells[] to NULL in case of error exit */
+	cells = xmalloczero((numrows * cols + 1) * sizeof(*cells));
 
-	for (i = 0; i < PQntuples(res); i++)
+	for (i = 0; i < numrows; i++)
 	{
 		/* Name */
 		cells[i * cols + 0] = PQgetvalue(res, i, 0);	/* don't free this
@@ -747,12 +763,11 @@ describeOneTableDetails(const char *schemaname,
 
 		/* Extra: not null and default */
 		/* (I'm cutting off the 'default' string at 128) */
-		if (tableinfo.relkind == 'r' || tableinfo.relkind == 'v')
+		if (show_modifiers)
 		{
-			cells[i * cols + 2] = xmalloc(128 + 128);
-			cells[i * cols + 2][0] = '\0';
+			resetPQExpBuffer(&tmpbuf);
 			if (strcmp(PQgetvalue(res, i, 2), "t") == 0)
-				strcat(cells[i * cols + 2], "not null");
+				appendPQExpBufferStr(&tmpbuf, "not null");
 
 			/* handle "default" here */
 			if (strcmp(PQgetvalue(res, i, 3), "t") == 0)
@@ -761,18 +776,21 @@ describeOneTableDetails(const char *schemaname,
 
 				printfPQExpBuffer(&buf,
 								  "SELECT substring(d.adsrc for 128) FROM pg_catalog.pg_attrdef d\n"
-							   "WHERE d.adrelid = '%s' AND d.adnum = %s",
+								  "WHERE d.adrelid = '%s' AND d.adnum = %s",
 								  oid, PQgetvalue(res, i, 4));
 
 				result = PSQLexec(buf.data, false);
 
-				if (cells[i * cols + 2][0])
-					strcat(cells[i * cols + 2], " ");
-				strcat(cells[i * cols + 2], "default ");
-				strcat(cells[i * cols + 2], result ? PQgetvalue(result, 0, 0) : "?");
+				if (tmpbuf.len > 0)
+					appendPQExpBufferStr(&tmpbuf, " ");
+
+				appendPQExpBuffer(&tmpbuf, "default %s",
+								  result ? PQgetvalue(result, 0, 0) : "?");
 
 				PQclear(result);
 			}
+
+			cells[i * cols + 2] = xstrdup(tmpbuf.data);
 		}
 
 		/* Description */
@@ -841,15 +859,12 @@ describeOneTableDetails(const char *schemaname,
 		}
 		else
 		{
-			PQExpBufferData tmpbuf;
 			char	   *indisunique = PQgetvalue(result, 0, 0);
 			char	   *indisprimary = PQgetvalue(result, 0, 1);
 			char	   *indamname = PQgetvalue(result, 0, 2);
 			char	   *indtable = PQgetvalue(result, 0, 3);
 			char	   *indpred = PQgetvalue(result, 0, 4);
 
-			initPQExpBuffer(&tmpbuf);
-
 			if (strcmp(indisprimary, "t") == 0)
 				printfPQExpBuffer(&tmpbuf, _("primary key, "));
 			else if (strcmp(indisunique, "t") == 0)
@@ -865,10 +880,9 @@ describeOneTableDetails(const char *schemaname,
 			if (strlen(indpred))
 				appendPQExpBuffer(&tmpbuf, ", predicate %s", indpred);
 
-			footers = xmalloc(2 * sizeof(*footers));
+			footers = xmalloczero(2 * sizeof(*footers));
 			footers[0] = xstrdup(tmpbuf.data);
 			footers[1] = NULL;
-			termPQExpBuffer(&tmpbuf);
 		}
 
 		PQclear(result);
@@ -895,7 +909,7 @@ describeOneTableDetails(const char *schemaname,
 		}
 
 		/* Footer information about a view */
-		footers = xmalloc((rule_count + 2) * sizeof(*footers));
+		footers = xmalloczero((rule_count + 2) * sizeof(*footers));
 		footers[count_footers] = xmalloc(64 + strlen(view_def));
 		snprintf(footers[count_footers], 64 + strlen(view_def),
 				 _("View definition: %s"), view_def);
@@ -1018,8 +1032,8 @@ describeOneTableDetails(const char *schemaname,
 				foreignkey_count = PQntuples(result5);
 		}
 
-		footers = xmalloc((index_count + check_count + rule_count + trigger_count + foreignkey_count + 1)
-						  * sizeof(*footers));
+		footers = xmalloczero((index_count + check_count + rule_count + trigger_count + foreignkey_count + 1)
+							  * sizeof(*footers));
 
 		/* print indexes */
 		for (i = 0; i < index_count; i++)
@@ -1148,12 +1162,15 @@ describeOneTableDetails(const char *schemaname,
 	/* clean up */
 	termPQExpBuffer(&buf);
 	termPQExpBuffer(&title);
+	termPQExpBuffer(&tmpbuf);
 
 	if (cells)
 	{
-		for (i = 0; i < PQntuples(res); i++)
-			if (tableinfo.relkind == 'r' || tableinfo.relkind == 'v')
+		for (i = 0; i < numrows; i++)
+		{
+			if (show_modifiers)
 				free(cells[i * cols + 2]);
+		}
 		free(cells);
 	}
 
