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

On Fri, Feb 12, 2021 at 03:21:40PM +0900, Kyotaro Horiguchi wrote:
> The v3 drops the changes of the uuid_ossp contrib.  I'm not sure the
> change of scram_HMAC_final is needed.

Meaning that v3 would fail to compile uuid-ossp.  v3 also produces
compilation warnings in auth-scram.c.

> In v2, int_md5_finish() calls pg_cryptohash_final() with
> h->block_size(h) (64) but it should be h->result_size(h)
> (16). int_sha1_finish() is wrong the same way. (and, v3 seems fixing
> them in the wrong way.)

Right.  These should just use h->result_size(h), and not
h->block_size(h).

-extern int scram_HMAC_final(uint8 *result, scram_HMAC_ctx *ctx);
+extern int scram_HMAC_final(scram_HMAC_ctx *ctx, uint8 *result);
There is no point in this change.  You just make back-patching harder
while doing nothing about the problem at hand.

-   if (pg_cryptohash_final(manifest->manifest_ctx, checksumbuf) < 0)
+   if (pg_cryptohash_final(manifest->manifest_ctx, checksumbuf,
+                           PG_SHA256_DIGEST_LENGTH) < 0)
Here this could just use sizeof(checksumbuf)?  This pattern could be
used elsewhere as well, like in md5_common.c.