]> granicus.if.org Git - postgresql/commitdiff
Teach libpq to detect integer overflow in the row count of a PGresult.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 29 Aug 2017 19:18:01 +0000 (15:18 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 29 Aug 2017 19:18:01 +0000 (15:18 -0400)
Adding more than 1 billion rows to a PGresult would overflow its ntups and
tupArrSize fields, leading to client crashes.  It'd be desirable to use
wider fields on 64-bit machines, but because all of libpq's external APIs
use plain "int" for row counters, that's going to be hard to accomplish
without an ABI break.  Given the lack of complaints so far, and the general
pain that would be involved in using such huge PGresults, let's settle for
just preventing the overflow and reporting a useful error message if it
does happen.  Also, for a couple more lines of code we can increase the
threshold of trouble from INT_MAX/2 to INT_MAX rows.

To do that, refactor pqAddTuple() to allow returning an error message that
replaces the default assumption that it failed because of out-of-memory.

Along the way, fix PQsetvalue() so that it reports all failures via
pqInternalNotice().  It already did so in the case of bad field number,
but neglected to report anything for other error causes.

Because of the potential for crashes, this seems like a back-patchable
bug fix, despite the lack of field reports.

Michael Paquier, per a complaint from Igor Korot.

Discussion: https://postgr.es/m/CA+FnnTxyLWyjY1goewmJNxC==HQCCF4fKkoCTa9qR36oRAHDPw@mail.gmail.com

src/interfaces/libpq/fe-exec.c

index 87ff5659ff35ef63081af161e9e850cacbdfedb9..ca20dd130c30487cf107eec07e0713e8edf53e21 100644 (file)
@@ -16,6 +16,7 @@
 
 #include <ctype.h>
 #include <fcntl.h>
+#include <limits.h>
 
 #include "libpq-fe.h"
 #include "libpq-int.h"
@@ -51,7 +52,8 @@ static bool static_std_strings = false;
 
 
 static PGEvent *dupEvents(PGEvent *events, int count);
-static bool pqAddTuple(PGresult *res, PGresAttValue *tup);
+static bool pqAddTuple(PGresult *res, PGresAttValue *tup,
+                  const char **errmsgp);
 static bool PQsendQueryStart(PGconn *conn);
 static int PQsendQueryGuts(PGconn *conn,
                                const char *command,
@@ -416,18 +418,26 @@ dupEvents(PGEvent *events, int count)
  * equal to PQntuples(res).  If it is equal, a new tuple is created and
  * added to the result.
  * Returns a non-zero value for success and zero for failure.
+ * (On failure, we report the specific problem via pqInternalNotice.)
  */
 int
 PQsetvalue(PGresult *res, int tup_num, int field_num, char *value, int len)
 {
        PGresAttValue *attval;
+       const char *errmsg = NULL;
 
+       /* Note that this check also protects us against null "res" */
        if (!check_field_number(res, field_num))
                return FALSE;
 
        /* Invalid tup_num, must be <= ntups */
        if (tup_num < 0 || tup_num > res->ntups)
+       {
+               pqInternalNotice(&res->noticeHooks,
+                                                "row number %d is out of range 0..%d",
+                                                tup_num, res->ntups);
                return FALSE;
+       }
 
        /* need to allocate a new tuple? */
        if (tup_num == res->ntups)
@@ -440,7 +450,7 @@ PQsetvalue(PGresult *res, int tup_num, int field_num, char *value, int len)
                                                  TRUE);
 
                if (!tup)
-                       return FALSE;
+                       goto fail;
 
                /* initialize each column to NULL */
                for (i = 0; i < res->numAttributes; i++)
@@ -450,8 +460,8 @@ PQsetvalue(PGresult *res, int tup_num, int field_num, char *value, int len)
                }
 
                /* add it to the array */
-               if (!pqAddTuple(res, tup))
-                       return FALSE;
+               if (!pqAddTuple(res, tup, &errmsg))
+                       goto fail;
        }
 
        attval = &res->tuples[tup_num][field_num];
@@ -471,13 +481,24 @@ PQsetvalue(PGresult *res, int tup_num, int field_num, char *value, int len)
        {
                attval->value = (char *) pqResultAlloc(res, len + 1, TRUE);
                if (!attval->value)
-                       return FALSE;
+                       goto fail;
                attval->len = len;
                memcpy(attval->value, value, len);
                attval->value[len] = '\0';
        }
 
        return TRUE;
+
+       /*
+        * Report failure via pqInternalNotice.  If preceding code didn't provide
+        * an error message, assume "out of memory" was meant.
+        */
+fail:
+       if (!errmsg)
+               errmsg = libpq_gettext("out of memory");
+       pqInternalNotice(&res->noticeHooks, "%s", errmsg);
+
+       return FALSE;
 }
 
 /*
@@ -847,10 +868,13 @@ pqInternalNotice(const PGNoticeHooks *hooks, const char *fmt,...)
 /*
  * pqAddTuple
  *       add a row pointer to the PGresult structure, growing it if necessary
- *       Returns TRUE if OK, FALSE if not enough memory to add the row
+ *       Returns TRUE if OK, FALSE if an error prevented adding the row
+ *
+ * On error, *errmsgp can be set to an error string to be returned.
+ * If it is left NULL, the error is presumed to be "out of memory".
  */
 static bool
-pqAddTuple(PGresult *res, PGresAttValue *tup)
+pqAddTuple(PGresult *res, PGresAttValue *tup, const char **errmsgp)
 {
        if (res->ntups >= res->tupArrSize)
        {
@@ -865,9 +889,36 @@ pqAddTuple(PGresult *res, PGresAttValue *tup)
                 * existing allocation. Note that the positions beyond res->ntups are
                 * garbage, not necessarily NULL.
                 */
-               int                     newSize = (res->tupArrSize > 0) ? res->tupArrSize * 2 : 128;
+               int                     newSize;
                PGresAttValue **newTuples;
 
+               /*
+                * Since we use integers for row numbers, we can't support more than
+                * INT_MAX rows.  Make sure we allow that many, though.
+                */
+               if (res->tupArrSize <= INT_MAX / 2)
+                       newSize = (res->tupArrSize > 0) ? res->tupArrSize * 2 : 128;
+               else if (res->tupArrSize < INT_MAX)
+                       newSize = INT_MAX;
+               else
+               {
+                       *errmsgp = libpq_gettext("PGresult cannot support more than INT_MAX tuples");
+                       return FALSE;
+               }
+
+               /*
+                * Also, on 32-bit platforms we could, in theory, overflow size_t even
+                * before newSize gets to INT_MAX.  (In practice we'd doubtless hit
+                * OOM long before that, but let's check.)
+                */
+#if INT_MAX >= (SIZE_MAX / 2)
+               if (newSize > SIZE_MAX / sizeof(PGresAttValue *))
+               {
+                       *errmsgp = libpq_gettext("size_t overflow");
+                       return FALSE;
+               }
+#endif
+
                if (res->tuples == NULL)
                        newTuples = (PGresAttValue **)
                                malloc(newSize * sizeof(PGresAttValue *));
@@ -1093,7 +1144,7 @@ pqRowProcessor(PGconn *conn, const char **errmsgp)
        }
 
        /* And add the tuple to the PGresult's tuple array */
-       if (!pqAddTuple(res, tup))
+       if (!pqAddTuple(res, tup, errmsgp))
                goto fail;
 
        /*