Tweak libpq to avoid crashing due to incorrect buffer size calculation when
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 29 May 2008 22:02:44 +0000 (22:02 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 29 May 2008 22:02:44 +0000 (22:02 +0000)
we are on a 64-bit machine (ie, size_t is wider than int) and someone passes
in a query string that approaches or exceeds INT_MAX bytes.  Also, just for
paranoia's sake, guard against similar overflows in sizing the input buffer.

The backend will not in the foreseeable future be prepared to send or receive
strings exceeding 1GB, so I didn't take the more invasive step of switching
all the buffer index variables from int to size_t; though someday we might
want to do that.

I have a suspicion that this is not the only such bug in libpq, but this
fix is enough to take care of the crash reported by Francisco Reyes.

src/interfaces/libpq/fe-connect.c
src/interfaces/libpq/fe-exec.c
src/interfaces/libpq/fe-misc.c
src/interfaces/libpq/fe-protocol3.c
src/interfaces/libpq/libpq-int.h

index ec9a9a570a5987bcd4ba80d997d6f400a5ccb20c..5e687c15585fca08c01ab7452503aab500f44ad6 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/interfaces/libpq/fe-connect.c,v 1.358 2008/05/16 18:30:53 mha Exp $
+ *   $PostgreSQL: pgsql/src/interfaces/libpq/fe-connect.c,v 1.359 2008/05/29 22:02:44 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1581,7 +1581,8 @@ keep_going:                       /* We will come back to here until there is
                     * needed to hold the whole message; see notes in
                     * pqParseInput3.
                     */
-                   if (pqCheckInBufferSpace(conn->inCursor + msgLength, conn))
+                   if (pqCheckInBufferSpace(conn->inCursor + (size_t) msgLength,
+                                            conn))
                        goto error_return;
                    /* We'll come back when there is more data */
                    return PGRES_POLLING_READING;
index e7941dffe024f16e5ed554018506e2c7ca1b86c7..25ad0e4a1a6fb31e6d4b93ec11f992d6882cbd28 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/interfaces/libpq/fe-exec.c,v 1.194 2008/01/01 19:46:00 momjian Exp $
+ *   $PostgreSQL: pgsql/src/interfaces/libpq/fe-exec.c,v 1.195 2008/05/29 22:02:44 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1685,7 +1685,8 @@ PQputCopyData(PGconn *conn, const char *buffer, int nbytes)
        {
            if (pqFlush(conn) < 0)
                return -1;
-           if (pqCheckOutBufferSpace(conn->outCount + 5 + nbytes, conn))
+           if (pqCheckOutBufferSpace(conn->outCount + 5 + (size_t) nbytes,
+                                     conn))
                return pqIsnonblocking(conn) ? 0 : -1;
        }
        /* Send the data (too simple to delegate to fe-protocol files) */
index 3d06386542713d52325f23ccc02247228c6b85a6..33c704b5190f9a1e9de835c9525257aedb354c61 100644 (file)
@@ -23,7 +23,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/interfaces/libpq/fe-misc.c,v 1.133 2008/01/01 19:46:00 momjian Exp $
+ *   $PostgreSQL: pgsql/src/interfaces/libpq/fe-misc.c,v 1.134 2008/05/29 22:02:44 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -278,12 +278,12 @@ pqPutInt(int value, size_t bytes, PGconn *conn)
  * Returns 0 on success, EOF if failed to enlarge buffer
  */
 int
-pqCheckOutBufferSpace(int bytes_needed, PGconn *conn)
+pqCheckOutBufferSpace(size_t bytes_needed, PGconn *conn)
 {
    int         newsize = conn->outBufSize;
    char       *newbuf;
 
-   if (bytes_needed <= newsize)
+   if (bytes_needed <= (size_t) newsize)
        return 0;
 
    /*
@@ -296,9 +296,9 @@ pqCheckOutBufferSpace(int bytes_needed, PGconn *conn)
    do
    {
        newsize *= 2;
-   } while (bytes_needed > newsize && newsize > 0);
+   } while (newsize > 0 && bytes_needed > (size_t) newsize);
 
-   if (bytes_needed <= newsize)
+   if (newsize > 0 && bytes_needed <= (size_t) newsize)
    {
        newbuf = realloc(conn->outBuffer, newsize);
        if (newbuf)
@@ -314,9 +314,9 @@ pqCheckOutBufferSpace(int bytes_needed, PGconn *conn)
    do
    {
        newsize += 8192;
-   } while (bytes_needed > newsize && newsize > 0);
+   } while (newsize > 0 && bytes_needed > (size_t) newsize);
 
-   if (bytes_needed <= newsize)
+   if (newsize > 0 && bytes_needed <= (size_t) newsize)
    {
        newbuf = realloc(conn->outBuffer, newsize);
        if (newbuf)
@@ -341,12 +341,12 @@ pqCheckOutBufferSpace(int bytes_needed, PGconn *conn)
  * Returns 0 on success, EOF if failed to enlarge buffer
  */
 int
-pqCheckInBufferSpace(int bytes_needed, PGconn *conn)
+pqCheckInBufferSpace(size_t bytes_needed, PGconn *conn)
 {
    int         newsize = conn->inBufSize;
    char       *newbuf;
 
-   if (bytes_needed <= newsize)
+   if (bytes_needed <= (size_t) newsize)
        return 0;
 
    /*
@@ -359,9 +359,9 @@ pqCheckInBufferSpace(int bytes_needed, PGconn *conn)
    do
    {
        newsize *= 2;
-   } while (bytes_needed > newsize && newsize > 0);
+   } while (newsize > 0 && bytes_needed > (size_t) newsize);
 
-   if (bytes_needed <= newsize)
+   if (newsize > 0 && bytes_needed <= (size_t) newsize)
    {
        newbuf = realloc(conn->inBuffer, newsize);
        if (newbuf)
@@ -377,9 +377,9 @@ pqCheckInBufferSpace(int bytes_needed, PGconn *conn)
    do
    {
        newsize += 8192;
-   } while (bytes_needed > newsize && newsize > 0);
+   } while (newsize > 0 && bytes_needed > (size_t) newsize);
 
-   if (bytes_needed <= newsize)
+   if (newsize > 0 && bytes_needed <= (size_t) newsize)
    {
        newbuf = realloc(conn->inBuffer, newsize);
        if (newbuf)
@@ -572,7 +572,7 @@ pqReadData(PGconn *conn)
     */
    if (conn->inBufSize - conn->inEnd < 8192)
    {
-       if (pqCheckInBufferSpace(conn->inEnd + 8192, conn))
+       if (pqCheckInBufferSpace(conn->inEnd + (size_t) 8192, conn))
        {
            /*
             * We don't insist that the enlarge worked, but we need some room
index 89c0d5018f00d40f66d81ba2d617097b8b8c5bec..66059493bb0788a475c1243d7b05c8d58aa259de 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/interfaces/libpq/fe-protocol3.c,v 1.34 2008/01/17 21:21:50 tgl Exp $
+ *   $PostgreSQL: pgsql/src/interfaces/libpq/fe-protocol3.c,v 1.35 2008/05/29 22:02:44 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -115,7 +115,8 @@ pqParseInput3(PGconn *conn)
             * recovery strategy if we are unable to make the buffer big
             * enough.
             */
-           if (pqCheckInBufferSpace(conn->inCursor + msgLength, conn))
+           if (pqCheckInBufferSpace(conn->inCursor + (size_t) msgLength,
+                                    conn))
            {
                /*
                 * XXX add some better recovery code... plan is to skip over
@@ -1310,7 +1311,8 @@ getCopyDataMessage(PGconn *conn)
             * Before returning, enlarge the input buffer if needed to hold
             * the whole message.  See notes in parseInput.
             */
-           if (pqCheckInBufferSpace(conn->inCursor + msgLength - 4, conn))
+           if (pqCheckInBufferSpace(conn->inCursor + (size_t) msgLength - 4,
+                                    conn))
            {
                /*
                 * XXX add some better recovery code... plan is to skip over
@@ -1745,7 +1747,8 @@ pqFunctionCall3(PGconn *conn, Oid fnid,
             * Before looping, enlarge the input buffer if needed to hold the
             * whole message.  See notes in parseInput.
             */
-           if (pqCheckInBufferSpace(conn->inCursor + msgLength, conn))
+           if (pqCheckInBufferSpace(conn->inCursor + (size_t) msgLength,
+                                    conn))
            {
                /*
                 * XXX add some better recovery code... plan is to skip over
index 89b37473506cc256d74013603443be27f827456c..fd94952f1802e2f294a50c73801b7f162d9f1d9e 100644 (file)
@@ -12,7 +12,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/interfaces/libpq/libpq-int.h,v 1.130 2008/05/16 18:30:53 mha Exp $
+ * $PostgreSQL: pgsql/src/interfaces/libpq/libpq-int.h,v 1.131 2008/05/29 22:02:44 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -511,8 +511,8 @@ extern PGresult *pqFunctionCall3(PGconn *conn, Oid fnid,
   * Get, EOF merely means the buffer is exhausted, not that there is
   * necessarily any error.
   */
-extern int pqCheckOutBufferSpace(int bytes_needed, PGconn *conn);
-extern int pqCheckInBufferSpace(int bytes_needed, PGconn *conn);
+extern int pqCheckOutBufferSpace(size_t bytes_needed, PGconn *conn);
+extern int pqCheckInBufferSpace(size_t bytes_needed, PGconn *conn);
 extern int pqGetc(char *result, PGconn *conn);
 extern int pqPutc(char c, PGconn *conn);
 extern int pqGets(PQExpBuffer buf, PGconn *conn);