Use dynamic buffer to parse NODE_LIST_RESULT in GTM

When processing NODE_LIST_RESULT messages, gtmpqParseSuccess() used
a static buffer, defined as "char buf[8092]".  This is an issue, as
the message has variable length, and may get long enough to exceed
any hard-coded limit.  While that's not very common (it requires
long paths, node names and/or many GTM sessions on the node), it
may happen, in which case the memcpy() causes a buffer overflow and
corrupts the stack.

Fixing this is simple - allocate the buffer using malloc() intead,
requesting exactly the right amount of memory.  This however hits
a latent pre-existing issue in the code, because the code was doing
memcpy(&buf,...) instead of memcpy(buf,...).  With static buffers
this was harmless, because (buf == &buf), so the code was working
as intended (except when there were more than 8092 bytes).  With
dynamic memory this is no longer true, becase (buf != &buf), and
the stack corruption was much easier to trigger (just 8 bytes).

Per report and debug info by Hengbing. Patch by Pavan and me.

diff --git a/src/gtm/client/fe-protocol.c b/src/gtm/client/fe-protocol.c
index b52249bcea521bf03ddda2d10a888a8bc352c645..d3a80d987588ad4d413eb8bbebd2b79698bc8413 100644 (file)
--- a/src/gtm/client/fe-protocol.c
+++ b/src/gtm/client/fe-protocol.c
@@ -744,34 +744,50 @@ gtmpqParseSuccess(GTM_Conn *conn, GTM_Result *result)
                        for (i = 0 ; i < result->gr_resdata.grd_node_list.num_node; i++)
                        {
                                int size;
-                               char buf[8092];
+                               char *buf;
                                GTM_PGXCNodeInfo *data = (GTM_PGXCNodeInfo *)malloc(sizeof(GTM_PGXCNodeInfo));
 
-                               if (gtmpqGetInt(&size, sizeof(int32), conn))
+                               if (data == NULL)
                                {
                                        result->gr_status = GTM_RESULT_ERROR;
+                                       printfGTMPQExpBuffer(&conn->errorMessage, "Out of memory");
                                        break;
                                }
-                               if (size > 8092)
+
+                               if (gtmpqGetInt(&size, sizeof(int32), conn))
                                {
                                        result->gr_status = GTM_RESULT_ERROR;
-                                       printfGTMPQExpBuffer(&conn->errorMessage, "buffer size not large enough for node list data");
+                                       free(data);
+                                       break;
+                               }
+
+                               buf = (char *) malloc(size);
+                               if (buf == NULL)
+                               {
                                        result->gr_status = GTM_RESULT_ERROR;
+                                       printfGTMPQExpBuffer(&conn->errorMessage, "Out of memory");
+                                       free(data);
+                                       break;
                                }
 
-                               if (gtmpqGetnchar((char *)&buf, size, conn))
+                               if (gtmpqGetnchar(buf, size, conn))
                                {
                                        result->gr_status = GTM_RESULT_ERROR;
+                                       free(buf);
+                                       free(data);
                                        break;
                                }
                                if (!gtm_deserialize_pgxcnodeinfo(data, buf, size, &conn->errorMessage))
                                {
                                        result->gr_status = GTM_RESULT_ERROR;
+                                       free(buf);
+                                       free(data);
                                        break;
                                } 
                                else 
                                {
                                        result->gr_resdata.grd_node_list.nodeinfo[i] = data;
+                                       free(buf);
                                } 
                        }