On 11/11/22 07:13, Peter Eisentraut wrote:
> On 09.11.22 00:08, Jacob Champion wrote:
>> pqGetNegotiateProtocolVersion3() is still ignoring the message length,
>> though; it won't necessarily stop at the message boundary.
>
> I don't follow. The calls to pqGetInt(), pqGets(), etc. check the
> message length.
I may be missing something obvious, but I don't see any message length
checks in those functions, just bounds checks on the connection buffer.
> Do you have something else in mind? Can you give an
> example or existing code?
Sure. Consider the case where the server sends a
NegotiateProtocolVersion with a reasonable length, but then runs over
its own message (either by sending an unterminated string as one of the
extension names, or by sending a huge extension number). When I test
that against a client on my machine, it churns CPU and memory waiting
for the end of a message that will never come, even though it had
already decided that the maximum length of the message should have been
less than 2K.
Put another way, why do we loop around and poll for more data when we
hit the end of the connection buffer, if we've already checked at this
point that we should have the entire message buffered locally?
>+ initPQExpBuffer(&buf);
>+ if (pqGetInt(&tmp, 4, conn) != 0)
>+ return EOF;
Tangentially related -- I think the temporary PQExpBuffer is being
leaked in the EOF case.
--Jacob