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

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
index f5a2c895eeb38cb0a9e5272f191dd6d0a3f964de..d4f7e1ededd2f065b42059451faeb91ddcba176c 100644 (file)
--- 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 @@ error_return:
        /* 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);
        }