postgres_fdw: Replace buffers in RemoteAttributeMapping with pointers.

Commit 28972b6fc ("Add support for importing statistics from remote
servers.") stored the names of local/remote columns for a foreign table
into the buffers of NAMEDATALEN bytes in this structure, without
accounting for the possibility that the remote column name in particular
could be longer than NAMEDATALEN - 1.  If it was longer than that, this
would leave it unterminated/truncated in the buffer, invoking undefined
behavior when match_attrmap() processes it, which assumes that it's
fully-contained/terminated in the buffer.

To fix, replace the buffers with char pointers, pstrdup the local/remote
column names, and store the results into the pointers.  This commit also
adds a function to clean up the nested data structure.

Per Coverity and Tom Lane.

Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Author: Corey Huinker <corey.huinker@gmail.com>
Reviewed-by: Etsuro Fujita <etsuro.fujita@gmail.com>
Discussion: https://postgr.es/m/342868.1776017700%40sss.pgh.pa.us

diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 85be46eb2a2de793e0fd91e6a1191df3a3375f14..0a589f8db74a6108ca567cb6e19ca202d3305e28 100644 (file)
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -325,8 +325,8 @@ typedef struct
 typedef struct
 {
        AttrNumber      local_attnum;
-       char            local_attname[NAMEDATALEN];
-       char            remote_attname[NAMEDATALEN];
+       char       *local_attname;
+       char       *remote_attname;
        int                     res_index;
 } RemoteAttributeMapping;
 
@@ -704,6 +704,7 @@ static PGresult *fetch_attstats(PGconn *conn, int server_version_num,
                                                                const char *column_list);
 static RemoteAttributeMapping *build_remattrmap(Relation relation, List *va_cols,
                                                                                                int *p_attrcnt, StringInfo column_list);
+static void free_remattrmap(RemoteAttributeMapping *map, int len);
 static bool attname_in_list(const char *attname, List *va_cols);
 static int     remattrmap_cmp(const void *v1, const void *v2);
 static bool match_attrmap(PGresult *res,
@@ -5670,8 +5671,7 @@ postgresImportForeignStatistics(Relation relation, List *va_cols, int elevel)
 
        PQclear(remstats.rel);
        PQclear(remstats.att);
-       if (remattrmap)
-               pfree(remattrmap);
+       free_remattrmap(remattrmap, attrcnt);
 
        return ok;
 }
@@ -5957,8 +5957,8 @@ build_remattrmap(Relation relation, List *va_cols,
                deparseStringLiteral(column_list, remote_attname);
 
                remattrmap[attrcnt].local_attnum = attnum;
-               strncpy(remattrmap[attrcnt].local_attname, attname, NAMEDATALEN);
-               strncpy(remattrmap[attrcnt].remote_attname, remote_attname, NAMEDATALEN);
+               remattrmap[attrcnt].local_attname = pstrdup(attname);
+               remattrmap[attrcnt].remote_attname = pstrdup(remote_attname);
                remattrmap[attrcnt].res_index = -1;
                attrcnt++;
        }
@@ -5972,6 +5972,26 @@ build_remattrmap(Relation relation, List *va_cols,
        return remattrmap;
 }
 
+/*
+ * Free the structure created by build_remattrmap().
+ */
+static void
+free_remattrmap(RemoteAttributeMapping *map, int len)
+{
+       if (!map)
+               return;
+
+       for (int i = 0; i < len; i++)
+       {
+               Assert(map[i].local_attname);
+               pfree(map[i].local_attname);
+               Assert(map[i].remote_attname);
+               pfree(map[i].remote_attname);
+       }
+
+       pfree(map);
+}
+
 /*
  * Test if an attribute name is in the list.
  *