Check for unbounded authentication exchanges in libpq.

A couple of code paths in CONNECTION_AWAITING_RESPONSE will eagerly read
bytes off a connection that should be closed. Don't let a misbehaving
server chew up client resources here; a v2 error can't be infinitely
long, and a v3 error should be bounded by its original message length.

For the existing error_return cases, I added some additional error
messages for symmetry with the new ones, and cleaned up some message
rot.

Author: Jacob Champion
Discussion: https://www.postgresql.org/message-id/8e729daf-7d71-6965-9687-8bc0630599b3%40timescale.com

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 50b5df3490bfa77ef21e3a6aec56ca562b8f3995..8f80c35c89448f936a108b72fe00b31127f07365 100644 (file)
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -3214,28 +3214,46 @@ keep_going:                                             /* We will come back to here until there is
 
                                /*
                                 * Try to validate message length before using it.
+                                *
                                 * Authentication requests can't be very large, although GSS
                                 * auth requests may not be that small.  Same for
-                                * NegotiateProtocolVersion.  Errors can be a
-                                * little larger, but not huge.  If we see a large apparent
-                                * length in an error, it means we're really talking to a
-                                * pre-3.0-protocol server; cope.  (Before version 14, the
-                                * server also used the old protocol for errors that happened
-                                * before processing the startup packet.)
+                                * NegotiateProtocolVersion.
+                                *
+                                * Errors can be a little larger, but not huge.  If we see a
+                                * large apparent length in an error, it means we're really
+                                * talking to a pre-3.0-protocol server; cope.  (Before
+                                * version 14, the server also used the old protocol for
+                                * errors that happened before processing the startup packet.)
                                 */
-                               if ((beresp == 'R' || beresp == 'v') && (msgLength < 8 || msgLength > 2000))
+                               if (beresp == 'R' && (msgLength < 8 || msgLength > 2000))
                                {
-                                       libpq_append_conn_error(conn, "expected authentication request from server, but received %c",
-                                                                          beresp);
+                                       libpq_append_conn_error(conn, "received invalid authentication request");
+                                       goto error_return;
+                               }
+                               if (beresp == 'v' && (msgLength < 8 || msgLength > 2000))
+                               {
+                                       libpq_append_conn_error(conn, "received invalid protocol negotiation message");
                                        goto error_return;
                                }
 
-                               if (beresp == 'E' && (msgLength < 8 || msgLength > 30000))
+#define MAX_ERRLEN 30000
+                               if (beresp == 'E' && (msgLength < 8 || msgLength > MAX_ERRLEN))
                                {
                                        /* Handle error from a pre-3.0 server */
                                        conn->inCursor = conn->inStart + 1; /* reread data */
                                        if (pqGets_append(&conn->errorMessage, conn))
                                        {
+                                               /*
+                                                * We may not have authenticated the server yet, so
+                                                * don't let the buffer grow forever.
+                                                */
+                                               avail = conn->inEnd - conn->inCursor;
+                                               if (avail > MAX_ERRLEN)
+                                               {
+                                                       libpq_append_conn_error(conn, "received invalid error message");
+                                                       goto error_return;
+                                               }
+
                                                /* We'll come back when there is more data */
                                                return PGRES_POLLING_READING;
                                        }
@@ -3255,9 +3273,15 @@ keep_going:                                              /* We will come back to here until there is
 
                                        goto error_return;
                                }
+#undef MAX_ERRLEN
 
                                /*
                                 * Can't process if message body isn't all here yet.
+                                *
+                                * After this check passes, any further EOF during parsing
+                                * implies that the server sent a bad/truncated message.
+                                * Reading more bytes won't help in that case, so don't return
+                                * PGRES_POLLING_READING after this point.
                                 */
                                msgLength -= 4;
                                avail = conn->inEnd - conn->inCursor;
@@ -3280,8 +3304,8 @@ keep_going:                                               /* We will come back to here until there is
                                {
                                        if (pqGetErrorNotice3(conn, true))
                                        {
-                                               /* We'll come back when there is more data */
-                                               return PGRES_POLLING_READING;
+                                               libpq_append_conn_error(conn, "received invalid error message");
+                                               goto error_return;
                                        }
                                        /* OK, we read the message; mark data consumed */
                                        conn->inStart = conn->inCursor;
@@ -3357,6 +3381,7 @@ keep_going:                                               /* We will come back to here until there is
                                {
                                        if (pqGetNegotiateProtocolVersion3(conn))
                                        {
+                                               libpq_append_conn_error(conn, "received invalid protocol negotiation message");
                                                goto error_return;
                                        }
                                        /* OK, we read the message; mark data consumed */
@@ -3370,6 +3395,8 @@ keep_going:                                               /* We will come back to here until there is
                                /* Get the type of request. */
                                if (pqGetInt((int *) &areq, 4, conn))
                                {
+                                       /* can't happen because we checked the length already */
+                                       libpq_append_conn_error(conn, "received invalid authentication request");
                                        goto error_return;
                                }
                                msgLength -= 4;