Re: [PATCH] Fix unbounded authentication exchanges during PQconnectPoll()

On 22/02/2023 20:49, Jacob Champion wrote:
> On Tue, Feb 21, 2023 at 12:35 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>>> @@ -3370,6 +3389,7 @@ keep_going:                                               /* We will come back to here until
thereis
 
>>>                                  /* Get the type of request. */
>>>                                  if (pqGetInt((int *) &areq, 4, conn))
>>>                                  {
>>> +                                       libpq_append_conn_error(conn, "server sent truncated authentication
request");
>>>                                          goto error_return;
>>>                                  }
>>>                                  msgLength -= 4;
>>
>> This is unreachable, because we already checked the length. Better safe
>> than sorry I guess, but let's avoid the translation overhead of this at
>> least.
> 
> Should we just Assert() instead of an error message?

I separated the earlier message-length checks so that you get "invalid 
invalid authentication request" or "received invalid protocol 
negotiation message", depending on whether it was an 'R' or 'v' message. 
With that, "invalid invalid authentication request" becomes translatable 
anyway, which makes the point on translation overhead moot. I added a 
comment to mention that it's unreachable, though.

I also reformatted the comments a little more.

Pushed with those changes, thanks!

- Heikki