Re: minor auth code cleanup

Neil Conway <neilc@samurai.com> writes:

> !     /*
> !      * We don't actually use the startup packet length the frontend sent
> !      * us; however, it's a reasonable sanity check to ensure that we
> !      * read as much data as we expected to.
> !      *
> !      * The actual startup packet size is the length of the buffer, plus
> !      * the size part of the message (4 bytes), plus a terminator.
> !      */
> !     Assert(len == (buf.len + 4 + 1));

This takes a non-problem and converts it into a problem, no?

There may be existing clients out there that miscompute the password
packet length.  Right now that does no harm.  With an Assert in place
in the backend, it will cause a database system restart.

Sir Mordred would be quite justified in labeling this a DOS
vulnerability...

On the pqcomm.h comment changes, I would like to see the options field
be variable-length too, with a fairly high upper limit since you might
want to fit several constructs like "-c guc_variable_name=value" in
there.  While at it we may as well get rid of the tty field, which is
unused since a long time.

On the subject of the timeout calculations, this code still looks
utterly bizarre:

> !             if (0 > (finish_time.tv_usec -= start_time.tv_usec))
> !             {
> !                 remains.tv_sec++;
> !                 finish_time.tv_usec += 1000000;
> !             }
> !             if (0 > (remains.tv_usec -= finish_time.tv_usec))
> !             {
> !                 remains.tv_sec--;
> !                 remains.tv_usec += 1000000;
> !             }
> !             remains.tv_sec -= finish_time.tv_sec - start_time.tv_sec;

It might be correct, I'm not sure; it's definitely going out of its way
to be confusing.  A more serious objection is that the code is actively
wrong on systems where tv_sec is unsigned, as for instance HPUX (dunno
whether that's standard or not).  If you manage to underflow
remains.tv_sec then you continue to wait forever ... or at least till
the long wraps around again...

            regards, tom lane