From: | Noah Misch <noah(at)leadboat(dot)com> |
---|---|
To: | Zhihong Yu <zyu(at)yugabyte(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: null iv parameter passed to combo_init() |
Date: | 2022-01-09 01:52:02 |
Message-ID: | 20220109015202.GB283924@rfd.leadboat.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Jan 07, 2022 at 04:32:01PM -0800, Zhihong Yu wrote:
> In contrib/pgcrypto/pgcrypto.c :
>
> err = px_combo_init(c, (uint8 *) VARDATA_ANY(key), klen, NULL, 0);
>
> Note: NULL is passed as iv.
>
> When combo_init() is called,
>
> if (ivlen > ivs)
> memcpy(ivbuf, iv, ivs);
> else
> memcpy(ivbuf, iv, ivlen);
>
> It seems we need to consider the case of null being passed as iv for
> memcpy() because of this:
>
> /usr/include/string.h:44:28: note: nonnull attribute specified here
I agree it's time to fix cases like this, given
https://postgr.es/m/flat/20200904023648(dot)GB3426768(at)rfd(dot)leadboat(dot)com(dot) However,
it should be one patch fixing all (or at least many) of them.
> --- a/contrib/pgcrypto/px.c
> +++ b/contrib/pgcrypto/px.c
> @@ -198,10 +198,13 @@ combo_init(PX_Combo *cx, const uint8 *key, unsigned klen,
> if (ivs > 0)
> {
> ivbuf = palloc0(ivs);
> - if (ivlen > ivs)
> - memcpy(ivbuf, iv, ivs);
> - else
> - memcpy(ivbuf, iv, ivlen);
> + if (iv != NULL)
> + {
> + if (ivlen > ivs)
> + memcpy(ivbuf, iv, ivs);
> + else
> + memcpy(ivbuf, iv, ivlen);
> + }
> }
If someone were to pass NULL iv with nonzero ivlen, that will silently
malfunction. I'd avoid that risk by writing this way:
--- a/contrib/pgcrypto/px.c
+++ b/contrib/pgcrypto/px.c
@@ -202,3 +202,3 @@ combo_init(PX_Combo *cx, const uint8 *key, unsigned klen,
memcpy(ivbuf, iv, ivs);
- else
+ else if (ivlen > 0)
memcpy(ivbuf, iv, ivlen);
That also gives the compiler an additional optimization strategy.
From | Date | Subject | |
---|---|---|---|
Next Message | Zhihong Yu | 2022-01-09 02:52:14 | Re: null iv parameter passed to combo_init() |
Previous Message | Tom Lane | 2022-01-09 01:17:16 | Re: Why is src/test/modules/committs/t/002_standby.pl flaky? |