Re: pg_cryptohash_final possible out-of-bounds access (per Coverity)

Ranier Vilela <ranier.vf@gmail.com>

From: Ranier Vilela <ranier.vf@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2021-02-14T00:33:48Z
Lists: pgsql-hackers
Em sáb., 13 de fev. de 2021 às 20:32, Michael Paquier <michael@paquier.xyz>
escreveu:

> On Sat, Feb 13, 2021 at 05:37:32PM -0300, Ranier Vilela wrote:
> > IMO there is no necessity in back-patching.
>
> You are missing the point here.  What you are proposing here would not
> be backpatched.  However, reusing the same words as upthread, this has
> a cost in terms of *future* maintenance.  In short, any *future*
> potential bug fix that would require to be backpatched in need of
> using this function or touching its area would result in a conflict.
>
Ok. +1 for back-patching.

Any future maintenance, or use of that functions, need to consult the api.

scram_HMAC_init(scram_HMAC_ctx *ctx, const uint8 *key, int keylen);
scram_HMAC_update(scram_HMAC_ctx *ctx, const char *str, int slen);
scram_HMAC_final(uint8 *result, scram_HMAC_ctx *ctx);

See both "result" and "ctx" are pointers.
Someone can use like this:

scram_HMAC_init(&ctx, key, keylen);
scram_HMAC_update(&ctx, str, slen);
scram_HMAC_final(&ctx, result); // parameters wrong order

And many compilers won't complain.

regards,
Ranier Vilela

Commits

  1. Add result size as argument of pg_cryptohash_final() for overflow checks