oauth integer overflow - Mailing list pgsql-hackers

Hi,

I was once more looking at what it'd take to work with -ftrapv, when cassert
is enabled.  Partially motivated with the worry that we support compilers that
don't understand -fwrapv so code relying on signed overflow isn't actually
safe. And because it sometimes leads to unexpected results that can cause
trouble and -ftrapv helps find those.

One thing that quickly triggers when doing so is:

#0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at
./nptl/pthread_kill.c:44
#1  0x00007fe1a05a9dbf in __pthread_kill_internal (threadid=<optimized out>, signo=6) at ./nptl/pthread_kill.c:89
#2  0x00007fe1a0552d02 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#3  0x00007fe1a053a4b2 in __GI_abort () at ./stdlib/abort.c:77
#4  0x00007fe19f218ac1 in __addvsi3 ()
   from
/srv/dev/build/postgres/m-dev-assert/tmp_install//srv/dev/install/postgres/m-dev-assert/lib/x86_64-linux-gnu/libpq-oauth.so
#5  0x00007fe19f20da05 in handle_token_response (actx=0x561825a06350, token=0x7ffc2f677e48)
    at ../../../../../home/andres/src/postgresql/src/interfaces/libpq-oauth/oauth-curl.c:2625
#6  0x00007fe19f20dec5 in pg_fe_run_oauth_flow_impl (conn=0x561825998490, request=0x561825a06e30,
altsock=0x561825998860)
    at ../../../../../home/andres/src/postgresql/src/interfaces/libpq-oauth/oauth-curl.c:2924
#7  0x00007fe19f20e0a8 in pg_fe_run_oauth_flow (conn=0x561825998490, request=0x561825a06e30, altsock=0x561825998860)
    at ../../../../../home/andres/src/postgresql/src/interfaces/libpq-oauth/oauth-curl.c:3027
#8  0x00007fe1a0a35627 in do_async (state=0x5618259a1880, request=0x561825a06e30)
    at ../../../../../home/andres/src/postgresql/src/interfaces/libpq/fe-auth-oauth.c:1528
#9  0x00007fe1a0a346e3 in run_oauth_flow (conn=0x561825998490) at
../../../../../home/andres/src/postgresql/src/interfaces/libpq/fe-auth-oauth.c:751
#10 0x00007fe1a0a3ffc1 in PQconnectPoll (conn=0x561825998490) at
../../../../../home/andres/src/postgresql/src/interfaces/libpq/fe-connect.c:4302
#11 0x00007fe1a0a3db6c in pqConnectDBComplete (conn=0x561825998490) at
../../../../../home/andres/src/postgresql/src/interfaces/libpq/fe-connect.c:2893
#12 0x00007fe1a0a3a1f7 in PQconnectdbParams (keywords=0x5618259983f0, values=0x561825998440, expand_dbname=1)

    /*
     * A slow_down error requires us to permanently increase our retry
     * interval by five seconds.
     */
    if (strcmp(err->error, "slow_down") == 0)
    {
        int            prev_interval = actx->authz.interval;

        actx->authz.interval += 5;
        if (actx->authz.interval < prev_interval)
        {
            actx_error(actx, "slow_down interval overflow");
            goto token_cleanup;
        }
    }

I don't think it's safe to rely on that type of check working without -fwrapv
support, which in turn we can't rely on having.

I think this should use pg_add_s32_overflow().

Greetings,

Andres Freund